controller: Update physical flows for peer port when the patch port i…

…s removed. Similar to commit 1431276 ("controller: fix ovn patch port incremental processing"), update peer logical flows when the related patch port is removed. Reported-at: https://issues.redhat.com/browse/FDP-947 Signed-off-by: Lorenzo Bianconi <[email protected]> Signed-off-by: Numan Siddique <[email protected]>
ovn-org · Jan 17, 2025 · 73a1034 · 73a1034
1 parent a5d6329
commit 73a1034
Show file tree

Hide file tree

Showing 4 changed files with 119 additions and 6 deletions.
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
@@ -4567,6 +4567,11 @@ pflow_output_sb_port_binding_handler(struct engine_node *node,
      */
     const struct sbrec_port_binding *pb;
     SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) {
+        /* Trigger a full recompute if type column is updated. */
+        if (sbrec_port_binding_is_updated(pb, SBREC_PORT_BINDING_COL_TYPE)) {
+            destroy_physical_ctx(&p_ctx);
+            return false;
+        }
         bool removed = sbrec_port_binding_is_deleted(pb);
         if (!physical_handle_flows_for_lport(pb, removed, &p_ctx,
                                              &pfo->flow_table)) {

diff --git a/controller/physical.c b/controller/physical.c
@@ -2412,13 +2412,21 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
         physical_multichassis_reprocess(pb, p_ctx, flow_table);
     }
 
-    if (!removed) {
+    /* Always update pb and the configured peer for patch ports. */
+    if (!removed || !strcmp(pb->type, "patch")) {
         physical_eval_port_binding(p_ctx, pb, flow_table);
-        if (!strcmp(pb->type, "patch")) {
-            const struct sbrec_port_binding *peer =
-                get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb);
-            if (peer) {
-                physical_eval_port_binding(p_ctx, peer, flow_table);
+    }
+
+    if (!strcmp(pb->type, "patch")) {
+        if (removed) {
+            ofctrl_remove_flows(flow_table, &pb->header_.uuid);
+        }
+        const struct sbrec_port_binding *peer =
+            get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb);
+        if (peer) {
+            physical_eval_port_binding(p_ctx, peer, flow_table);
+            if (removed) {
+                ofctrl_remove_flows(flow_table, &peer->header_.uuid);
             }
         }
     }

diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
@@ -1,5 +1,53 @@
 AT_BANNER([ovn-controller])
 
+m4_divert_push([PREPARE_TESTS])
+# Checks if the provided engine node recomputed or not
+# and if it computed or not.
+# 1st argument is the simulated hypervisor name.
+# 2nd argument is the engine node.
+# 3rd argument is the expected recompute state.
+#   Possible values are - "norecompute" and "recompute".
+# 4th argument is the expected compute state.
+#   Possible values are - "nocompute" and "compute".
+#
+# Eg. 'check_controller_engine_stats hv1 logical_flow_output recompute nocompute'
+# will verify that if the lflow engine node recompute stat is > 0 and
+# compute stat is equal to 0.  It fails otherwise.
+check_controller_engine_stats() {
+  hv=$1
+  node=$2
+  recompute=$3
+  compute=$4
+
+  echo "Checking engine stats for node $node : recompute - \
+$recompute : compute - $compute"
+
+  node_stat=$(as $hv ovn-appctl -t ovn-controller inc-engine/show-stats $node)
+  # node_stat will be of this format :
+  #     - Node: logical_flow_output - recompute: 3 - compute: 0 - cancel: 0
+  node_recompute_ct=$(echo $node_stat | cut -d '-' -f2 | cut -d ':' -f2)
+  node_compute_ct=$(echo $node_stat | cut -d '-' -f3 | cut -d ':' -f2)
+
+  if [[ "$recompute" == "norecompute" ]]; then
+    # node should not be recomputed
+    echo "Expecting $node recompute count - $node_recompute_ct to be 0"
+    check test "$node_recompute_ct" -eq "0"
+  else
+    echo "Expecting $node recompute count - $node_recompute_ct not to be 0"
+    check test "$node_recompute_ct" -ne "0"
+  fi
+
+  if [[ "$compute" == "nocompute" ]]; then
+    # node should not be computed
+    echo "Expecting $node compute count - $node_compute_ct to be 0"
+    check test "$node_compute_ct" -eq "0"
+  else
+    echo "Expecting $node compute count - $node_compute_ct not to be 0"
+    check test "$node_compute_ct" -ne "0"
+  fi
+}
+m4_divert_pop([PREPARE_TESTS])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ovn-controller - ovn-bridge-mappings])
 AT_KEYWORDS([ovn])
@@ -917,6 +965,11 @@ check ovn-nbctl lrp-add lr0 rp-ls1 00:00:01:01:02:03 192.168.1.254/24
 
 OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int | grep table=OFTABLE_LOCAL_OUTPUT | grep -q "reg15=0x${port},metadata=0x${meta}"])
 
+# Check we have a full recompute if type column is updated
+check as hv1 ovn-appctl -t ovn-controller inc-engine/clear-stats
+check ovn-nbctl --wait=hv lsp-set-type ls0-rp localnet
+check_controller_engine_stats hv1 physical_flow_output recompute nocompute
+
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])

diff --git a/tests/ovn.at b/tests/ovn.at
@@ -40754,3 +40754,50 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v
 OVN_CLEANUP([hv1],[hv2])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([requested-tnl-key-recompute])
+AT_KEYWORDS([requested-tnl-key-recompute])
+
+m4_define([OFTABLE_LOG_TO_PHY], [65])
+
+ovn_start
+net_add n1
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls lsp -- lsp-set-addresses lsp "00:00:10:01:02:01 10.0.0.1"
+
+check ovn-nbctl lr-add lr
+check ovn-nbctl set logical_router lr options:mac_binding_age_threshold=3600
+check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:ff:01 10.0.0.254/24
+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 lsp-set-options ls-lr router-port=lr-ls
+
+sim_add hv1
+
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys
+ovs-vsctl add-port br-int vif -- set Interface vif external-ids:iface-id=lsp
+
+check ovn-nbctl --wait=hv sync
+wait_for_ports_up
+
+check ovn-nbctl --wait=hv set logical_router_port lr-ls options:requested-tnl-key=42
+ls_tnl_key=$(fetch_column datapath_binding tunnel_key external_ids:name=ls)
+AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_LOG_TO_PHY,metadata=0x${ls_tnl_key} | grep -q load:0x2a->NXM_NX_REG14])
+
+check ovn-nbctl lrp-del lr-ls
+check ovn-nbctl                                             \
+    -- lrp-add lr lr-ls 00:00:00:00:10:00 192.168.10.1/24   \
+    -- set logical_router_port lr-ls options:requested-tnl-key=43
+check ovn-nbctl --wait=hv sync
+
+AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_LOG_TO_PHY,metadata=0x${ls_tnl_key} | grep -q load:0x2b->NXM_NX_REG14])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])