Thanks Ales and Ihar. I added my ack and pushed this to main and branch-22.03.

On 4/20/22 18:54, Ihar Hrachyshka wrote:
Thank you, this looks great.

Signed-off-by: Ihar Hrachyshka <ihrac...@redhat.com>

On Wed, Apr 20, 2022 at 4:09 AM Ales Musil <amu...@redhat.com> wrote:

The GARP was sent even on chassis that were not serving
as l3gw for the specified router. This could lead to race
on the physical network when multiple chassis send the same
ARP but the physical interfaces have different port numbers
on the external bridge.

Add check to filter out port_bindings for l3gw that
are sitting on different chassis.

Reported-at: https://bugzilla.redhat.com/2062580
Signed-off-by: Ales Musil <amu...@redhat.com>
---
  controller/pinctrl.c |   2 +-
  tests/ovn.at         | 108 +++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 25b37ee88..42d1c2a46 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -5554,7 +5554,7 @@ get_localnet_vifs_l3gwports(
          sbrec_port_binding_index_set_datapath(target, ld->datapath);
          SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
                                             sbrec_port_binding_by_datapath) {
-            if (!strcmp(pb->type, "l3gateway")
+            if ((!strcmp(pb->type, "l3gateway") && pb->chassis == chassis)
                  || !strcmp(pb->type, "patch")) {
                  sset_add(local_l3gw_ports, pb->logical_port);
              }
diff --git a/tests/ovn.at b/tests/ovn.at
index f9551b843..f370e99ae 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8707,6 +8707,114 @@ OVN_CLEANUP([hv1])
  AT_CLEANUP
  ])

+OVN_FOR_EACH_NORTHD([
+AT_SETUP([send gratuitous arp for l3gateway only on selected chassis])
+ovn_start
+
+# Create logical switch
+ovn-nbctl ls-add ls0
+# Create gateway router
+ovn-nbctl lr-add lr0
+# Add router port to gateway router
+ovn-nbctl lrp-add lr0 lr0-ls0 f0:00:00:00:00:01 192.168.0.1/24
+ovn-nbctl lsp-add ls0 ls0-lr0 -- set Logical_Switch_Port ls0-lr0 \
+    type=router options:router-port=lr0-ls0 addresses='"f0:00:00:00:00:01"'
+
+# Create a localnet port.
+ovn-nbctl lsp-add ls0 ln_port
+ovn-nbctl lsp-set-addresses ln_port unknown
+ovn-nbctl lsp-set-type ln_port localnet
+ovn-nbctl --wait=hv lsp-set-options ln_port network_name=physnet1
+
+# Prepare packets
+touch empty_expected
+echo 
"fffffffffffff0000000000108060001080006040001f00000000001c0a80001000000000000c0a80001"
 > arp_expected
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl \
+    -- add-br br-phys \
+    -- add-br br-eth0
+
+ovn_attach n1 br-phys 192.168.0.10
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
external-ids:ovn-bridge-mappings=physnet1:br-eth0])
+AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif 
options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
+
+sim_add hv2
+as hv2
+ovs-vsctl \
+    -- add-br br-phys \
+    -- add-br br-eth0
+
+ovn_attach n1 br-phys 192.168.0.20
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
external-ids:ovn-bridge-mappings=physnet1:br-eth0])
+AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif 
options:tx_pcap=hv2/snoopvif-tx.pcap options:rxq_pcap=hv2/snoopvif-rx.pcap])
+
+ovn-sbctl dump-flows > sbflows
+AT_CAPTURE_FILE([sbflows])
+
+# Wait until the patch ports are created in hv1 and hv2 to connect br-int to 
br-eth0
+AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
+OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-vsctl show | \
+grep "Port patch-br-int-to-ln_port" | wc -l`])
+AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
+OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
+grep "Port patch-br-int-to-ln_port" | wc -l`])
+
+# Temporarily remove lr0 chassis
+AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
+
+reset_pcap_file() {
+    local hv=$1
+    local iface=$2
+    local pcap_file=$3
+    as $hv
+    ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
+options:rxq_pcap=dummy-rx.pcap
+    rm -f ${pcap_file}*.pcap
+    ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
+options:rxq_pcap=${pcap_file}-rx.pcap
+}
+
+reset_pcap_file hv1 snoopvif hv1/snoopvif
+reset_pcap_file hv2 snoopvif hv2/snoopvif
+
+hv1_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv1)
+AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
+OVS_WAIT_UNTIL([
+    ls0_lr0=$(ovn-sbctl --bare --columns chassis list port_binding ls0-lr0)
+    test "$ls0_lr0" = $hv1_uuid
+])
+
+sleep 2
+OVN_CHECK_PACKETS_CONTAIN([hv1/snoopvif-tx.pcap], [arp_expected])
+OVN_CHECK_PACKETS([hv2/snoopvif-tx.pcap], [empty_expected])
+
+# Temporarily remove lr0 chassis
+AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
+
+reset_pcap_file hv1 snoopvif hv1/snoopvif
+reset_pcap_file hv2 snoopvif hv2/snoopvif
+
+hv2_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv2)
+AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
+OVS_WAIT_UNTIL([
+    ls0_lr0=$(ovn-sbctl --bare --columns chassis list port_binding ls0-lr0)
+    test "$ls0_lr0" = $hv2_uuid
+])
+
+sleep 2
+OVN_CHECK_PACKETS_CONTAIN([hv2/snoopvif-tx.pcap], [arp_expected])
+OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap], [empty_expected])
+
+OVN_CLEANUP([hv1],[hv2])
+
+AT_CLEANUP
+])
+
  OVN_FOR_EACH_NORTHD([
  AT_SETUP([send gratuitous arp with nat-addresses router in localnet])
  ovn_start
--
2.35.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to