Message ID | 20240917062145.74454-1-mansi.sharma@nutanix.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,v6] ovn-controller: Reserve zones for upcoming ports. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Tue, Sep 17, 2024 at 8:22 AM Mansi Sharma <mansi.sharma@nutanix.com> wrote: > 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> > --- > v5-->v6 > Updated tests to check duplicates more frequntly with input as ct_zones. > --- > controller/ct-zone.c | 25 ++++++++++ > controller/ovn-controller.8.xml | 15 +++++- > tests/ovn-controller.at | 86 ++++++++++++++++++++++++++++++--- > 3 files changed, 117 insertions(+), 9 deletions(-) > > diff --git a/controller/ct-zone.c b/controller/ct-zone.c > index 77eb16ac9..bd1b0623b 100644 > --- a/controller/ct-zone.c > +++ b/controller/ct-zone.c > @@ -183,6 +183,31 @@ ct_zones_update(const struct sset *local_lports, > sset_add(&all_users, local_lport); > } > > + /* Add local_lport 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) { > + const char *reserve_ct_zone_request_list = smap_get( > + &cfg->external_ids, "reserve_ct_zones"); > + > + if (reserve_ct_zone_request_list) { > + char *dup_reserve = xstrdup(reserve_ct_zone_request_list); > + char *reserve_port; > + char *save_ptr = NULL; > + > + for (reserve_port = strtok_r(dup_reserve, ",", &save_ptr); > + reserve_port != NULL; > + reserve_port = strtok_r(NULL, ",", &save_ptr)) { > + sset_add(&all_users, reserve_port); > + } > + > + free(dup_reserve); > + } > + } > + > /* 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/ovn-controller.8.xml > b/controller/ovn-controller.8.xml > index faefa77b9..b070b724c 100644 > --- a/controller/ovn-controller.8.xml > +++ b/controller/ovn-controller.8.xml > @@ -626,7 +626,20 @@ > <code>external_ids:ovn-installed-ts</code>. > </p> > </dd> > - </dl> > + > + <dt> > + <code>external-ids:reserve_ct_zones</code> in the > <code>Bridge</code> > + table > + </dt> > + > + <dd> > + <p> > + This key represents list of ports which are supposed to come up > on > + the chassis, and hence need advance reservation of ct-zones. > + It is comma seprated list of port names. > + </p> > + </dd> > + </dl> > > <h1>OVN Southbound Database Usage</h1> > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index 74bff9035..4377ac6f2 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -2533,8 +2533,14 @@ check_ovsdb_zone() { > test $ct_zone -eq $db_zone > } > > +check_duplicates() { > + output=$1 > + AT_CHECK([printf "$output" | tr ' ' '\n' | sort | uniq -d | grep .], > [1]) > +} > + > 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 > > @@ -2560,17 +2566,18 @@ 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" > - > 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]) > + > +check_duplicates "$ct_zones" > > # Now purposely request an SNAT zone for lr0 that conflicts with a zone > # currently assigned to a logical port > @@ -2585,15 +2592,78 @@ 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" > + > +check_duplicates "$ct_zones" > + > +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" > +check_duplicates "$ct_zones" > + > +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" > +check_duplicates "$ct_zones" > > 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 > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
On Tue, Sep 17, 2024 at 2:31 AM Ales Musil <amusil@redhat.com> wrote: > > On Tue, Sep 17, 2024 at 8:22 AM Mansi Sharma <mansi.sharma@nutanix.com> > wrote: > > > 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> Thanks for v6. During my testing I found one issue related to incremental processing. Looks like that needs to be fixed. Below are the steps --------------- ovn-nbctl ls-add sw0 ovn-nbctl lsp-add sw0 sw0p1 ovs-vsctl \ -- add-port br-int vif1 \ -- set Interface vif1 external_ids:iface-id=sw0p1 ovn-nbctl --wait=hv sync ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=sw0p1 ovs-vsctl get bridge br-int external_ids:ct-zone-sw0p1 # Delete the lsp - sw0p1 ovn-nbctl --wait=hv lsp-del sw0p1 # This below command returns error since the zone id for sw0p1 is not reserved. ovs-vsctl get bridge br-int external_ids:ct-zone-sw0p1 # After recompute, the problem is fixed. ovn-appctl -t ovn-controller inc-engine/recompute ovs-vsctl get bridge br-int external_ids:ct-zone-sw0p1 ------------ I think you also need to add some code in ct_zones_runtime_data_handler() to address this issue. Other than this, the patch LGTM. Thanks Numan > > --- > > v5-->v6 > > Updated tests to check duplicates more frequntly with input as ct_zones. > > --- > > controller/ct-zone.c | 25 ++++++++++ > > controller/ovn-controller.8.xml | 15 +++++- > > tests/ovn-controller.at | 86 ++++++++++++++++++++++++++++++--- > > 3 files changed, 117 insertions(+), 9 deletions(-) > > > > diff --git a/controller/ct-zone.c b/controller/ct-zone.c > > index 77eb16ac9..bd1b0623b 100644 > > --- a/controller/ct-zone.c > > +++ b/controller/ct-zone.c > > @@ -183,6 +183,31 @@ ct_zones_update(const struct sset *local_lports, > > sset_add(&all_users, local_lport); > > } > > > > + /* Add local_lport 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) { > > + const char *reserve_ct_zone_request_list = smap_get( > > + &cfg->external_ids, "reserve_ct_zones"); > > + > > + if (reserve_ct_zone_request_list) { > > + char *dup_reserve = xstrdup(reserve_ct_zone_request_list); > > + char *reserve_port; > > + char *save_ptr = NULL; > > + > > + for (reserve_port = strtok_r(dup_reserve, ",", &save_ptr); > > + reserve_port != NULL; > > + reserve_port = strtok_r(NULL, ",", &save_ptr)) { > > + sset_add(&all_users, reserve_port); > > + } > > + > > + free(dup_reserve); > > + } > > + } > > + > > /* 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/ovn-controller.8.xml > > b/controller/ovn-controller.8.xml > > index faefa77b9..b070b724c 100644 > > --- a/controller/ovn-controller.8.xml > > +++ b/controller/ovn-controller.8.xml > > @@ -626,7 +626,20 @@ > > <code>external_ids:ovn-installed-ts</code>. > > </p> > > </dd> > > - </dl> > > + > > + <dt> > > + <code>external-ids:reserve_ct_zones</code> in the > > <code>Bridge</code> > > + table > > + </dt> > > + > > + <dd> > > + <p> > > + This key represents list of ports which are supposed to come up > > on > > + the chassis, and hence need advance reservation of ct-zones. > > + It is comma seprated list of port names. > > + </p> > > + </dd> > > + </dl> > > > > <h1>OVN Southbound Database Usage</h1> > > > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > > index 74bff9035..4377ac6f2 100644 > > --- a/tests/ovn-controller.at > > +++ b/tests/ovn-controller.at > > @@ -2533,8 +2533,14 @@ check_ovsdb_zone() { > > test $ct_zone -eq $db_zone > > } > > > > +check_duplicates() { > > + output=$1 > > + AT_CHECK([printf "$output" | tr ' ' '\n' | sort | uniq -d | grep .], > > [1]) > > +} > > + > > 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 > > > > @@ -2560,17 +2566,18 @@ 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" > > - > > 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]) > > + > > +check_duplicates "$ct_zones" > > > > # Now purposely request an SNAT zone for lr0 that conflicts with a zone > > # currently assigned to a logical port > > @@ -2585,15 +2592,78 @@ 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" > > + > > +check_duplicates "$ct_zones" > > + > > +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" > > +check_duplicates "$ct_zones" > > + > > +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" > > +check_duplicates "$ct_zones" > > > > 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 > > > > > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > amusil@redhat.com > <https://red.ht/sig> > _______________________________________________ > 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 77eb16ac9..bd1b0623b 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -183,6 +183,31 @@ ct_zones_update(const struct sset *local_lports, sset_add(&all_users, local_lport); } + /* Add local_lport 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) { + const char *reserve_ct_zone_request_list = smap_get( + &cfg->external_ids, "reserve_ct_zones"); + + if (reserve_ct_zone_request_list) { + char *dup_reserve = xstrdup(reserve_ct_zone_request_list); + char *reserve_port; + char *save_ptr = NULL; + + for (reserve_port = strtok_r(dup_reserve, ",", &save_ptr); + reserve_port != NULL; + reserve_port = strtok_r(NULL, ",", &save_ptr)) { + sset_add(&all_users, reserve_port); + } + + free(dup_reserve); + } + } + /* 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/ovn-controller.8.xml b/controller/ovn-controller.8.xml index faefa77b9..b070b724c 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -626,7 +626,20 @@ <code>external_ids:ovn-installed-ts</code>. </p> </dd> - </dl> + + <dt> + <code>external-ids:reserve_ct_zones</code> in the <code>Bridge</code> + table + </dt> + + <dd> + <p> + This key represents list of ports which are supposed to come up on + the chassis, and hence need advance reservation of ct-zones. + It is comma seprated list of port names. + </p> + </dd> + </dl> <h1>OVN Southbound Database Usage</h1> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 74bff9035..4377ac6f2 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2533,8 +2533,14 @@ check_ovsdb_zone() { test $ct_zone -eq $db_zone } +check_duplicates() { + output=$1 + AT_CHECK([printf "$output" | tr ' ' '\n' | sort | uniq -d | grep .], [1]) +} + 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 @@ -2560,17 +2566,18 @@ 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" - 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]) + +check_duplicates "$ct_zones" # Now purposely request an SNAT zone for lr0 that conflicts with a zone # currently assigned to a logical port @@ -2585,15 +2592,78 @@ 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" + +check_duplicates "$ct_zones" + +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" +check_duplicates "$ct_zones" + +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" +check_duplicates "$ct_zones" 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> --- v5-->v6 Updated tests to check duplicates more frequntly with input as ct_zones. --- controller/ct-zone.c | 25 ++++++++++ controller/ovn-controller.8.xml | 15 +++++- tests/ovn-controller.at | 86 ++++++++++++++++++++++++++++++--- 3 files changed, 117 insertions(+), 9 deletions(-)