On Wed, Aug 14, 2024 at 7:39 AM Mansi Sharma <[email protected]> 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 <[email protected]> > Hi, sorry for the delay, I have a couple of small comments down below. --- > V1-->V2 : > 1. Rebased from main. > 2. Updated ovn-controller.xml documentation. > --- > controller/ct-zone.c | 22 ++++++++ > controller/ovn-controller.8.xml | 15 +++++- > tests/ovn-controller.at | 92 ++++++++++++++++++++++++++++++--- > 3 files changed, 121 insertions(+), 8 deletions(-) > > diff --git a/controller/ct-zone.c b/controller/ct-zone.c > index 77eb16ac9..9bcb78c7e 100644 > --- a/controller/ct-zone.c > +++ b/controller/ct-zone.c > @@ -183,6 +183,28 @@ 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 > typo: local_lport > + * 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_id, "reserve_ct_zones"); > + if (reserve_ct_zone_request_list) { > It can be much more readable by renaming a couple of variables down below. See for example extract_sbrec_binding_first_mac() how do we format the for loop with strtok_r(). > + 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/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..7910939aa 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -2535,6 +2535,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 > > @@ -2560,17 +2561,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 > @@ -2585,15 +2594,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 > The check loop is repeated twice, it would be better to make it a function. > + > +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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
