From 5521da70830446373265999b6d994d986a02ce01 Mon Sep 17 00:00:00 2001
From: Numan Siddique <numans@ovn.org>
Date: Thu, 19 Mar 2020 16:52:17 +0530
Subject: [PATCH] ovn-northd: Don't add arp responder flows for lports with
 'unknown' address.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If a logical port has 'unknown' address, it means it can send and receive
packet with any IP and MAC and generally port security is not set for
such logical ports. If an lport has addresses set to - ["MAC1 IP1", unknown],
right now we add arp responder flows for IP1 and respond MAC1 in the arp
response. But it's possible that the VIF of the logical port can use the IP1
with a different MAC. This patch supports this usecase. When another logical port
sends ARP request for IP1, the VIF of the logical port will anyway respond.

Reported-by: Maciej Józefczyk <mjozefcz@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/ovn-northd.8.xml |  5 +++--
 northd/ovn-northd.c     | 13 ++++++++-----
 tests/ovn.at            | 16 ++++++++++++----
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 9b44720d1..7d03cbc83 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -699,8 +699,9 @@ output;
 
         <p>
           These flows are omitted for logical ports (other than router ports or
-          <code>localport</code> ports) that are down and for logical ports of
-          type <code>virtual</code>.
+          <code>localport</code> ports) that are down, for logical ports of
+          type <code>virtual</code> and for logical ports with 'unknown'
+          address set.
         </p>
       </li>
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 3a77f2e3a..356c5436c 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1148,7 +1148,7 @@ struct ovn_port {
 
     bool derived; /* Indicates whether this is an additional port
                    * derived from nbsp or nbrp. */
-
+    bool has_unknown; /* If the addresses have 'unknown' defined. */
     /* The port's peer:
      *
      *     - A switch port S of type "router" has a router port R as a peer,
@@ -2055,8 +2055,11 @@ join_logical_ports(struct northd_context *ctx,
                 op->lsp_addrs
                     = xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses);
                 for (size_t j = 0; j < nbsp->n_addresses; j++) {
-                    if (!strcmp(nbsp->addresses[j], "unknown")
-                        || !strcmp(nbsp->addresses[j], "router")) {
+                    if (!strcmp(nbsp->addresses[j], "unknown")) {
+                        op->has_unknown = true;
+                        continue;
+                    }
+                    if (!strcmp(nbsp->addresses[j], "router")) {
                         continue;
                     }
                     if (is_dynamic_lsp_address(nbsp->addresses[j])) {
@@ -6123,7 +6126,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
         } else {
             /*
              * Add ARP/ND reply flows if either the
-             *  - port is up or
+             *  - port is up and it doesn't have 'unknown' address defined or
              *  - port type is router or
              *  - port type is localport
              */
@@ -6132,7 +6135,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
                 continue;
             }
 
-            if (lsp_is_external(op->nbsp)) {
+            if (lsp_is_external(op->nbsp) || op->has_unknown) {
                 continue;
             }
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 8cdbad743..1b6073ff0 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1758,11 +1758,13 @@ for is in 1 2 3; do
                 sip=`ip_to_hex 192 168 0 $is$js`
                 tip=`ip_to_hex 192 168 0 $id$jd`
                 tip_unknown=`ip_to_hex 11 11 11 11`
+                reply_ha=;
                 if test $d != $s; then
-                    reply_ha=f000000000$d
-                else
-                    reply_ha=
+                    if test $jd != 1; then
+                        reply_ha=f000000000$d
+                    fi
                 fi
+
                 test_arp $s f000000000$s $sip $tip $reply_ha               #9
                 test_arp $s f000000000$s $sip $tip_unknown                 #10
 
@@ -2199,7 +2201,13 @@ for s in 1 2 3; do
         sip=192.168.0.$s
         tip=192.168.0.$d
         tip_unknown=11.11.11.11
-        if test $d != $s; then reply_ha=f0:00:00:00:00:0$d; else reply_ha=; fi
+        reply_ha=;
+        if test $d != $s; then
+            if test $d != 1; then
+                reply_ha=f0:00:00:00:00:0$d;
+            fi
+        fi
+
         test_arp $s f0:00:00:00:00:0$s $sip $tip $reply_ha                 #9
         test_arp $s f0:00:00:00:00:0$s $sip $tip_unknown                   #10
 
-- 
2.24.1