Message ID | 20240723065716.165340-1-mansi.sharma@nutanix.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev] ct-zones: reserve zones for upcoming ports | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | warning | apply and check: warning |
ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
References: <20240723065716.165340-1-mansi.sharma@nutanix.com> Bleep bloop. Greetings Mansi Sharma, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject summary should start with a capital. WARNING: The subject summary should end with a dot. Subject: ct-zones: reserve zones for upcoming ports WARNING: Line has trailing whitespace #39 FILE: controller/ct-zone.c:109: * chassis and might need ct-zone reservation in advance. WARNING: Line is 81 characters long (recommended limit is 79) #47 FILE: controller/ct-zone.c:117: char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list); WARNING: Line has trailing whitespace #50 FILE: controller/ct-zone.c:120: for (reserve_port = strtok_r(duplicate_reserve_list, ",", WARNING: Line has trailing whitespace #51 FILE: controller/ct-zone.c:121: &save_ptr); reserve_port != NULL; Lines checked: 222, Warnings: 6, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
Ping for review. From: Mansi Sharma <mansi.sharma@nutanix.com> Date: Tuesday, 23 July 2024 at 12:27 PM To: dev@openvswitch.org <dev@openvswitch.org> Cc: Mansi Sharma <mansi.sharma@nutanix.com> Subject: [PATCH ovn] ct-zones: reserve zones for upcoming ports This change will be useful for migration cases, where it can be used to sync ct-entries before port is up on new chassis, resulting in reduced network package drops. It also fulfills the need of any other service which might need advance ct-zone reservation in future. Signed-off-by: Mansi Sharma <mansi.sharma@nutanix.com> --- controller/ct-zone.c | 24 +++++++++- controller/ct-zone.h | 3 +- controller/ovn-controller.c | 2 +- tests/ovn-controller.at | 92 ++++++++++++++++++++++++++++++++++--- 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/controller/ct-zone.c b/controller/ct-zone.c index e4f66a52a..a3c5b90ff 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -89,7 +89,8 @@ ct_zones_restore(struct ct_zone_ctx *ctx, void ct_zones_update(const struct sset *local_lports, - const struct hmap *local_datapaths, struct ct_zone_ctx *ctx) + const struct hmap *local_datapaths, struct ct_zone_ctx *ctx, + struct ovsrec_open_vswitch_table *ovs_table) { struct simap_node *ct_zone; int scan_start = 1; @@ -104,6 +105,27 @@ ct_zones_update(const struct sset *local_lports, sset_add(&all_users, local_lport); } + /* Add local_loprt name which are supposed to come up on the + * chassis and might need ct-zone reservation in advance. + * The data is picked up from ovs_vswitch table. */ + const struct ovsrec_open_vswitch *cfg; + cfg = ovsrec_open_vswitch_table_first(ovs_table); + if (cfg) { + char *reserve_ct_zone_request_list = smap_get(&cfg->external_ids, + "reserve_ct_zones"); + if (reserve_ct_zone_request_list) { + char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list); + char *reserve_port; + char *save_ptr = NULL; + for (reserve_port = strtok_r(duplicate_reserve_list, ",", + &save_ptr); reserve_port != NULL; + reserve_port = strtok_r(NULL, ",", &save_ptr)) { + sset_add(&all_users, reserve_port); + } + free(duplicate_reserve_list); + } + } + /* Local patched datapath (gateway routers) need zones assigned. */ const struct local_datapath *ld; HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { diff --git a/controller/ct-zone.h b/controller/ct-zone.h index 889bdf2fc..f5a6b793e 100644 --- a/controller/ct-zone.h +++ b/controller/ct-zone.h @@ -62,7 +62,8 @@ void ct_zones_restore(struct ct_zone_ctx *ctx, const struct ovsrec_bridge *br_int); void ct_zones_update(const struct sset *local_lports, const struct hmap *local_datapaths, - struct ct_zone_ctx *ctx); + struct ct_zone_ctx *ctx, + struct ovsrec_open_vswitch_table *ovs_table); void ct_zones_commit(const struct ovsrec_bridge *br_int, struct shash *pending_ct_zones); void ct_zones_pending_clear_commited(struct shash *pending); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index d6d001b1a..8764dfefa 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2231,7 +2231,7 @@ en_ct_zones_run(struct engine_node *node, void *data) ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int); ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths, - &ct_zones_data->ctx); + &ct_zones_data->ctx, ovs_table); ct_zones_data->recomputed = true; engine_set_node_state(node, EN_UPDATED); diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 2cb86dc98..6872dc587 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2529,6 +2529,7 @@ check_ovsdb_zone() { check ovs-vsctl add-port br-int ls0-hv1 -- set Interface ls0-hv1 external-ids:iface-id=ls0-hv1 check ovs-vsctl add-port br-int ls0-hv2 -- set Interface ls0-hv2 external-ids:iface-id=ls0-hv2 +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv3,ls0-req-hv4 check ovn-nbctl lr-add lr0 @@ -2554,17 +2555,25 @@ echo "$ct_zones" port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) - +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) snat_zone=$(get_zone_num "$ct_zones" lr0_snat) echo "snat_zone is $snat_zone" -check test "$port1_zone" -ne "$port2_zone" -check test "$port2_zone" -ne "$snat_zone" -check test "$port1_zone" -ne "$snat_zone" +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone)) +for i in $zone_list; do + for j in $zone_list; do + if ["$i" != "$j"]; then + check tests "$i" -ne "$j" + fi + done +done OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) # Now purposely request an SNAT zone for lr0 that conflicts with a zone # currently assigned to a logical port @@ -2579,15 +2588,84 @@ echo "$ct_zones" port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) snat_zone=$(get_zone_num "$ct_zones" lr0_snat) +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) check test "$snat_zone" -eq "$snat_req_zone" -check test "$port1_zone" -ne "$port2_zone" -check test "$port2_zone" -ne "$snat_zone" -check test "$port1_zone" -ne "$snat_zone" + +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone)) +for i in $zone_list; do + for j in $zone_list; do + if ["$i" != "$j"]; then + check tests "$i" -ne "$j" + fi + done +done + +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) + +# Add port named ls0-req-hv3 and check if same zone assigned +# previously get assigned to it this time as well. + +check ovn-nbctl lsp-add ls0 ls0-req-hv3 +check ovs-vsctl -- add-port br-int hv3-vif3 -- \ + set interface hv3-vif3 external-ids:iface-id=ls0-req-hv3 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +echo "$ct_zones" + +check ovn-nbctl --wait=hv sync + +req_port3_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv3) + +check test "$req_port3_zone -eq $req_port3_zone_new" + +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) + +# Checks for two cases after removing entry from ovs_vswitch table - +# 1. If port is already up, ct-zone should be reserved. +# 2. If port is not up yet, ct-zone should not be reserved. + +check ovs-vsctl remove Open_vSwitch . external_ids reserve_ct_zones + +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +echo "$ct_zones" + +req_port3_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv4) + +check test "$req_port3_zone_new" -eq "$req_port3_zone_after_delete" +check test "$req_port4_zone_after_delete" == "" + +# Checks for case when a ct-zone is reserved it comes up on that chassis, and +# gets deleted, but its persisted in ovs_vswitch table, it should persist the +# same zone throughout. + +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv5 +check ovn-nbctl lsp-add ls0 ls0-req-hv5 +check ovs-vsctl -- add-port br-int hv5-vif5 -- \ + set interface hv5-vif5 external-ids:iface-id=ls0-req-hv5 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +echo "$ct_zones" +req_port5_zone=$(get_zone_num "$ct_zones" ls0-req-hv5) + +check ovs-vsctl remove interface hv5-vif5 external_ids iface-id +echo "$ct_zones" +req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5) + +check test "req_port5_zone" -eq "req_port5_zone_new" OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) # Now create a conflict in the OVSDB and restart ovn-controller. -- 2.22.3
On Tue, Aug 6, 2024 at 12:29 AM Mansi Sharma <mansi.sharma@nutanix.com> wrote: > > Ping for review. Thanks for the patch. This patch needs a rebase. And it is failing compilation --- env REAL_CC="gcc" CHECK="sparse -Wsparse-error -I /workspace/ovn-tmp/ovs/include/sparse -m64 -I /usr/local/include -I /usr/include/x86_64-linux-gnu " cgcc -target=x86_64 -DHAVE_CONFIG_H -I. -I../.. -I ../../include -I ../../include -I ../../ovn -I ./include -I ../../lib -I ./lib -I /workspace/ovn-tmp/ovs/include -I /workspace/ovn-tmp/ovs/include -I /workspace/ovn-tmp/ovs/lib -I /workspace/ovn-tmp/ovs/lib -I /workspace/ovn-tmp/ovs -I /workspace/ovn-tmp/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wswitch-bool -Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare -Wshift-negative-value -Wduplicated-cond -Wshadow -Wmultistatement-macros -Wcast-align=strict -Werror -Werror -MT controller/physical.o -MD -MP -MF $depbase.Tpo -c -o controller/physical.o ../../controller/physical.c &&\ 3918mv -f $depbase.Tpo $depbase.Po 3919../../controller/ovn-controller.c:2234:42: error: incorrect type in argument 4 (different modifiers) 3920../../controller/ovn-controller.c:2234:42: expected struct ovsrec_open_vswitch_table *ovs_table 3921../../controller/ovn-controller.c:2234:42: got struct ovsrec_open_vswitch_table const *ovs_table 3922make[2]: *** [Makefile:2385: controller/ovn-controller.o] Error 1 3923make[2]: *** Waiting for unfinished jobs.... 3924make[2]: Leaving directory '/workspace/ovn-tmp/ovn-24.03.90/_build/sub' 3925make[1]: *** [Makefile:1537: all] Error 2 3926make[1]: Leaving directory '/workspace/ovn-tmp/ovn-24.03.90/_build/sub' 3927make: *** [Makefile:2878: distcheck] Error 1 3928+ cat '*/_build/sub/tests/testsuite.log' 3929cat: '*/_build/sub/tests/testsuite.log': No such file or directory ---- Please see the CI run for this patch - https://github.com/ovsrobot/ovn/actions/runs/10054521567/job/27789433135 Also you need to document the option "reserve_ct_zones" in controller/ovn-controller.8.xml Thanks Numan > > From: Mansi Sharma <mansi.sharma@nutanix.com> > Date: Tuesday, 23 July 2024 at 12:27 PM > To: dev@openvswitch.org <dev@openvswitch.org> > Cc: Mansi Sharma <mansi.sharma@nutanix.com> > Subject: [PATCH ovn] ct-zones: reserve zones for upcoming ports > This change will be useful for migration cases, where it can be > used to sync ct-entries before port is up on new chassis, > resulting in reduced network package drops. > It also fulfills the need of any other service which might need > advance ct-zone reservation in future. > > Signed-off-by: Mansi Sharma <mansi.sharma@nutanix.com> > --- > controller/ct-zone.c | 24 +++++++++- > controller/ct-zone.h | 3 +- > controller/ovn-controller.c | 2 +- > tests/ovn-controller.at | 92 ++++++++++++++++++++++++++++++++++--- > 4 files changed, 111 insertions(+), 10 deletions(-) > > diff --git a/controller/ct-zone.c b/controller/ct-zone.c > index e4f66a52a..a3c5b90ff 100644 > --- a/controller/ct-zone.c > +++ b/controller/ct-zone.c > @@ -89,7 +89,8 @@ ct_zones_restore(struct ct_zone_ctx *ctx, > > void > ct_zones_update(const struct sset *local_lports, > - const struct hmap *local_datapaths, struct ct_zone_ctx *ctx) > + const struct hmap *local_datapaths, struct ct_zone_ctx *ctx, > + struct ovsrec_open_vswitch_table *ovs_table) > { > struct simap_node *ct_zone; > int scan_start = 1; > @@ -104,6 +105,27 @@ ct_zones_update(const struct sset *local_lports, > sset_add(&all_users, local_lport); > } > > + /* Add local_loprt name which are supposed to come up on the > + * chassis and might need ct-zone reservation in advance. > + * The data is picked up from ovs_vswitch table. */ > + const struct ovsrec_open_vswitch *cfg; > + cfg = ovsrec_open_vswitch_table_first(ovs_table); > + if (cfg) { > + char *reserve_ct_zone_request_list = smap_get(&cfg->external_ids, > + "reserve_ct_zones"); > + if (reserve_ct_zone_request_list) { > + char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list); > + char *reserve_port; > + char *save_ptr = NULL; > + for (reserve_port = strtok_r(duplicate_reserve_list, ",", > + &save_ptr); reserve_port != NULL; > + reserve_port = strtok_r(NULL, ",", &save_ptr)) { > + sset_add(&all_users, reserve_port); > + } > + free(duplicate_reserve_list); > + } > + } > + > /* Local patched datapath (gateway routers) need zones assigned. */ > const struct local_datapath *ld; > HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { > diff --git a/controller/ct-zone.h b/controller/ct-zone.h > index 889bdf2fc..f5a6b793e 100644 > --- a/controller/ct-zone.h > +++ b/controller/ct-zone.h > @@ -62,7 +62,8 @@ void ct_zones_restore(struct ct_zone_ctx *ctx, > const struct ovsrec_bridge *br_int); > void ct_zones_update(const struct sset *local_lports, > const struct hmap *local_datapaths, > - struct ct_zone_ctx *ctx); > + struct ct_zone_ctx *ctx, > + struct ovsrec_open_vswitch_table *ovs_table); > void ct_zones_commit(const struct ovsrec_bridge *br_int, > struct shash *pending_ct_zones); > void ct_zones_pending_clear_commited(struct shash *pending); > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index d6d001b1a..8764dfefa 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -2231,7 +2231,7 @@ en_ct_zones_run(struct engine_node *node, void *data) > > ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int); > ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths, > - &ct_zones_data->ctx); > + &ct_zones_data->ctx, ovs_table); > > ct_zones_data->recomputed = true; > engine_set_node_state(node, EN_UPDATED); > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index 2cb86dc98..6872dc587 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -2529,6 +2529,7 @@ check_ovsdb_zone() { > > check ovs-vsctl add-port br-int ls0-hv1 -- set Interface ls0-hv1 external-ids:iface-id=ls0-hv1 > check ovs-vsctl add-port br-int ls0-hv2 -- set Interface ls0-hv2 external-ids:iface-id=ls0-hv2 > +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv3,ls0-req-hv4 > > check ovn-nbctl lr-add lr0 > > @@ -2554,17 +2555,25 @@ echo "$ct_zones" > > port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) > port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) > - > +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) > +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) > snat_zone=$(get_zone_num "$ct_zones" lr0_snat) > echo "snat_zone is $snat_zone" > > -check test "$port1_zone" -ne "$port2_zone" > -check test "$port2_zone" -ne "$snat_zone" > -check test "$port1_zone" -ne "$snat_zone" > +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone)) > +for i in $zone_list; do > + for j in $zone_list; do > + if ["$i" != "$j"]; then > + check tests "$i" -ne "$j" > + fi > + done > +done > > OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) > OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) > OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) > > # Now purposely request an SNAT zone for lr0 that conflicts with a zone > # currently assigned to a logical port > @@ -2579,15 +2588,84 @@ echo "$ct_zones" > port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) > port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) > snat_zone=$(get_zone_num "$ct_zones" lr0_snat) > +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) > +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) > > check test "$snat_zone" -eq "$snat_req_zone" > -check test "$port1_zone" -ne "$port2_zone" > -check test "$port2_zone" -ne "$snat_zone" > -check test "$port1_zone" -ne "$snat_zone" > + > +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone)) > +for i in $zone_list; do > + for j in $zone_list; do > + if ["$i" != "$j"]; then > + check tests "$i" -ne "$j" > + fi > + done > +done > + > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) > + > +# Add port named ls0-req-hv3 and check if same zone assigned > +# previously get assigned to it this time as well. > + > +check ovn-nbctl lsp-add ls0 ls0-req-hv3 > +check ovs-vsctl -- add-port br-int hv3-vif3 -- \ > + set interface hv3-vif3 external-ids:iface-id=ls0-req-hv3 > +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) > +echo "$ct_zones" > + > +check ovn-nbctl --wait=hv sync > + > +req_port3_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv3) > + > +check test "$req_port3_zone -eq $req_port3_zone_new" > + > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) > + > +# Checks for two cases after removing entry from ovs_vswitch table - > +# 1. If port is already up, ct-zone should be reserved. > +# 2. If port is not up yet, ct-zone should not be reserved. > + > +check ovs-vsctl remove Open_vSwitch . external_ids reserve_ct_zones > + > +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) > +echo "$ct_zones" > + > +req_port3_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv3) > +req_port4_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv4) > + > +check test "$req_port3_zone_new" -eq "$req_port3_zone_after_delete" > +check test "$req_port4_zone_after_delete" == "" > + > +# Checks for case when a ct-zone is reserved it comes up on that chassis, and > +# gets deleted, but its persisted in ovs_vswitch table, it should persist the > +# same zone throughout. > + > +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv5 > +check ovn-nbctl lsp-add ls0 ls0-req-hv5 > +check ovs-vsctl -- add-port br-int hv5-vif5 -- \ > + set interface hv5-vif5 external-ids:iface-id=ls0-req-hv5 > +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) > +echo "$ct_zones" > +req_port5_zone=$(get_zone_num "$ct_zones" ls0-req-hv5) > + > +check ovs-vsctl remove interface hv5-vif5 external_ids iface-id > +echo "$ct_zones" > +req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5) > + > +check test "req_port5_zone" -eq "req_port5_zone_new" > > OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) > OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) > OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) > +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) > > # Now create a conflict in the OVSDB and restart ovn-controller. > > -- > 2.22.3 > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/ct-zone.c b/controller/ct-zone.c index e4f66a52a..a3c5b90ff 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -89,7 +89,8 @@ ct_zones_restore(struct ct_zone_ctx *ctx, void ct_zones_update(const struct sset *local_lports, - const struct hmap *local_datapaths, struct ct_zone_ctx *ctx) + const struct hmap *local_datapaths, struct ct_zone_ctx *ctx, + struct ovsrec_open_vswitch_table *ovs_table) { struct simap_node *ct_zone; int scan_start = 1; @@ -104,6 +105,27 @@ ct_zones_update(const struct sset *local_lports, sset_add(&all_users, local_lport); } + /* Add local_loprt name which are supposed to come up on the + * chassis and might need ct-zone reservation in advance. + * The data is picked up from ovs_vswitch table. */ + const struct ovsrec_open_vswitch *cfg; + cfg = ovsrec_open_vswitch_table_first(ovs_table); + if (cfg) { + char *reserve_ct_zone_request_list = smap_get(&cfg->external_ids, + "reserve_ct_zones"); + if (reserve_ct_zone_request_list) { + char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list); + char *reserve_port; + char *save_ptr = NULL; + for (reserve_port = strtok_r(duplicate_reserve_list, ",", + &save_ptr); reserve_port != NULL; + reserve_port = strtok_r(NULL, ",", &save_ptr)) { + sset_add(&all_users, reserve_port); + } + free(duplicate_reserve_list); + } + } + /* Local patched datapath (gateway routers) need zones assigned. */ const struct local_datapath *ld; HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { diff --git a/controller/ct-zone.h b/controller/ct-zone.h index 889bdf2fc..f5a6b793e 100644 --- a/controller/ct-zone.h +++ b/controller/ct-zone.h @@ -62,7 +62,8 @@ void ct_zones_restore(struct ct_zone_ctx *ctx, const struct ovsrec_bridge *br_int); void ct_zones_update(const struct sset *local_lports, const struct hmap *local_datapaths, - struct ct_zone_ctx *ctx); + struct ct_zone_ctx *ctx, + struct ovsrec_open_vswitch_table *ovs_table); void ct_zones_commit(const struct ovsrec_bridge *br_int, struct shash *pending_ct_zones); void ct_zones_pending_clear_commited(struct shash *pending); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index d6d001b1a..8764dfefa 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2231,7 +2231,7 @@ en_ct_zones_run(struct engine_node *node, void *data) ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int); ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths, - &ct_zones_data->ctx); + &ct_zones_data->ctx, ovs_table); ct_zones_data->recomputed = true; engine_set_node_state(node, EN_UPDATED); diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 2cb86dc98..6872dc587 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2529,6 +2529,7 @@ check_ovsdb_zone() { check ovs-vsctl add-port br-int ls0-hv1 -- set Interface ls0-hv1 external-ids:iface-id=ls0-hv1 check ovs-vsctl add-port br-int ls0-hv2 -- set Interface ls0-hv2 external-ids:iface-id=ls0-hv2 +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv3,ls0-req-hv4 check ovn-nbctl lr-add lr0 @@ -2554,17 +2555,25 @@ echo "$ct_zones" port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) - +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) snat_zone=$(get_zone_num "$ct_zones" lr0_snat) echo "snat_zone is $snat_zone" -check test "$port1_zone" -ne "$port2_zone" -check test "$port2_zone" -ne "$snat_zone" -check test "$port1_zone" -ne "$snat_zone" +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone)) +for i in $zone_list; do + for j in $zone_list; do + if ["$i" != "$j"]; then + check tests "$i" -ne "$j" + fi + done +done OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) # Now purposely request an SNAT zone for lr0 that conflicts with a zone # currently assigned to a logical port @@ -2579,15 +2588,84 @@ echo "$ct_zones" port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) snat_zone=$(get_zone_num "$ct_zones" lr0_snat) +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) check test "$snat_zone" -eq "$snat_req_zone" -check test "$port1_zone" -ne "$port2_zone" -check test "$port2_zone" -ne "$snat_zone" -check test "$port1_zone" -ne "$snat_zone" + +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone)) +for i in $zone_list; do + for j in $zone_list; do + if ["$i" != "$j"]; then + check tests "$i" -ne "$j" + fi + done +done + +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) + +# Add port named ls0-req-hv3 and check if same zone assigned +# previously get assigned to it this time as well. + +check ovn-nbctl lsp-add ls0 ls0-req-hv3 +check ovs-vsctl -- add-port br-int hv3-vif3 -- \ + set interface hv3-vif3 external-ids:iface-id=ls0-req-hv3 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +echo "$ct_zones" + +check ovn-nbctl --wait=hv sync + +req_port3_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv3) + +check test "$req_port3_zone -eq $req_port3_zone_new" + +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) + +# Checks for two cases after removing entry from ovs_vswitch table - +# 1. If port is already up, ct-zone should be reserved. +# 2. If port is not up yet, ct-zone should not be reserved. + +check ovs-vsctl remove Open_vSwitch . external_ids reserve_ct_zones + +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +echo "$ct_zones" + +req_port3_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv4) + +check test "$req_port3_zone_new" -eq "$req_port3_zone_after_delete" +check test "$req_port4_zone_after_delete" == "" + +# Checks for case when a ct-zone is reserved it comes up on that chassis, and +# gets deleted, but its persisted in ovs_vswitch table, it should persist the +# same zone throughout. + +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv5 +check ovn-nbctl lsp-add ls0 ls0-req-hv5 +check ovs-vsctl -- add-port br-int hv5-vif5 -- \ + set interface hv5-vif5 external-ids:iface-id=ls0-req-hv5 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +echo "$ct_zones" +req_port5_zone=$(get_zone_num "$ct_zones" ls0-req-hv5) + +check ovs-vsctl remove interface hv5-vif5 external_ids iface-id +echo "$ct_zones" +req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5) + +check test "req_port5_zone" -eq "req_port5_zone_new" OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) # Now create a conflict in the OVSDB and restart ovn-controller.
This change will be useful for migration cases, where it can be used to sync ct-entries before port is up on new chassis, resulting in reduced network package drops. It also fulfills the need of any other service which might need advance ct-zone reservation in future. Signed-off-by: Mansi Sharma <mansi.sharma@nutanix.com> --- controller/ct-zone.c | 24 +++++++++- controller/ct-zone.h | 3 +- controller/ovn-controller.c | 2 +- tests/ovn-controller.at | 92 ++++++++++++++++++++++++++++++++++--- 4 files changed, 111 insertions(+), 10 deletions(-)