diff mbox series

[ovs-dev,branch-22.09,2/3] pinctrl: Send RARPs for external ipv6 interfaces

Message ID 20230131091047.184728-2-amusil@redhat.com
State Accepted
Headers show
Series [ovs-dev,branch-22.09,1/3] ovn-macros: support ipv6 in ovn_attach | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Ales Musil Jan. 31, 2023, 9:10 a.m. UTC 
From: Felix Hüttner <felix.huettner@mail.schwarz>

previously garps/rarps were only sent for NAT IPs if these had an
ipv4 address attached. For lsp's on gateway routers that do not have
an ipv4 address assigned (e.g. if they are ipv6 only) no rarps where
send out.

This causes traffic outages when changing the priority of a gateway
chassis as the physical switches to not get the information where the
mac address now resides. To fix this, we send out rarps with just the mac
address of the interface and no ip address.

This change has been tested in an environment with 600 logical routers
on a single ipv6 external network.

Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz>
Signed-off-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Ales Musil <amusil@redhat.com>
(cherry picked from commit 9ac54852)
---
 controller/pinctrl.c | 23 +++++++++++++
 tests/ovn.at         | 80 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 99 insertions(+), 4 deletions(-)

Comments

0-day Robot Jan. 31, 2023, 9:18 a.m. UTC | #1 
Bleep bloop.  Greetings Ales Musil, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Felix Hüttner <felix.huettner@mail.schwarz> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Felix Huettner <felix.huettner@mail.schwarz>, Numan Siddique <numans@ovn.org>, Ales Musil <amusil@redhat.com>
Lines checked: 182, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Dumitru Ceara Feb. 2, 2023, 4:31 p.m. UTC | #2 
On 1/31/23 10:10, Ales Musil wrote:
> From: Felix Hüttner <felix.huettner@mail.schwarz>
> 
> previously garps/rarps were only sent for NAT IPs if these had an
> ipv4 address attached. For lsp's on gateway routers that do not have
> an ipv4 address assigned (e.g. if they are ipv6 only) no rarps where
> send out.
> 
> This causes traffic outages when changing the priority of a gateway
> chassis as the physical switches to not get the information where the
> mac address now resides. To fix this, we send out rarps with just the mac
> address of the interface and no ip address.
> 
> This change has been tested in an environment with 600 logical routers
> on a single ipv6 external network.
> 
> Acked-by: Numan Siddique <numans@ovn.org>
> Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> Signed-off-by: Ales Musil <amusil@redhat.com>
> (cherry picked from commit 9ac54852)
> ---

Thanks for the backports!  I applied this and related patches 1/3 and
3/3 to branches 22.09, 22.06, 22.03, 21.12.

Just a note, for upcoming series, a (very brief) cover letter would help
point out what exactly we're backporting and why we also need the
related patches.

Best regards,
Dumitru
diff mbox series

Patch

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index ace086f00..77d2854fe 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -4534,6 +4534,24 @@  send_garp_rarp_update(struct ovsdb_idl_txn *ovnsb_idl_txn,
                 }
                 free(name);
             }
+            /*
+             * Send RARPs even if we do not have a ipv4 address as it e.g.
+             * happens on ipv6 only ports.
+             */
+            if (laddrs->n_ipv4_addrs == 0) {
+                    char *name = xasprintf("%s-noip",
+                                           binding_rec->logical_port);
+                    garp_rarp = shash_find_data(&send_garp_rarp_data, name);
+                    if (garp_rarp) {
+                        garp_rarp->dp_key = binding_rec->datapath->tunnel_key;
+                        garp_rarp->port_key = binding_rec->tunnel_key;
+                    } else {
+                        add_garp_rarp(name, laddrs->ea,
+                                      0, binding_rec->datapath->tunnel_key,
+                                      binding_rec->tunnel_key);
+                    }
+                    free(name);
+            }
             destroy_lport_addresses(laddrs);
             free(laddrs);
         }
@@ -5846,6 +5864,11 @@  consider_nat_address(struct ovsdb_idl_index *sbrec_port_binding_by_name,
         sset_add(nat_address_keys, name);
         free(name);
     }
+    if (laddrs->n_ipv4_addrs == 0) {
+        char *name = xasprintf("%s-noip", pb->logical_port);
+        sset_add(nat_address_keys, name);
+        free(name);
+    }
     shash_add(nat_addresses, pb->logical_port, laddrs);
 }
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 7e2dfd0bf..9f8e8b8fc 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9032,6 +9032,76 @@  OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([send reverse arp for router without ipv4 address])
+ovn_start
+# Create logical switch
+ovn-nbctl ls-add ls0
+# Create gateway router
+ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
+# Add router port to gateway router
+ovn-nbctl lrp-add lr0 lrp0 f0:00:00:00:00:01 fd12:3456:789a:1::1/64
+ovn-nbctl lsp-add ls0 lrp0-rp -- set Logical_Switch_Port lrp0-rp \
+    type=router options:router-port=lrp0 addresses='"f0:00:00:00:00:01"'
+# Add nat-address option
+ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router"
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl \
+    -- add-br br-phys \
+    -- add-br br-eth0
+
+ovn_attach n1 br-phys fd12:3456:789a:1::1 64
+
+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])
+
+# Create a localnet port.
+AT_CHECK([ovn-nbctl lsp-add ls0 ln_port])
+AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown])
+AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet])
+AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1])
+
+# Wait until the patch ports are created to connect br-int to br-eth0
+OVS_WAIT_UNTIL([test 1 = `ovs-vsctl show | \
+grep "Port patch-br-int-to-ln_port" | wc -l`])
+
+ovn-sbctl list port_binding lrp0-rp
+echo "*****"
+ovn-nbctl list logical_switch_port lrp0-rp
+ovn-nbctl list logical_router_port lrp0
+ovn-nbctl show
+# Wait for packet to be received.
+OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50])
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap  | sort | uniq > packets
+expected="fffffffffffff0000000000180350001080006040003f0000000000100000000f0000000000100000000"
+echo $expected > expout
+AT_CHECK([sort packets], [0], [expout])
+
+# Temporarily remove nat-addresses option to avoid race conditions
+# due to GARP backoff
+ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses=""
+
+as hv1 reset_pcap_file snoopvif hv1/snoopvif
+
+# Re-add nat-addresses option
+ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router" exclude-lb-vips-from-garp="true"
+
+# Wait for packets to be received.
+OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50])
+
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap  > packets
+expected="fffffffffffff0000000000180350001080006040003f0000000000100000000f0000000000100000000"
+echo $expected > expout
+AT_CHECK([sort packets], [0], [expout])
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([delete mac bindings])
 ovn_start
@@ -13892,10 +13962,11 @@  $mcast_node_ip $nd_target $nd_src_ip
 OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys_n1-tx.pcap | cut -d " " -f1)])
 OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys-tx.pcap | cut -d " " -f1)])
 
+# Use the grep here to filter out rarp packets that might have arrived
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/br-phys_n1-tx.pcap | \
-trim_zeros > 1.packets
+grep -v ffffffffffff | trim_zeros > 1.packets
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | \
-trim_zeros > 2.packets
+grep -v ffffffffffff | trim_zeros > 2.packets
 
 cat ipv6_ns.expected | cut -c -112 > expout
 AT_CHECK([cat 1.packets | cut -c -112], [0], [expout])
@@ -13932,10 +14003,11 @@  $mcast_node_ip $nd_target $nd_src_ip
 OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys_n1-tx.pcap | cut -d " " -f1)])
 OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys-tx.pcap | cut -d " " -f1)])
 
+# Use the grep here to filter out rarp packets that might have arrived
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/br-phys_n1-tx.pcap | \
-trim_zeros > 1.packets
+grep -v ffffffffffff | trim_zeros > 1.packets
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | \
-trim_zeros > 2.packets
+grep -v ffffffffffff | trim_zeros > 2.packets
 
 cat ipv6_ns.expected | cut -c -112 > expout
 AT_CHECK([cat 1.packets | cut -c -112], [0], [expout])