| Message ID | 20210922152642.1435-1-venugopali@nvidia.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] controller: Allow specifying tos option for tunnel interface | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Wed, Sep 22, 2021 at 8:26 AM <venugopali@nvidia.com> wrote: > > From: Venugopal Iyer <venugopali@nvidia.com> > > Currently, OVN tunnel interface supports the csum option along > with remote_ip and key. There are applications (e.g. RoCE) that rely > on setting the DSCP bits and expect it to be moved to the outer/ > tunnel header as well. > > This commit adds an "ovn-encap-tos" external-id that can be used to > set the tos option on the OVS tunnel interface, using: > > ovs-vsctl set Open_vSwitch . external_ids:ovn-encap-tos=inherit > > Tested by setting the external_id (as above) and checking the geneve > interfaces created, e.g: > > options : {csum="true", key=flow, remote_ip="X.X.X.X", tos=inherit} > > Also, added a simple test case to make sure the tos option is carried to the > tunnel interface when set. > > Signed-off-by: Venugopal Iyer <venugopali@nvidia.com> > --- > controller/encaps.c | 26 ++++++++++-- > controller/encaps.h | 1 + > controller/ovn-controller.8.xml | 7 +++ > controller/ovn-controller.c | 1 + > tests/ovn-controller.at | 75 +++++++++++++++++++++++++++++++++ > 5 files changed, 106 insertions(+), 4 deletions(-) > > diff --git a/controller/encaps.c b/controller/encaps.c > index fc93bf1ee..0666548f9 100644 > --- a/controller/encaps.c > +++ b/controller/encaps.c > @@ -152,7 +152,8 @@ encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id, > > static void > tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg, > - const char *new_chassis_id, const struct sbrec_encap *encap) > + const char *new_chassis_id, const struct sbrec_encap *encap, > + const struct ovsrec_open_vswitch_table *ovs_table) > { > struct smap options = SMAP_INITIALIZER(&options); > smap_add(&options, "remote_ip", encap->ip); > @@ -202,6 +203,18 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg, > smap_add(&options, "remote_name", new_chassis_id); > } > > + const struct ovsrec_open_vswitch *cfg = > + ovsrec_open_vswitch_table_first(ovs_table); > + /* If the tos option is configured, get it */ > + if (cfg) { > + const char *encap_tos = smap_get_def(&cfg->external_ids, > + "ovn-encap-tos", "none"); > + > + if (encap_tos && strcmp(encap_tos, "none")) { > + smap_add(&options, "tos", encap_tos); > + } > + } > + > /* If there's an existing chassis record that does not need any change, > * keep it. Otherwise, create a new record (if there was an existing > * record, the new record will supplant it and encaps_run() will delete > @@ -270,7 +283,10 @@ preferred_encap(const struct sbrec_chassis *chassis_rec) > * as there are VTEP of that type (differentiated by remote_ip) on that chassis. > */ > static int > -chassis_tunnel_add(const struct sbrec_chassis *chassis_rec, const struct sbrec_sb_global *sbg, struct tunnel_ctx *tc) > +chassis_tunnel_add(const struct sbrec_chassis *chassis_rec, > + const struct sbrec_sb_global *sbg, > + const struct ovsrec_open_vswitch_table *ovs_table, > + struct tunnel_ctx *tc) > { > struct sbrec_encap *encap = preferred_encap(chassis_rec); > int tuncnt = 0; > @@ -286,7 +302,8 @@ chassis_tunnel_add(const struct sbrec_chassis *chassis_rec, const struct sbrec_s > if (tun_type != pref_type) { > continue; > } > - tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i]); > + tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i], > + ovs_table); > tuncnt++; > } > return tuncnt; > @@ -321,6 +338,7 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > const struct sbrec_chassis_table *chassis_table, > const struct sbrec_chassis *this_chassis, > const struct sbrec_sb_global *sbg, > + const struct ovsrec_open_vswitch_table *ovs_table, > const struct sset *transport_zones) > { > if (!ovs_idl_txn || !br_int) { > @@ -390,7 +408,7 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > continue; > } > > - if (chassis_tunnel_add(chassis_rec, sbg, &tc) == 0) { > + if (chassis_tunnel_add(chassis_rec, sbg, ovs_table, &tc) == 0) { > VLOG_INFO("Creating encap for '%s' failed", chassis_rec->name); > continue; > } > diff --git a/controller/encaps.h b/controller/encaps.h > index f488393c4..25d44b034 100644 > --- a/controller/encaps.h > +++ b/controller/encaps.h > @@ -35,6 +35,7 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > const struct sbrec_chassis_table *, > const struct sbrec_chassis *, > const struct sbrec_sb_global *, > + const struct ovsrec_open_vswitch_table *, > const struct sset *transport_zones); > > bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, > diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml > index 8c180f576..37c7efb5b 100644 > --- a/controller/ovn-controller.8.xml > +++ b/controller/ovn-controller.8.xml > @@ -201,6 +201,13 @@ > performance loss. In such cases, encapsulation checksums can be disabled. > </dd> > > + <dt><code>external_ids:ovn-encap-tos</code></dt> > + <dd> > + <code>ovn-encap-tos</code> indicates the value to be applied to OVN > + tunnel interface's option:tos as specified in the Open_vSwitch database > + Interface table. Please refer to Open VSwitch Manual for details. > + </dd> > + > <dt><code>external_ids:ovn-cms-options</code></dt> > <dd> > A list of options that will be consumed by the CMS Plugin and which > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index aa7941eeb..a719beb0e 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -3593,6 +3593,7 @@ main(int argc, char *argv[]) > sbrec_chassis_table_get(ovnsb_idl_loop.idl), > chassis, > sbrec_sb_global_first(ovnsb_idl_loop.idl), > + ovs_table, > &transport_zones); > > stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME, > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index 4ae218ed6..a1169c569 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -723,3 +723,78 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=0 > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +# Checks that ovn-controller honors the change to tunnel interface if we > +# set ovn-encap-tos option > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn-controller - change Encap ToS option]) > +AT_KEYWORDS([ovn]) > +ovn_start > + > +net_add n1 > +sim_add hv > +as hv > +ovs-vsctl \ > + -- add-br br-phys \ > + -- add-br br-eth0 \ > + -- add-br br-eth1 \ > + -- add-br br-eth2 > +ovn_attach n1 br-phys 192.168.0.1 > + > +check_tunnel_property () { > + test "`ovs-vsctl get interface ovn-fakech-0 $1`" = "$2" > +} > + > +# without any tos options > +no_tos_options="{csum=\"true\", key=flow, remote_ip=\"192.168.0.2\"}" > + > +# > +# Start off with a remote chassis supporting geneve > +ovn-sbctl chassis-add fakechassis geneve 192.168.0.2 > +OVS_WAIT_UNTIL([check_tunnel_property type geneve]) > + > +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options) > +AT_CHECK([test "$tos_option" = "$no_tos_options"], [0], []) > + > +expected_tos_option="inherit" > +as hv > +ovs-vsctl \ > + -- set Open_vSwitch . external-ids:ovn-encap-tos="$expected_tos_option" > + > +# now, wait for a sec > +sleep 1 > + > +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options:tos) > +AT_CHECK([test "$tos_option" = "$expected_tos_option"], [0], []) > + > +# Try another value > +expected_tos_option="61" > +as hv > +ovs-vsctl \ > + -- set Open_vSwitch . external-ids:ovn-encap-tos="$expected_tos_option" > + > +# now, wait for a sec > +sleep 1 > + > +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options:tos) > +AT_CHECK([test "$tos_option" = "\"$expected_tos_option\""], [0], []) > + > +# Remove tos option and check if we are back to the original state > +as hv > +ovs-vsctl \ > + -- remove Open_vSwitch . external-ids column ovn-encap-tos > + > +# now, wait for a sec > +sleep 1 > + > +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options) > +AT_CHECK([test "$tos_option" = "$no_tos_options"], [0], []) > + > +# Gracefully terminate daemons > +OVN_CLEANUP_SBOX([hv]) > +OVN_CLEANUP_VSWITCH([main]) > +as ovn-sb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +AT_CLEANUP > +]) > -- > 2.17.1 > Thanks Venu, I merged it.
From: robot@bytheb.org
Test-Label: github-robot: Build and Test
Test-Status: fail
http://patchwork.ozlabs.org/api/patches/1531292/
_github build: failed_
Build URL: https://github.com/ovsrobot/ovn/actions/runs/1279056165
Build Logs:
-----------------------Summary of failed steps-----------------------
"linux clang test asan" failed at step build
----------------------End summary of failed steps--------------------
-------------------------------BEGIN LOGS----------------------------
####################################################################################
#### [Begin job log] "linux clang test asan" at step build
####################################################################################
| #define HAVE_LINUX_TYPES_H 1
| #define HAVE_LINUX_IF_ETHER_H 1
| #define HAVE_LINUX_NET_NAMESPACE_H 1
| #define HAVE_STDATOMIC_H 1
| #define HAVE_BITS_FLOATN_COMMON_H 1
| #define HAVE_BACKTRACE 1
| #define HAVE_LINUX_PERF_EVENT_H 1
| #define HAVE_THREAD_LOCAL 1
| #define HAVE_GCC4_ATOMICS 1
| #define ATOMIC_ALWAYS_LOCK_FREE_1B 1
| #define ATOMIC_ALWAYS_LOCK_FREE_2B 1
| #define ATOMIC_ALWAYS_LOCK_FREE_4B 1
| #define ATOMIC_ALWAYS_LOCK_FREE_8B 1
| #define HAVE_GLIBC_PTHREAD_SETNAME_NP 1
| #define HAVE_CXX11 1
| #define HAVE_ATOMIC 1
| #define HAVE_POSIX_MEMALIGN 1
| #define HAVE_UNBOUND 1
| #define HAVE_STDIO_H 1
| #define HAVE_STRING_H 1
| #define HAVE_PRAGMA_MESSAGE 1
|
| configure: exit 0
##[error]Process completed with exit code 1.
####################################################################################
#### [End job log] "linux clang test asan" at step build
####################################################################################
--------------------------------END LOGS-----------------------------
From: robot@bytheb.org
Test-Label: github-robot: ovn-kubernetes
Test-Status: fail
http://patchwork.ozlabs.org/api/patches/1531292/
_github build: failed_
Build URL: https://github.com/ovsrobot/ovn/actions/runs/1279056162
diff --git a/controller/encaps.c b/controller/encaps.c index fc93bf1ee..0666548f9 100644 --- a/controller/encaps.c +++ b/controller/encaps.c @@ -152,7 +152,8 @@ encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id, static void tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg, - const char *new_chassis_id, const struct sbrec_encap *encap) + const char *new_chassis_id, const struct sbrec_encap *encap, + const struct ovsrec_open_vswitch_table *ovs_table) { struct smap options = SMAP_INITIALIZER(&options); smap_add(&options, "remote_ip", encap->ip); @@ -202,6 +203,18 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg, smap_add(&options, "remote_name", new_chassis_id); } + const struct ovsrec_open_vswitch *cfg = + ovsrec_open_vswitch_table_first(ovs_table); + /* If the tos option is configured, get it */ + if (cfg) { + const char *encap_tos = smap_get_def(&cfg->external_ids, + "ovn-encap-tos", "none"); + + if (encap_tos && strcmp(encap_tos, "none")) { + smap_add(&options, "tos", encap_tos); + } + } + /* If there's an existing chassis record that does not need any change, * keep it. Otherwise, create a new record (if there was an existing * record, the new record will supplant it and encaps_run() will delete @@ -270,7 +283,10 @@ preferred_encap(const struct sbrec_chassis *chassis_rec) * as there are VTEP of that type (differentiated by remote_ip) on that chassis. */ static int -chassis_tunnel_add(const struct sbrec_chassis *chassis_rec, const struct sbrec_sb_global *sbg, struct tunnel_ctx *tc) +chassis_tunnel_add(const struct sbrec_chassis *chassis_rec, + const struct sbrec_sb_global *sbg, + const struct ovsrec_open_vswitch_table *ovs_table, + struct tunnel_ctx *tc) { struct sbrec_encap *encap = preferred_encap(chassis_rec); int tuncnt = 0; @@ -286,7 +302,8 @@ chassis_tunnel_add(const struct sbrec_chassis *chassis_rec, const struct sbrec_s if (tun_type != pref_type) { continue; } - tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i]); + tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i], + ovs_table); tuncnt++; } return tuncnt; @@ -321,6 +338,7 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct sbrec_chassis_table *chassis_table, const struct sbrec_chassis *this_chassis, const struct sbrec_sb_global *sbg, + const struct ovsrec_open_vswitch_table *ovs_table, const struct sset *transport_zones) { if (!ovs_idl_txn || !br_int) { @@ -390,7 +408,7 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, continue; } - if (chassis_tunnel_add(chassis_rec, sbg, &tc) == 0) { + if (chassis_tunnel_add(chassis_rec, sbg, ovs_table, &tc) == 0) { VLOG_INFO("Creating encap for '%s' failed", chassis_rec->name); continue; } diff --git a/controller/encaps.h b/controller/encaps.h index f488393c4..25d44b034 100644 --- a/controller/encaps.h +++ b/controller/encaps.h @@ -35,6 +35,7 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct sbrec_chassis_table *, const struct sbrec_chassis *, const struct sbrec_sb_global *, + const struct ovsrec_open_vswitch_table *, const struct sset *transport_zones); bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml index 8c180f576..37c7efb5b 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -201,6 +201,13 @@ performance loss. In such cases, encapsulation checksums can be disabled. </dd> + <dt><code>external_ids:ovn-encap-tos</code></dt> + <dd> + <code>ovn-encap-tos</code> indicates the value to be applied to OVN + tunnel interface's option:tos as specified in the Open_vSwitch database + Interface table. Please refer to Open VSwitch Manual for details. + </dd> + <dt><code>external_ids:ovn-cms-options</code></dt> <dd> A list of options that will be consumed by the CMS Plugin and which diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index aa7941eeb..a719beb0e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -3593,6 +3593,7 @@ main(int argc, char *argv[]) sbrec_chassis_table_get(ovnsb_idl_loop.idl), chassis, sbrec_sb_global_first(ovnsb_idl_loop.idl), + ovs_table, &transport_zones); stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME, diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 4ae218ed6..a1169c569 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -723,3 +723,78 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=0 OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +# Checks that ovn-controller honors the change to tunnel interface if we +# set ovn-encap-tos option +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller - change Encap ToS option]) +AT_KEYWORDS([ovn]) +ovn_start + +net_add n1 +sim_add hv +as hv +ovs-vsctl \ + -- add-br br-phys \ + -- add-br br-eth0 \ + -- add-br br-eth1 \ + -- add-br br-eth2 +ovn_attach n1 br-phys 192.168.0.1 + +check_tunnel_property () { + test "`ovs-vsctl get interface ovn-fakech-0 $1`" = "$2" +} + +# without any tos options +no_tos_options="{csum=\"true\", key=flow, remote_ip=\"192.168.0.2\"}" + +# +# Start off with a remote chassis supporting geneve +ovn-sbctl chassis-add fakechassis geneve 192.168.0.2 +OVS_WAIT_UNTIL([check_tunnel_property type geneve]) + +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options) +AT_CHECK([test "$tos_option" = "$no_tos_options"], [0], []) + +expected_tos_option="inherit" +as hv +ovs-vsctl \ + -- set Open_vSwitch . external-ids:ovn-encap-tos="$expected_tos_option" + +# now, wait for a sec +sleep 1 + +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options:tos) +AT_CHECK([test "$tos_option" = "$expected_tos_option"], [0], []) + +# Try another value +expected_tos_option="61" +as hv +ovs-vsctl \ + -- set Open_vSwitch . external-ids:ovn-encap-tos="$expected_tos_option" + +# now, wait for a sec +sleep 1 + +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options:tos) +AT_CHECK([test "$tos_option" = "\"$expected_tos_option\""], [0], []) + +# Remove tos option and check if we are back to the original state +as hv +ovs-vsctl \ + -- remove Open_vSwitch . external-ids column ovn-encap-tos + +# now, wait for a sec +sleep 1 + +tos_option=$(ovs-vsctl get interface ovn-fakech-0 options) +AT_CHECK([test "$tos_option" = "$no_tos_options"], [0], []) + +# Gracefully terminate daemons +OVN_CLEANUP_SBOX([hv]) +OVN_CLEANUP_VSWITCH([main]) +as ovn-sb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) + +AT_CLEANUP +])