| Message ID | 3f261b2729b4059c7055a9b7617ab58faf1224ff.1725976220.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] controller: Do not remove snat-ct-zone requested by the CMS. | 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 | success | github build: passed |
Excellent, thanks Lorenzo! Acked-by: Mark Michelson <mmichels@redhat.com> On 9/10/24 09:51, Lorenzo Bianconi wrote: > Do not check if snat-ct-zones are compliant with min/max boundary since > they have been explicitly requested by the CMS and this will trigger an > unnecessary NXT_CT_FLUSH_ZONE of that zones on ovn-controller restart. > > Fixes: f2363f49f6a4 ("controller: Add the capability to specify a min/max value for ct_zone.") > Fixes: 493ef704a973 ("controller: Prepare structure around CT zone limiting.") > Reported-at: https://issues.redhat.com/browse/FDP-773 > Co-developed-by: Ales Musil <amusil@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/ct-zone.c | 14 ++++--- > tests/ovn-controller.at | 89 ++++++++++++++++++++++++++++++----------- > 2 files changed, 75 insertions(+), 28 deletions(-) > > diff --git a/controller/ct-zone.c b/controller/ct-zone.c > index 77eb16ac9..469a8fc54 100644 > --- a/controller/ct-zone.c > +++ b/controller/ct-zone.c > @@ -216,12 +216,15 @@ ct_zones_update(const struct sset *local_lports, > struct shash_node *node; > SHASH_FOR_EACH_SAFE (node, &ctx->current) { > struct ct_zone *ct_zone = node->data; > - if (!sset_contains(&all_users, node->name) || > - ct_zone->zone < min_ct_zone || ct_zone->zone > max_ct_zone) { > + if (!sset_contains(&all_users, node->name)) { > ct_zone_remove(ctx, node->name); > } else if (!simap_find(&req_snat_zones, node->name)) { > - bitmap_set1(unreq_snat_zones_map, ct_zone->zone); > - simap_put(&unreq_snat_zones, node->name, ct_zone->zone); > + if (ct_zone->zone < min_ct_zone || ct_zone->zone > max_ct_zone) { > + ct_zone_remove(ctx, node->name); > + } else { > + bitmap_set1(unreq_snat_zones_map, ct_zone->zone); > + simap_put(&unreq_snat_zones, node->name, ct_zone->zone); > + } > } > } > > @@ -249,10 +252,11 @@ ct_zones_update(const struct sset *local_lports, > > struct ct_zone *ct_zone = shash_find_data(&ctx->current, > snat_req_node->name); > + bool flush = !(ct_zone && ct_zone->zone == snat_req_node->data); > if (ct_zone && ct_zone->zone != snat_req_node->data) { > ct_zone_remove(ctx, snat_req_node->name); > } > - ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, true); > + ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, flush); > } > > /* xxx This is wasteful to assign a zone to each port--even if no > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index dcab5f2e9..a43c6cffb 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -3140,12 +3140,12 @@ ovn_start > > check_ct_zone_min() { > min_val=$1 > - OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{print $2}' | sort | head -n1) -ge ${min_val}]) > + OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{printf "%02d\n", $2}' | sort | head -n1) -ge ${min_val}]) > } > > check_ct_zone_max() { > max_val=$1 > - AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{print $2}' | sort | tail -n1) -le ${max_val}]) > + AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{printf "%02d\n", $2}' | sort | tail -n1) -le ${max_val}]) > } > > net_add n1 > @@ -3158,44 +3158,87 @@ check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone > > check ovs-vsctl add-port br-int lsp0 \ > -- set Interface lsp0 external-ids:iface-id=lsp0 > +check ovs-vsctl add-port br-int lsp1 \ > + -- set Interface lsp1 external-ids:iface-id=lsp1 > > check ovn-nbctl lr-add lr > +check ovn-nbctl ls-add ls0 > +check ovn-nbctl ls-add ls1 > > -check ovn-nbctl ls-add ls > -check ovn-nbctl lsp-add ls ls-lr > -check ovn-nbctl lsp-set-type ls-lr router > -check ovn-nbctl lsp-set-addresses ls-lr router > -check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:00:01 10.0.0.1 > +check ovn-nbctl set logical_router lr options:chassis=hv1 > + > +check ovn-nbctl lrp-add lr lr-ls0 00:00:00:00:ff:01 10.0.0.1/24 > +check ovn-nbctl lsp-add ls0 ls0-lr > +check ovn-nbctl lsp-set-type ls0-lr router > +check ovn-nbctl lsp-set-addresses ls0-lr 00:00:00:00:ff:01 > +check ovn-nbctl lsp-set-options ls0-lr router-port=lr-ls0 > > -check ovn-nbctl lsp-add ls lsp0 > +check ovn-nbctl lsp-add ls0 lsp0 > check ovn-nbctl lsp-set-addresses lsp0 "00:00:00:00:00:02 10.0.0.2" > > -check ovn-nbctl lrp-add lr lrp-gw 01:00:00:00:00:01 172.16.0.1 > -check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1 > +check ovn-nbctl lrp-add lr lr-ls1 00:00:00:00:ff:02 172.16.0.1/24 > +check ovn-nbctl lsp-add ls1 ls1-lr > +check ovn-nbctl lsp-set-type ls1-lr router > +check ovn-nbctl lsp-set-addresses ls1-lr 00:00:00:00:ff:02 > +check ovn-nbctl lsp-set-options ls1-lr router-port=lr-ls1 > + > +check ovn-nbctl lsp-add ls1 lsp1 > +check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:02 172.16.0.2" > > -# check regular boundaries > +# Check regular boundaries > check_ct_zone_min 1 > -check_ct_zone_max 10 > +check_ct_zone_max 12 > > # increase boundaries > -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-20\" > +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-30\" > check_ct_zone_min 10 > -check_ct_zone_max 20 > +check_ct_zone_max 22 > > -# reset min boundary > -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-20\" > +# Reset min boundary > +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\" > > -# add a new port to the switch > -check ovs-vsctl add-port br-int lsp1 \ > - -- set Interface lsp1 external-ids:iface-id=lsp1 > -check ovn-nbctl lsp-add ls lsp1 > -check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:03 10.0.0.3" > +# Add a new port to the ls0 switch > +check ovs-vsctl add-port br-int lsp2 \ > + -- set Interface lsp2 external-ids:iface-id=lsp2 > +check ovn-nbctl lsp-add ls0 lsp2 > +check ovn-nbctl lsp-set-addresses lsp2 "00:00:00:00:00:03 10.0.0.3" > check_ct_zone_min 5 > -check_ct_zone_max 20 > +check_ct_zone_max 22 > > check ovn-nbctl set logical_router lr options:snat-ct-zone=2 > check_ct_zone_min 2 > -check_ct_zone_max 20 > +check_ct_zone_max 22 > +# Check lr-snat zone value > +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '/lr_snat/{print $2}') -eq 2]) > + > +check ovs-appctl vlog/disable-rate-limit > +check ovs-appctl vlog/set vconn:DBG > + > +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log) > +check ovn-appctl -t ovn-controller exit --restart > +# Make sure ovn-controller stopped before restarting it > +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" != "running"]) > +start_daemon ovn-controller --verbose="ct_zone:dbg" > +wait_for_ports_up > + > +# Check we do not have unexpected ct-flush restarting ovn-controller > +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) > + > +# snat-ct-zone 0 is allowed > +check ovn-nbctl set logical_router lr options:snat-ct-zone=0 > +check_ct_zone_min 0 > +check_ct_zone_max 22 > +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '/lr_snat/{print $2}') -eq 0]) > + > +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log) > +check ovn-appctl -t ovn-controller exit --restart > +# Make sure ovn-controller stopped before restarting it > +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" != "running"]) > +start_daemon ovn-controller --verbose="ct_zone:dbg" > +wait_for_ports_up > + > +# Check we do not have unexpected ct-flush restarting ovn-controller > +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) > > OVN_CLEANUP([hv1]) > AT_CLEANUP
On 9/10/24 19:25, Mark Michelson wrote: > Excellent, thanks Lorenzo! > > Acked-by: Mark Michelson <mmichels@redhat.com> > Thanks, Lorenzo, Ales and Mark! I applied this to main and 24.09 with a small change to the test (I think the original version had a chance of being flaky): diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index a43c6cffb7..a2e451880c 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -3145,13 +3145,16 @@ check_ct_zone_min() { check_ct_zone_max() { max_val=$1 - AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{printf "%02d\n", $2}' | sort | tail -n1) -le ${max_val}]) + OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{printf "%02d\n", $2}' | sort | tail -n1) -le ${max_val}]) } net_add n1 sim_add hv1 as hv1 check ovs-vsctl add-br br-phys +check ovs-appctl vlog/disable-rate-limit +check ovs-appctl vlog/set vconn:DBG + ovn_attach n1 br-phys 192.168.0.1 check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone @@ -3184,18 +3187,21 @@ check ovn-nbctl lsp-set-options ls1-lr router-port=lr-ls1 check ovn-nbctl lsp-add ls1 lsp1 check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:02 172.16.0.2" +wait_for_ports_up +check ovn-nbctl --wait=hv sync -# Check regular boundaries +AS_BOX([Check regular boundaries]) check_ct_zone_min 1 check_ct_zone_max 12 -# increase boundaries +AS_BOX([Increase boundaries]) ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-30\" + check_ct_zone_min 10 check_ct_zone_max 22 -# Reset min boundary -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\" +AS_BOX([Reset min boundary]) +check ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\" # Add a new port to the ls0 switch check ovs-vsctl add-port br-int lsp2 \ @@ -3206,13 +3212,14 @@ check_ct_zone_min 5 check_ct_zone_max 22 check ovn-nbctl set logical_router lr options:snat-ct-zone=2 +wait_for_ports_up +check ovn-nbctl --wait=hv sync + check_ct_zone_min 2 check_ct_zone_max 22 -# Check lr-snat zone value -AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '/lr_snat/{print $2}') -eq 2]) -check ovs-appctl vlog/disable-rate-limit -check ovs-appctl vlog/set vconn:DBG +AS_BOX([Check LR snat requested zone 2]) +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '/lr_snat/{print $2}') -eq 2]) n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log) check ovn-appctl -t ovn-controller exit --restart @@ -3220,12 +3227,15 @@ check ovn-appctl -t ovn-controller exit --restart OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" != "running"]) start_daemon ovn-controller --verbose="ct_zone:dbg" wait_for_ports_up +check ovn-nbctl --wait=hv sync # Check we do not have unexpected ct-flush restarting ovn-controller AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) -# snat-ct-zone 0 is allowed +AS_BOX([Check LR snat allowed requested zone 0]) check ovn-nbctl set logical_router lr options:snat-ct-zone=0 +check ovn-nbctl --wait=hv sync + check_ct_zone_min 0 check_ct_zone_max 22 AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '/lr_snat/{print $2}') -eq 0]) @@ -3236,6 +3246,7 @@ check ovn-appctl -t ovn-controller exit --restart OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" != "running"]) start_daemon ovn-controller --verbose="ct_zone:dbg" wait_for_ports_up +check ovn-nbctl --wait=hv sync # Check we do not have unexpected ct-flush restarting ovn-controller AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) --- Regards, Dumitru > On 9/10/24 09:51, Lorenzo Bianconi wrote: >> Do not check if snat-ct-zones are compliant with min/max boundary since >> they have been explicitly requested by the CMS and this will trigger an >> unnecessary NXT_CT_FLUSH_ZONE of that zones on ovn-controller restart. >> >> Fixes: f2363f49f6a4 ("controller: Add the capability to specify a >> min/max value for ct_zone.") >> Fixes: 493ef704a973 ("controller: Prepare structure around CT zone >> limiting.") >> Reported-at: https://issues.redhat.com/browse/FDP-773 >> Co-developed-by: Ales Musil <amusil@redhat.com> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> --- >> controller/ct-zone.c | 14 ++++--- >> tests/ovn-controller.at | 89 ++++++++++++++++++++++++++++++----------- >> 2 files changed, 75 insertions(+), 28 deletions(-) >> >> diff --git a/controller/ct-zone.c b/controller/ct-zone.c >> index 77eb16ac9..469a8fc54 100644 >> --- a/controller/ct-zone.c >> +++ b/controller/ct-zone.c >> @@ -216,12 +216,15 @@ ct_zones_update(const struct sset *local_lports, >> struct shash_node *node; >> SHASH_FOR_EACH_SAFE (node, &ctx->current) { >> struct ct_zone *ct_zone = node->data; >> - if (!sset_contains(&all_users, node->name) || >> - ct_zone->zone < min_ct_zone || ct_zone->zone > >> max_ct_zone) { >> + if (!sset_contains(&all_users, node->name)) { >> ct_zone_remove(ctx, node->name); >> } else if (!simap_find(&req_snat_zones, node->name)) { >> - bitmap_set1(unreq_snat_zones_map, ct_zone->zone); >> - simap_put(&unreq_snat_zones, node->name, ct_zone->zone); >> + if (ct_zone->zone < min_ct_zone || ct_zone->zone > >> max_ct_zone) { >> + ct_zone_remove(ctx, node->name); >> + } else { >> + bitmap_set1(unreq_snat_zones_map, ct_zone->zone); >> + simap_put(&unreq_snat_zones, node->name, ct_zone->zone); >> + } >> } >> } >> @@ -249,10 +252,11 @@ ct_zones_update(const struct sset *local_lports, >> struct ct_zone *ct_zone = shash_find_data(&ctx->current, >> snat_req_node->name); >> + bool flush = !(ct_zone && ct_zone->zone == snat_req_node->data); >> if (ct_zone && ct_zone->zone != snat_req_node->data) { >> ct_zone_remove(ctx, snat_req_node->name); >> } >> - ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, >> true); >> + ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, >> flush); >> } >> /* xxx This is wasteful to assign a zone to each port--even if no >> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at >> index dcab5f2e9..a43c6cffb 100644 >> --- a/tests/ovn-controller.at >> +++ b/tests/ovn-controller.at >> @@ -3140,12 +3140,12 @@ ovn_start >> check_ct_zone_min() { >> min_val=$1 >> - OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list >> | awk '{print $2}' | sort | head -n1) -ge ${min_val}]) >> + OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list >> | awk '{printf "%02d\n", $2}' | sort | head -n1) -ge ${min_val}]) >> } >> check_ct_zone_max() { >> max_val=$1 >> - AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk >> '{print $2}' | sort | tail -n1) -le ${max_val}]) >> + AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk >> '{printf "%02d\n", $2}' | sort | tail -n1) -le ${max_val}]) >> } >> net_add n1 >> @@ -3158,44 +3158,87 @@ check ovn-appctl -t ovn-controller vlog/set >> dbg:ct_zone >> check ovs-vsctl add-port br-int lsp0 \ >> -- set Interface lsp0 external-ids:iface-id=lsp0 >> +check ovs-vsctl add-port br-int lsp1 \ >> + -- set Interface lsp1 external-ids:iface-id=lsp1 >> check ovn-nbctl lr-add lr >> +check ovn-nbctl ls-add ls0 >> +check ovn-nbctl ls-add ls1 >> -check ovn-nbctl ls-add ls >> -check ovn-nbctl lsp-add ls ls-lr >> -check ovn-nbctl lsp-set-type ls-lr router >> -check ovn-nbctl lsp-set-addresses ls-lr router >> -check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:00:01 10.0.0.1 >> +check ovn-nbctl set logical_router lr options:chassis=hv1 >> + >> +check ovn-nbctl lrp-add lr lr-ls0 00:00:00:00:ff:01 10.0.0.1/24 >> +check ovn-nbctl lsp-add ls0 ls0-lr >> +check ovn-nbctl lsp-set-type ls0-lr router >> +check ovn-nbctl lsp-set-addresses ls0-lr 00:00:00:00:ff:01 >> +check ovn-nbctl lsp-set-options ls0-lr router-port=lr-ls0 >> -check ovn-nbctl lsp-add ls lsp0 >> +check ovn-nbctl lsp-add ls0 lsp0 >> check ovn-nbctl lsp-set-addresses lsp0 "00:00:00:00:00:02 10.0.0.2" >> -check ovn-nbctl lrp-add lr lrp-gw 01:00:00:00:00:01 172.16.0.1 >> -check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1 >> +check ovn-nbctl lrp-add lr lr-ls1 00:00:00:00:ff:02 172.16.0.1/24 >> +check ovn-nbctl lsp-add ls1 ls1-lr >> +check ovn-nbctl lsp-set-type ls1-lr router >> +check ovn-nbctl lsp-set-addresses ls1-lr 00:00:00:00:ff:02 >> +check ovn-nbctl lsp-set-options ls1-lr router-port=lr-ls1 >> + >> +check ovn-nbctl lsp-add ls1 lsp1 >> +check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:02 172.16.0.2" >> -# check regular boundaries >> +# Check regular boundaries >> check_ct_zone_min 1 >> -check_ct_zone_max 10 >> +check_ct_zone_max 12 >> # increase boundaries >> -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-20\" >> +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-30\" >> check_ct_zone_min 10 >> -check_ct_zone_max 20 >> +check_ct_zone_max 22 >> -# reset min boundary >> -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-20\" >> +# Reset min boundary >> +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\" >> -# add a new port to the switch >> -check ovs-vsctl add-port br-int lsp1 \ >> - -- set Interface lsp1 external-ids:iface-id=lsp1 >> -check ovn-nbctl lsp-add ls lsp1 >> -check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:03 10.0.0.3" >> +# Add a new port to the ls0 switch >> +check ovs-vsctl add-port br-int lsp2 \ >> + -- set Interface lsp2 external-ids:iface-id=lsp2 >> +check ovn-nbctl lsp-add ls0 lsp2 >> +check ovn-nbctl lsp-set-addresses lsp2 "00:00:00:00:00:03 10.0.0.3" >> check_ct_zone_min 5 >> -check_ct_zone_max 20 >> +check_ct_zone_max 22 >> check ovn-nbctl set logical_router lr options:snat-ct-zone=2 >> check_ct_zone_min 2 >> -check_ct_zone_max 20 >> +check_ct_zone_max 22 >> +# Check lr-snat zone value >> +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk >> '/lr_snat/{print $2}') -eq 2]) >> + >> +check ovs-appctl vlog/disable-rate-limit >> +check ovs-appctl vlog/set vconn:DBG >> + >> +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log) >> +check ovn-appctl -t ovn-controller exit --restart >> +# Make sure ovn-controller stopped before restarting it >> +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" >> != "running"]) >> +start_daemon ovn-controller --verbose="ct_zone:dbg" >> +wait_for_ports_up >> + >> +# Check we do not have unexpected ct-flush restarting ovn-controller >> +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) >> + >> +# snat-ct-zone 0 is allowed >> +check ovn-nbctl set logical_router lr options:snat-ct-zone=0 >> +check_ct_zone_min 0 >> +check_ct_zone_max 22 >> +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk >> '/lr_snat/{print $2}') -eq 0]) >> + >> +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log) >> +check ovn-appctl -t ovn-controller exit --restart >> +# Make sure ovn-controller stopped before restarting it >> +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" >> != "running"]) >> +start_daemon ovn-controller --verbose="ct_zone:dbg" >> +wait_for_ports_up >> + >> +# Check we do not have unexpected ct-flush restarting ovn-controller >> +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) >> OVN_CLEANUP([hv1]) >> AT_CLEANUP >
diff --git a/controller/ct-zone.c b/controller/ct-zone.c index 77eb16ac9..469a8fc54 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -216,12 +216,15 @@ ct_zones_update(const struct sset *local_lports, struct shash_node *node; SHASH_FOR_EACH_SAFE (node, &ctx->current) { struct ct_zone *ct_zone = node->data; - if (!sset_contains(&all_users, node->name) || - ct_zone->zone < min_ct_zone || ct_zone->zone > max_ct_zone) { + if (!sset_contains(&all_users, node->name)) { ct_zone_remove(ctx, node->name); } else if (!simap_find(&req_snat_zones, node->name)) { - bitmap_set1(unreq_snat_zones_map, ct_zone->zone); - simap_put(&unreq_snat_zones, node->name, ct_zone->zone); + if (ct_zone->zone < min_ct_zone || ct_zone->zone > max_ct_zone) { + ct_zone_remove(ctx, node->name); + } else { + bitmap_set1(unreq_snat_zones_map, ct_zone->zone); + simap_put(&unreq_snat_zones, node->name, ct_zone->zone); + } } } @@ -249,10 +252,11 @@ ct_zones_update(const struct sset *local_lports, struct ct_zone *ct_zone = shash_find_data(&ctx->current, snat_req_node->name); + bool flush = !(ct_zone && ct_zone->zone == snat_req_node->data); if (ct_zone && ct_zone->zone != snat_req_node->data) { ct_zone_remove(ctx, snat_req_node->name); } - ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, true); + ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, flush); } /* xxx This is wasteful to assign a zone to each port--even if no diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index dcab5f2e9..a43c6cffb 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -3140,12 +3140,12 @@ ovn_start check_ct_zone_min() { min_val=$1 - OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{print $2}' | sort | head -n1) -ge ${min_val}]) + OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{printf "%02d\n", $2}' | sort | head -n1) -ge ${min_val}]) } check_ct_zone_max() { max_val=$1 - AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{print $2}' | sort | tail -n1) -le ${max_val}]) + AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{printf "%02d\n", $2}' | sort | tail -n1) -le ${max_val}]) } net_add n1 @@ -3158,44 +3158,87 @@ check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone check ovs-vsctl add-port br-int lsp0 \ -- set Interface lsp0 external-ids:iface-id=lsp0 +check ovs-vsctl add-port br-int lsp1 \ + -- set Interface lsp1 external-ids:iface-id=lsp1 check ovn-nbctl lr-add lr +check ovn-nbctl ls-add ls0 +check ovn-nbctl ls-add ls1 -check ovn-nbctl ls-add ls -check ovn-nbctl lsp-add ls ls-lr -check ovn-nbctl lsp-set-type ls-lr router -check ovn-nbctl lsp-set-addresses ls-lr router -check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:00:01 10.0.0.1 +check ovn-nbctl set logical_router lr options:chassis=hv1 + +check ovn-nbctl lrp-add lr lr-ls0 00:00:00:00:ff:01 10.0.0.1/24 +check ovn-nbctl lsp-add ls0 ls0-lr +check ovn-nbctl lsp-set-type ls0-lr router +check ovn-nbctl lsp-set-addresses ls0-lr 00:00:00:00:ff:01 +check ovn-nbctl lsp-set-options ls0-lr router-port=lr-ls0 -check ovn-nbctl lsp-add ls lsp0 +check ovn-nbctl lsp-add ls0 lsp0 check ovn-nbctl lsp-set-addresses lsp0 "00:00:00:00:00:02 10.0.0.2" -check ovn-nbctl lrp-add lr lrp-gw 01:00:00:00:00:01 172.16.0.1 -check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1 +check ovn-nbctl lrp-add lr lr-ls1 00:00:00:00:ff:02 172.16.0.1/24 +check ovn-nbctl lsp-add ls1 ls1-lr +check ovn-nbctl lsp-set-type ls1-lr router +check ovn-nbctl lsp-set-addresses ls1-lr 00:00:00:00:ff:02 +check ovn-nbctl lsp-set-options ls1-lr router-port=lr-ls1 + +check ovn-nbctl lsp-add ls1 lsp1 +check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:02 172.16.0.2" -# check regular boundaries +# Check regular boundaries check_ct_zone_min 1 -check_ct_zone_max 10 +check_ct_zone_max 12 # increase boundaries -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-20\" +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-30\" check_ct_zone_min 10 -check_ct_zone_max 20 +check_ct_zone_max 22 -# reset min boundary -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-20\" +# Reset min boundary +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\" -# add a new port to the switch -check ovs-vsctl add-port br-int lsp1 \ - -- set Interface lsp1 external-ids:iface-id=lsp1 -check ovn-nbctl lsp-add ls lsp1 -check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:03 10.0.0.3" +# Add a new port to the ls0 switch +check ovs-vsctl add-port br-int lsp2 \ + -- set Interface lsp2 external-ids:iface-id=lsp2 +check ovn-nbctl lsp-add ls0 lsp2 +check ovn-nbctl lsp-set-addresses lsp2 "00:00:00:00:00:03 10.0.0.3" check_ct_zone_min 5 -check_ct_zone_max 20 +check_ct_zone_max 22 check ovn-nbctl set logical_router lr options:snat-ct-zone=2 check_ct_zone_min 2 -check_ct_zone_max 20 +check_ct_zone_max 22 +# Check lr-snat zone value +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '/lr_snat/{print $2}') -eq 2]) + +check ovs-appctl vlog/disable-rate-limit +check ovs-appctl vlog/set vconn:DBG + +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log) +check ovn-appctl -t ovn-controller exit --restart +# Make sure ovn-controller stopped before restarting it +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" != "running"]) +start_daemon ovn-controller --verbose="ct_zone:dbg" +wait_for_ports_up + +# Check we do not have unexpected ct-flush restarting ovn-controller +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) + +# snat-ct-zone 0 is allowed +check ovn-nbctl set logical_router lr options:snat-ct-zone=0 +check_ct_zone_min 0 +check_ct_zone_max 22 +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '/lr_snat/{print $2}') -eq 0]) + +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log) +check ovn-appctl -t ovn-controller exit --restart +# Make sure ovn-controller stopped before restarting it +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" != "running"]) +start_daemon ovn-controller --verbose="ct_zone:dbg" +wait_for_ports_up + +# Check we do not have unexpected ct-flush restarting ovn-controller +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}]) OVN_CLEANUP([hv1]) AT_CLEANUP
Do not check if snat-ct-zones are compliant with min/max boundary since they have been explicitly requested by the CMS and this will trigger an unnecessary NXT_CT_FLUSH_ZONE of that zones on ovn-controller restart. Fixes: f2363f49f6a4 ("controller: Add the capability to specify a min/max value for ct_zone.") Fixes: 493ef704a973 ("controller: Prepare structure around CT zone limiting.") Reported-at: https://issues.redhat.com/browse/FDP-773 Co-developed-by: Ales Musil <amusil@redhat.com> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- controller/ct-zone.c | 14 ++++--- tests/ovn-controller.at | 89 ++++++++++++++++++++++++++++++----------- 2 files changed, 75 insertions(+), 28 deletions(-)