If a patch port that connects a logical router to a logical switch is
added in a different ovn-controller run with respect to the logical
switch peer port, just the router port will be processed and
ovn-controller will miss missing some open-flows for the ls patch port
since the peer pointer was NULL when it has been processed.
Fix the issue always processing both patch ports.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2025623
Tested-by: Slawek Kaplonski <[email protected]>
Acked-by: Mark Michelson <[email protected]>
Fixes: 3ae8470edc64 ("I-P: Handle runtime data changes for pflow_output 
engine.")
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Changes since v1:
- add unit-test
---
 controller/physical.c   | 49 +++++++++++++++++++++++------------------
 tests/ovn-controller.at | 48 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 22 deletions(-)

diff --git a/controller/physical.c b/controller/physical.c
index aa1d44bc6..836fc769a 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1564,6 +1564,24 @@ consider_mc_group(struct ovsdb_idl_index 
*sbrec_port_binding_by_name,
     sset_destroy(&vtep_chassis);
 }
 
+static void
+physical_eval_port_binding(struct physical_ctx *p_ctx,
+                           const struct sbrec_port_binding *pb,
+                           struct ovn_desired_flow_table *flow_table)
+{
+    struct ofpbuf ofpacts;
+    ofpbuf_init(&ofpacts, 0);
+    consider_port_binding(p_ctx->sbrec_port_binding_by_name,
+                          p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
+                          p_ctx->active_tunnels,
+                          p_ctx->local_datapaths,
+                          p_ctx->local_bindings,
+                          p_ctx->patch_ofports,
+                          p_ctx->chassis_tunnels,
+                          pb, p_ctx->chassis, flow_table, &ofpacts);
+    ofpbuf_uninit(&ofpacts);
+}
+
 bool
 physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
                                 bool removed, struct physical_ctx *p_ctx,
@@ -1585,33 +1603,20 @@ physical_handle_flows_for_lport(const struct 
sbrec_port_binding *pb,
             get_local_datapath(p_ctx->local_datapaths,
                                pb->datapath->tunnel_key);
         if (ldp && ldp->localnet_port) {
-            struct ofpbuf ofpacts;
             ofctrl_remove_flows(flow_table, &ldp->localnet_port->header_.uuid);
-            ofpbuf_init(&ofpacts, 0);
-            consider_port_binding(p_ctx->sbrec_port_binding_by_name,
-                                  p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
-                                  p_ctx->active_tunnels,
-                                  p_ctx->local_datapaths,
-                                  p_ctx->local_bindings,
-                                  p_ctx->patch_ofports,
-                                  p_ctx->chassis_tunnels,
-                                  ldp->localnet_port, p_ctx->chassis,
-                                  flow_table, &ofpacts);
-            ofpbuf_uninit(&ofpacts);
+            physical_eval_port_binding(p_ctx, ldp->localnet_port, flow_table);
         }
     }
 
     if (!removed) {
-        struct ofpbuf ofpacts;
-        ofpbuf_init(&ofpacts, 0);
-        consider_port_binding(p_ctx->sbrec_port_binding_by_name,
-                              p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
-                              p_ctx->active_tunnels, p_ctx->local_datapaths,
-                              p_ctx->local_bindings,
-                              p_ctx->patch_ofports,
-                              p_ctx->chassis_tunnels, pb,
-                              p_ctx->chassis, flow_table, &ofpacts);
-        ofpbuf_uninit(&ofpacts);
+        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);
+            }
+        }
     }
 
     return true;
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 7c683cbcc..ea632034b 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -806,3 +806,51 @@ OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller - ovn IP check path ports])
+AT_KEYWORDS([ovn-ip-patch-ports])
+
+ovn_start
+
+net_add n1
+sim_add hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+check ovn-nbctl lr-add lr0 \
+    -- lrp-add lr0 rp-ls0 00:00:01:01:02:02 192.168.0.254/24 \
+    -- ls-add ls0 \
+    -- lsp-add ls0 ls0-rp \
+    -- lsp-set-type ls0-rp router \
+    -- lsp-set-addresses ls0-rp router \
+    -- lsp-set-options ls0-rp router-port=rp-ls0 \
+    -- lsp-add ls0 lsp0 \
+    -- lsp-set-addresses lsp0 "00:11:00:00:00:01 192.168.0.1" \
+    -- ls-add ls1 \
+    -- lsp-add ls1 ls1-rp \
+    -- lsp-set-type ls1-rp router \
+    -- lsp-set-addresses ls1-rp router \
+    -- lsp-set-options ls1-rp router-port=rp-ls1 \
+    -- lsp-add ls1 lsp1 \
+    -- lsp-set-addresses lsp1 "00:00:00:00:00:01 192.168.1.1"
+
+as hv1
+check ovs-vsctl \
+    -- add-port br-int vif0 \
+    -- set Interface vif0 external_ids:iface-id=lsp0 \
+    -- add-port br-int vif1 \
+    -- set Interface vif1 external_ids:iface-id=lsp1
+
+OVS_WAIT_UNTIL([grep -q 'Setting lport lsp0 up in Southbound' 
hv1/ovn-controller.log])
+OVS_WAIT_UNTIL([grep -q 'Setting lport lsp1 up in Southbound' 
hv1/ovn-controller.log])
+
+meta=$(ovn-sbctl get datapath ls1 tunnel_key)
+port=$(ovn-sbctl get port_binding ls1-rp tunnel_key)
+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=38 | grep -q 
"reg15=0x${port},metadata=0x${meta}"])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
-- 
2.33.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to