Thanks for the fix Xavier.

Acked-by: Mark Michelson <[email protected]>

On 10/31/24 13:10, Xavier Simonart wrote:
This reverts commit edc064b4c589ab1bb69352523481bd6d997aa1ca.

In setups with multiple localnet ports w/o vlans (or multiple localnet
ports with the same vlans), that patch was trying to create the same
flows with action conjunction multiple times, which caused the following
assert:
0  0x00007ffff77cd834 in __pthread_kill_implementation () from /lib64/libc.so.6
1  0x00007ffff777b8ee in raise () from /lib64/libc.so.6
2  0x00007ffff77638ff in abort () from /lib64/libc.so.6
3  0x000000000042f861 in flow_is_preferred (a=0xafe9a0, b=0xaf9380) at 
controller/ofctrl.c:966
4  0x000000000042f340 in link_installed_to_desired (i=0xb2eaf0, d=0xafe9a0) at 
controller/ofctrl.c:987
5  0x000000000042c17c in update_installed_flows_by_track (flow_table=0x813a80, 
bc=0x7ffffffcc740, installed_flows=0x7894e0 <installed_pflows>, 
msgs=0x7ffffffcc790) at controller/ofctrl.c:2583
6  0x000000000042af14 in ofctrl_put (lflow_table=0x810180, 
pflow_table=0x813a80, pending_ct_zones=0x8129b0, pending_lb_tuples=0x80e030, 
sbrec_meter_by_name=0x7e6840, req_cfg=0, lflows_changed=true, 
pflows_changed=true) at controller/ofctrl.c:2826
7  0x000000000045015c in main (argc=1, argv=0x7fffffffe218) at 
controller/ovn-controller.c:5788

Reverting that patch means that flows such as
   table_id=0, priority=180, vlan_tci=0x0000/0x1000, 
actions=conjunction(100,2/2)
remains after the localnet port is deleted.

Fixes: edc064b4c589 ("controller: Properly handle localnet flows in I+P.")
Reported-at: https://issues.redhat.com/browse/FDP-926

Signed-off-by: Xavier Simonart <[email protected]>
---
  controller/physical.c |  9 ++--
  tests/ovn.at          | 98 +++++++++++++++++++------------------------
  2 files changed, 46 insertions(+), 61 deletions(-)

diff --git a/controller/physical.c b/controller/physical.c
index 2aaa16cbd..c6db4f376 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -699,7 +699,7 @@ put_replace_chassis_mac_flows(const struct shash *ct_zones,
          put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
          ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
                          rport_binding->header_.uuid.parts[0],
-                        &match, ofpacts_p, &localnet_port->header_.uuid);
+                        &match, ofpacts_p, hc_uuid);
/* Provide second search criteria, i.e localnet port's
           * vlan ID for conjunction flow */
@@ -719,7 +719,7 @@ put_replace_chassis_mac_flows(const struct shash *ct_zones,
          conj->clause = 1;
          ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
                          rport_binding->header_.uuid.parts[0],
-                        &match, ofpacts_p, &localnet_port->header_.uuid);
+                        &match, ofpacts_p, hc_uuid);
      }
  }
@@ -2393,9 +2393,8 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
      struct local_datapath *ldp =
          get_local_datapath(p_ctx->local_datapaths,
                             pb->datapath->tunnel_key);
-    if (!strcmp(pb->type, "external") ||
-        !strcmp(pb->type, "patch") || !strcmp(pb->type, "l3gateway")) {
-        /* Those lports have a dependency on the localnet port.
+    if (!strcmp(pb->type, "external")) {
+        /* External lports have a dependency on the localnet port.
           * We need to remove the flows of the localnet port as well
           * and re-consider adding the flows for it.
           */
diff --git a/tests/ovn.at b/tests/ovn.at
index 10cd7a79b..dfb08fd1e 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -39495,62 +39495,6 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
  OVN_CLEANUP([hv1])
  AT_CLEANUP
  ])
-OVN_FOR_EACH_NORTHD([
-AT_SETUP([localnet port flows after deletion])
-ovn_start
-net_add n1
-
-check ovn-nbctl ls-add sw0
-
-for i in 1 2; do
-    check ovn-nbctl lsp-add sw0 sw0-p${i} -- lsp-set-addresses sw0-p${i} 
"00:00:10:01:02:0${i} 10.0.0.${i}"
-    sim_add hv${i}
-    as hv${i}
-    ovs-vsctl add-br br-phys
-    ovn_attach n1 br-phys 192.168.0.${i}
-    ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys
-    ovs-vsctl add-port br-int vif${i} -- \
-        set Interface vif${i} external-ids:iface-id=sw0-p${i} \
-                              options:tx_pcap=hv${i}/vif${i}-tx.pcap \
-                              options:rxq_pcap=hv${i}/vif${i}-rx.pcap
-done
-
-check ovn-nbctl lr-add lr0
-check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24
-check ovn-nbctl lsp-add sw0 sw0-lr0
-check ovn-nbctl lsp-set-type sw0-lr0 router
-check ovn-nbctl lsp-set-addresses sw0-lr0 router
-check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
-
-check ovn-nbctl --wait=hv sync
-wait_for_ports_up
-
-# We should not have any flows in table OFTABLE_PHY_TO_LOG from in_port 
different from vif1 and ovn-hv2-0
-OVN_WAIT_REMOTE_INPUT_FLOWS(["hv1"],["hv2"])
-of1=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=vif1)
-of2=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-hv2-0)
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v NXST_FLOW | grep 
"in_port=" | grep -v "in_port=$of1" | grep -v "in_port=$of2" | wc -l], [0], [dnl
-0
-])
-
-# Add localnet port to sw0
-check ovn-nbctl lsp-add sw0 ln-sw0 -- lsp-set-addresses ln-sw0 unknown -- 
lsp-set-type ln-sw0 localnet
-check ovn-nbctl --wait=hv lsp-set-options ln-sw0 network_name=physnet1 -- set 
logical_switch_port ln-sw0 tag_request=100
-
-OVN_WAIT_PATCH_PORT_FLOWS(["ln-sw0"], ["hv1"])
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v NXST_FLOW | grep 
"in_port=" | grep -v "in_port=$of1" | grep -v "in_port=$of2" | wc -l], [0], [dnl
-2
-])
-
-# Remove localnet port from sw0. Peer-ports flows should be deleted.
-check ovn-nbctl --wait=hv lsp-del ln-sw0
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v NXST_FLOW | grep 
"in_port=" | grep -v "in_port=$of1" | grep -v "in_port=$of2" | wc -l], [0], [dnl
-0
-])
-
-OVN_CLEANUP([hv1],[hv2])
-AT_CLEANUP
-])
AT_SETUP([Patch ports not owned by OVN]) @@ -39665,3 +39609,45 @@ check_patch_ports OVN_CLEANUP([hv1])
  AT_CLEANUP
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Multiple localnet ports without vlans])
+AT_KEYWORDS([localnet])
+
+ovn_start
+
+net_add n1
+sim_add hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovn-appctl vlog/set dbg
+ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=phys:br-phys
+
+check ovn-nbctl lr-add lr1
+check ovn-nbctl lrp-add lr1 lr1-ls1 00:00:01:ff:02:03 192.168.1.254/24
+
+check ovn-nbctl ls-add ls1
+check ovn-nbctl lsp-add ls1 ls1-lr1
+check ovn-nbctl lsp-set-type ls1-lr1 router
+check ovn-nbctl lsp-set-options ls1-lr1 router-port=lr1-ls1
+check ovn-nbctl lsp-set-addresses ls1-lr1 router
+
+check ovn-nbctl ls-add pub
+check ovn-nbctl lsp-add pub pub-lr1
+check ovn-nbctl lsp-set-type pub-lr1 router
+check ovn-nbctl lsp-set-options pub-lr1 router-port=lr1-pub
+check ovn-nbctl lsp-set-addresses pub-lr1 router
+
+check ovn-nbctl lrp-add lr1 lr1-pub 00:00:01:ff:01:03 172.16.1.254/24
+check ovn-nbctl lrp-set-gateway-chassis lr1-pub hv1
+
+check ovn-nbctl --wait=hv sync
+wait_for_ports_up
+
+check check ovn-nbctl -- lsp-add pub pub-ln -- lsp-set-type pub-ln localnet -- 
lsp-set-addresses pub-ln unknown -- lsp-set-options pub-ln network_name=phys
+check check ovn-nbctl -- lsp-add ls1 ls1-ln -- lsp-set-type ls1-ln localnet -- 
lsp-set-addresses ls1-ln unknown -- lsp-set-options ls1-ln network_name=phys
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
+])

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

Reply via email to