[ovs-dev] [PATCH ovn v2] ovn-northd: Add ARP responder flows for SNAT entries.

numans at ovn.org numans at ovn.org
Tue Aug 11 10:16:43 UTC 2020


From: Numan Siddique <numans at ovn.org>

If the below SNAT entry is added:
ovn-nbctl lr-nat-add snat 172.168.0.120 10.0.0.5

And when the logical port with IP - 10.0.0.5, sends a packet destined to
outside, gets SNATted to 172.168.0.120 as expected, but for the reverse traffic
if the upstream switch sends an ARP request for 172.168.0.120, then OVN doesn't
reply and the reply traffic never reaches the logical port.

This patch fixes this issue by addding ARP responder flows for SNAT entries.

Note that, if 172.168.0.120 happens to be the logical router IP, then it works.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1861294
Reported-by: Alexander Constantinescu <aconstan at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
---

v1 -> v2
---
  * Addressed comments from Dumitru by adding system test.

 northd/ovn-northd.8.xml | 18 ++++-----
 northd/ovn-northd.c     | 13 +++++-
 tests/ovn-northd.at     |  8 ++++
 tests/system-ovn.at     | 89 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 117 insertions(+), 11 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 20fd19450..ee21c825d 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1952,15 +1952,15 @@ nd_na_router {
       <li>
         <p>
           These flows reply to ARP requests or IPv6 neighbor solicitation
-          for the virtual IP addresses configured in the router for DNAT
-          or load balancing.
+          for the virtual IP addresses configured in the router for NAT
+          (both DNAT and SNAT) or load balancing.
         </p>
 
         <p>
-          IPv4: For a configured DNAT IP address or a load balancer
-          IPv4 VIP <var>A</var>, for each router port <var>P</var> with
-          Ethernet address <var>E</var>, a priority-90 flow matches
-          <code>arp.op == 1 && arp.tpa == <var>A</var></code>
+          IPv4: For a configured NAT (both DNAT and SNAT) IP address or a
+          load balancer IPv4 VIP <var>A</var>, for each router port
+          <var>P</var> with Ethernet address <var>E</var>, a priority-90 flow
+          matches <code>arp.op == 1 && arp.tpa == <var>A</var></code>
           (ARP request) with the following actions:
         </p>
 
@@ -1990,9 +1990,9 @@ output;
         </p>
 
         <p>
-          IPv6: For a configured DNAT IP address or a load balancer
-          IPv6 VIP <var>A</var>, solicited node address <var>S</var>,
-          for each router port <var>P</var> with
+          IPv6: For a configured NAT (both DNAT and SNAT) IP address or a
+          load balancer IPv6 VIP <var>A</var>, solicited node address
+          <var>S</var>, for each router port <var>P</var> with
           Ethernet address <var>E</var>, a priority-90 flow matches
           <code>inport == <var>P</var> && nd_ns &&
           ip6.dst == {<var>A</var>, <var>S</var>} &&
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 99eb582c3..a27d4b13d 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8648,6 +8648,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
          * per logical port but DNAT addresses can be handled per datapath
          * for non gateway router ports.
          */
+        struct sset snat_ips = SSET_INITIALIZER(&snat_ips);
         for (int i = 0; i < od->nbr->n_nat; i++) {
             struct ovn_nat *nat_entry = &od->nat_entries[i];
             const struct nbrec_nat *nat = nat_entry->nb;
@@ -8657,15 +8658,22 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                 continue;
             }
 
+            struct lport_addresses *ext_addrs = &nat_entry->ext_addrs;
+            char *ext_addr = nat_entry_is_v6(nat_entry) ?
+                             ext_addrs->ipv6_addrs[0].addr_s :
+                             ext_addrs->ipv4_addrs[0].addr_s;
+
             if (!strcmp(nat->type, "snat")) {
-                continue;
+                if (sset_contains(&snat_ips, ext_addr)) {
+                    continue;
+                }
+                sset_add(&snat_ips, ext_addr);
             }
 
             /* Priority 91 and 92 flows are added for each gateway router
              * port to handle the special cases. In case we get the packet
              * on a regular port, just reply with the port's ETH address.
              */
-            struct lport_addresses *ext_addrs = &nat_entry->ext_addrs;
             if (nat_entry_is_v6(nat_entry)) {
                 build_lrouter_nd_flow(od, NULL, "nd_na",
                                       ext_addrs->ipv6_addrs[0].addr_s,
@@ -8679,6 +8687,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                                        &nat->header_, lflows);
             }
         }
+        sset_destroy(&snat_ips);
 
         /* Drop ARP packets (priority 85). ARP request packets for router's own
          * IPs are handled with priority-90 flows.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 7872d5051..8344c7f5e 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1572,6 +1572,8 @@ ovn-nbctl set logical_router lr options:chassis=ch
 ovn-nbctl lr-nat-add lr dnat_and_snat 43.43.43.2 42.42.42.2
 ovn-nbctl lr-nat-add lr dnat 43.43.43.3 42.42.42.3
 ovn-nbctl lr-nat-add lr dnat_and_snat 43.43.43.4 42.42.42.4 ls-vm 00:00:00:00:00:02
+ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.50
+ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.51
 
 ovn-nbctl --wait=sb sync
 
@@ -1594,6 +1596,9 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 # Ingress router port ETH address is used for ARP reply/NA in lr_in_ip_input.
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.150), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(arp.op == 1 && arp.tpa == 43.43.43.2), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
@@ -1648,6 +1653,9 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 # Priority 90 flows (per router).
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.150), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(arp.op == 1 && arp.tpa == 43.43.43.2), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 5fda0960f..40ba6e44c 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -5308,3 +5308,92 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
 /could not open network device p1*/d"])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- ARP resolution for SNAT IP])
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_BR([br-int])
+ADD_BR([br-ext])
+
+ovs-ofctl add-flow br-ext action=normal
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+        -- set Open_vSwitch . external-ids:system-id=hv1 \
+        -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+ovn-nbctl lr-add R1
+
+ovn-nbctl ls-add sw0
+ovn-nbctl ls-add public
+
+ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24
+ovn-nbctl lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24 \
+    -- set Logical_Router_Port rp-public options:redirect-chassis=hv1
+
+ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
+    type=router options:router-port=rp-sw0 \
+    -- lsp-set-addresses sw0-rp router
+
+ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port public-rp \
+    type=router options:router-port=rp-public \
+    -- lsp-set-addresses public-rp router
+
+ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.1.0/24
+ovn-nbctl lr-nat-add R1 snat 172.16.1.20 192.168.1.2
+
+ADD_NAMESPACES(sw01-x)
+ADD_VETH(sw01-x, sw01-x, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \
+         "192.168.1.1")
+ovn-nbctl lsp-add sw0 sw01-x \
+    -- lsp-set-addresses sw01-x "f0:00:00:01:02:03 192.168.1.2"
+
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw01-x) = xup])
+
+ADD_NAMESPACES(ext-foo)
+ADD_VETH(ext-foo, ext-foo, br-ext, "172.16.1.100/24", "00:10:10:01:02:13", \
+         "172.16.1.1")
+
+OVS_WAIT_UNTIL([test "$(ip netns exec ext-foo ip a | grep fe80 | grep tentative)" = ""])
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=phynet:br-ext])
+ovn-nbctl lsp-add public public1 \
+        -- lsp-set-addresses public1 unknown \
+        -- lsp-set-type public1 localnet \
+        -- lsp-set-options public1 network_name=phynet
+
+ovn-nbctl --wait=hv sync
+
+# Send ping from sw01-x to ext-foo.
+NS_CHECK_EXEC([sw01-x], [ping -q -c 3 -i 0.3 -w 2 172.16.1.100 | FORMAT_PING], \
+[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+# Check conntrack entries.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.100) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+icmp,orig=(src=192.168.1.2,dst=172.16.1.100,id=<cleared>,type=8,code=0),reply=(src=172.16.1.100,dst=172.16.1.20,id=<cleared>,type=0,code=0),zone=<cleared>
+])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
+/.*terminating with signal 15.*/d"])
+AT_CLEANUP
-- 
2.26.2



More information about the dev mailing list