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

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

2 files changed, 104 insertions, 33 deletions

diff --git a/src/vnet/ethernet/arp.c b/src/vnet/ethernet/arp.c
index 619628b37ca..d5dc9cceb39 100644
--- 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 cdebfbce0a9..1143f05aae1 100644
--- 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);
 }