pinctrl.c: Send GARP only on chassis atached to l3gw

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 <amusil@redhat.com> Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> Signed-off-by: Mark Michelson <mmichels@redhat.com>
ovn-org · Apr 21, 2022 · 77ec310 · 77ec310
1 parent 388446f
commit 77ec310
Show file tree

Hide file tree

Showing 2 changed files with 109 additions and 1 deletion.
diff --git 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
@@ -8588,6 +8588,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