controller: Do not remove snat-ct-zone requested by the CMS.

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: f2363f4 ("controller: Add the capability to specify a min/max value for ct_zone.") Fixes: 493ef70 ("controller: Prepare structure around CT zone limiting.") Reported-at: https://issues.redhat.com/browse/FDP-773 Co-developed-by: Ales Musil <[email protected]> Signed-off-by: Lorenzo Bianconi <[email protected]> Acked-by: Mark Michelson <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]>
ovsrobot · Sep 11, 2024 · 8d652c5 · 8d652c5
1 parent 8770604
commit 8d652c5
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 29 deletions.
diff --git 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
@@ -3140,62 +3140,116 @@ 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}])
+    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
 
 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 regular boundaries
+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
+
+AS_BOX([Check regular boundaries])
 check_ct_zone_min 1
-check_ct_zone_max 10
+check_ct_zone_max 12
+
+AS_BOX([Increase boundaries])
+ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-30\"
 
-# increase boundaries
-ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-20\"
 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\"
+AS_BOX([Reset min boundary])
+check 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
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
 check_ct_zone_min 2
-check_ct_zone_max 20
+check_ct_zone_max 22
+
+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
+# 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 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}])
+
+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])
+
+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 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}])
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP