ARP: ignore non-connected routes and non-interface sources when determing if source...

Change-Id: I39fb0ec44cc322eaa12c0ff0700fc405d3982bfc
Signed-off-by: Neale Ranns <nranns@cisco.com>

diff --git a/src/vnet/ethernet/arp.c b/src/vnet/ethernet/arp.c
index 619628b..d5dc9cc 100644 (file)
--- a/src/vnet/ethernet/arp.c
+++ b/src/vnet/ethernet/arp.c
@@ -22,6 +22,7 @@
 #include <vnet/l2/l2_input.h>
 #include <vppinfra/mhash.h>
 #include <vnet/fib/ip4_fib.h>
+#include <vnet/fib/fib_entry_src.h>
 #include <vnet/adj/adj_nbr.h>
 #include <vnet/adj/adj_mcast.h>
 #include <vnet/mpls/mpls.h>
@@ -955,52 +956,118 @@ arp_input (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_frame_t * frame)
                                          32);
          dst_flags = fib_entry_get_flags (dst_fei);
 
-         src_fei = ip4_fib_table_lookup (ip4_fib_get (fib_index0),
-                                         &arp0->ip4_over_ethernet[0].ip4,
-                                         32);
-         src_flags = fib_entry_get_flags (src_fei);
-
          conn_sw_if_index0 = fib_entry_get_resolving_interface (dst_fei);
 
-         if (!(FIB_ENTRY_FLAG_CONNECTED & dst_flags))
-           {
-             error0 = ETHERNET_ARP_ERROR_l3_dst_address_not_local;
-             goto drop1;
-           }
-
          /* Honor unnumbered interface, if any */
          is_unnum0 = sw_if_index0 != conn_sw_if_index0;
 
-         /* Source must also be local to subnet of matching interface address. */
-         if (!((FIB_ENTRY_FLAG_ATTACHED & src_flags) ||
-               (FIB_ENTRY_FLAG_CONNECTED & src_flags)))
+         {
+           /*
+            * we're looking for FIB entries that indicate the source
+            * is attached. There may be more specific non-attached
+            * routes tht match the source, but these do not influence
+            * whether we respond to an ARP request, i.e. they do not
+            * influence whether we are the correct way for the sender
+            * to reach us, they only affect how we reach the sender.
+            */
+           fib_entry_t *src_fib_entry;
+           fib_entry_src_t *src;
+           fib_source_t source;
+           fib_prefix_t pfx;
+           int attached;
+           int mask;
+
+           mask = 32;
+           attached = 0;
+
+           do
+             {
+               src_fei = ip4_fib_table_lookup (ip4_fib_get (fib_index0),
+                                               &arp0->
+                                               ip4_over_ethernet[0].ip4,
+                                               mask);
+               src_fib_entry = fib_entry_get (src_fei);
+
+               /*
+                * It's possible that the source that provides the
+                * flags we need, or the flags we must not have,
+                * is not the best source, so check then all.
+                */
+                /* *INDENT-OFF* */
+                FOR_EACH_SRC_ADDED(src_fib_entry, src, source,
+                ({
+                  src_flags = fib_entry_get_flags_for_source (src_fei, source);
+
+                  /* Reject requests/replies with our local interface
+                     address. */
+                  if (FIB_ENTRY_FLAG_LOCAL & src_flags)
+                    {
+                      error0 = ETHERNET_ARP_ERROR_l3_src_address_is_local;
+                      goto drop2;
+                    }
+                  /* A Source must also be local to subnet of matching
+                   * interface address. */
+                  if ((FIB_ENTRY_FLAG_ATTACHED & src_flags) ||
+                      (FIB_ENTRY_FLAG_CONNECTED & src_flags))
+                    {
+                      attached = 1;
+                      break;
+                    }
+                  /*
+                   * else
+                   *  The packet was sent from an address that is not
+                   *  connected nor attached i.e. it is not from an
+                   *  address that is covered by a link's sub-net,
+                   *  nor is it a already learned host resp.
+                   */
+                }));
+                /* *INDENT-ON* */
+
+               /*
+                * shorter mask lookup for the next iteration.
+                */
+               fib_entry_get_prefix (src_fei, &pfx);
+               mask = pfx.fp_len - 1;
+
+               /*
+                * continue until we hit the default route or we find
+                * the attached we are looking for. The most likely
+                * outcome is we find the attached with the first source
+                * on the first lookup.
+                */
+             }
+           while (!attached &&
+                  !fib_entry_is_sourced (src_fei, FIB_SOURCE_DEFAULT_ROUTE));
+
+           if (!attached)
+             {
+               /*
+                * the matching route is a not attached, i.e. it was
+                * added as a result of routing, rather than interface/ARP
+                * configuration. If the matching route is not a host route
+                * (i.e. a /32)
+                */
+               error0 = ETHERNET_ARP_ERROR_l3_src_address_not_local;
+               goto drop2;
+             }
+         }
+
+         if (!(FIB_ENTRY_FLAG_CONNECTED & dst_flags))
            {
-             /*
-              * The packet was sent from an address that is not connected nor attached
-              * i.e. it is not from an address that is covered by a link's sub-net,
-              * nor is it a already learned host resp.
-              */
-             error0 = ETHERNET_ARP_ERROR_l3_src_address_not_local;
-             goto drop2;
+             error0 = ETHERNET_ARP_ERROR_l3_dst_address_not_local;
+             goto drop1;
            }
 
          if (sw_if_index0 != fib_entry_get_resolving_interface (src_fei))
            {
              /*
               * The interface the ARP was received on is not the interface
-              * on which the covering prefix is configured. Maybe this is a case
-              * for unnumbered.
+              * on which the covering prefix is configured. Maybe this is a
+              * case for unnumbered.
               */
              is_unnum0 = 1;
            }
 
-         /* Reject requests/replies with our local interface address. */
-         if (FIB_ENTRY_FLAG_LOCAL & src_flags)
-           {
-             error0 = ETHERNET_ARP_ERROR_l3_src_address_is_local;
-             goto drop2;
-           }
-
          dst_is_local0 = (FIB_ENTRY_FLAG_LOCAL & dst_flags);
          fib_entry_get_prefix (dst_fei, &pfx0);
          if_addr0 = &pfx0.fp_addr.ip4;

diff --git a/src/vnet/fib/fib_entry.c b/src/vnet/fib/fib_entry.c
index cdebfbc..1143f05 100644 (file)
--- a/src/vnet/fib/fib_entry.c
+++ b/src/vnet/fib/fib_entry.c
@@ -462,11 +462,15 @@ fib_entry_get_adj (fib_node_index_t fib_entry_index)
     const dpo_id_t *dpo;
 
     dpo = fib_entry_contribute_ip_forwarding(fib_entry_index);
-    dpo = load_balance_get_bucket(dpo->dpoi_index, 0);
 
-    if (dpo_is_adj(dpo))
+    if (dpo_id_is_valid(dpo))
     {
-       return (dpo->dpoi_index);
+        dpo = load_balance_get_bucket(dpo->dpoi_index, 0);
+
+        if (dpo_is_adj(dpo))
+        {
+            return (dpo->dpoi_index);
+        }
     }
     return (ADJ_INDEX_INVALID);
 }

diff --git a/test/test_neighbor.py b/test/test_neighbor.py
index 1167b26..b4a6878 100644 (file)
--- a/test/test_neighbor.py
+++ b/test/test_neighbor.py
@@ -142,7 +142,7 @@ class ARPTestCase(VppTestCase):
         #
         # Generate some hosts on the LAN
         #
-        self.pg1.generate_remote_hosts(10)
+        self.pg1.generate_remote_hosts(11)
 
         #
         # Send IP traffic to one of these unresolved hosts.
@@ -495,6 +495,65 @@ class ARPTestCase(VppTestCase):
                              self.pg1.local_ip4,
                              self.pg1._remote_hosts[9].ip4)
 
+        #
+        # Add a hierachy of routes for a host in the sub-net.
+        # Should still get an ARP resp since the cover is attached
+        #
+        p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg1.remote_mac) /
+             ARP(op="who-has",
+                 hwsrc=self.pg1.remote_mac,
+                 pdst=self.pg1.local_ip4,
+                 psrc=self.pg1.remote_hosts[10].ip4))
+
+        r1 = VppIpRoute(self, self.pg1.remote_hosts[10].ip4, 30,
+                        [VppRoutePath(self.pg1.remote_hosts[10].ip4,
+                                      self.pg1.sw_if_index)])
+        r1.add_vpp_config()
+
+        self.pg1.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        rx = self.pg1.get_capture(1)
+        self.verify_arp_resp(rx[0],
+                             self.pg1.local_mac,
+                             self.pg1.remote_mac,
+                             self.pg1.local_ip4,
+                             self.pg1.remote_hosts[10].ip4)
+
+        r2 = VppIpRoute(self, self.pg1.remote_hosts[10].ip4, 32,
+                        [VppRoutePath(self.pg1.remote_hosts[10].ip4,
+                                      self.pg1.sw_if_index)])
+        r2.add_vpp_config()
+
+        self.pg1.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        rx = self.pg1.get_capture(1)
+        self.verify_arp_resp(rx[0],
+                             self.pg1.local_mac,
+                             self.pg1.remote_mac,
+                             self.pg1.local_ip4,
+                             self.pg1.remote_hosts[10].ip4)
+
+        #
+        # add an ARP entry that's not on the sub-net and so whose
+        # adj-fib fails the refinement check. then send an ARP request
+        # from that source
+        #
+        a1 = VppNeighbor(self,
+                         self.pg0.sw_if_index,
+                         self.pg0.remote_mac,
+                         "100.100.100.50")
+        a1.add_vpp_config()
+
+        p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) /
+             ARP(op="who-has",
+                 hwsrc=self.pg0.remote_mac,
+                 psrc="100.100.100.50",
+                 pdst=self.pg0.remote_ip4))
+        self.send_and_assert_no_replies(self.pg0, p,
+                                        "ARP req for from failed adj-fib")
+
         #
         # ERROR Cases
         #  1 - don't respond to ARP request for address not within the
@@ -536,7 +595,8 @@ class ARPTestCase(VppTestCase):
         #
         #  2 - don't respond to ARP request from an address not within the
         #      interface's sub-net
-        #
+        #   2b - to a prxied address
+        #   2c - not within a differents interface's sub-net
         p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) /
              ARP(op="who-has",
                  hwsrc=self.pg0.remote_mac,
@@ -552,6 +612,13 @@ class ARPTestCase(VppTestCase):
         self.send_and_assert_no_replies(
             self.pg0, p,
             "ARP req for non-local source - unnum")
+        p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) /
+             ARP(op="who-has",
+                 hwsrc=self.pg0.remote_mac,
+                 psrc=self.pg1.remote_ip4,
+                 pdst=self.pg0.local_ip4))
+        self.send_and_assert_no_replies(self.pg0, p,
+                                        "ARP req for non-local source 2c")
 
         #
         #  3 - don't respond to ARP request from an address that belongs to
@@ -588,6 +655,7 @@ class ARPTestCase(VppTestCase):
         # need this to flush the adj-fibs
         self.pg2.unset_unnumbered(self.pg1.sw_if_index)
         self.pg2.admin_down()
+        self.pg1.admin_down()
 
     def test_proxy_arp(self):
         """ Proxy ARP """