ARP/ND entries for the same address on different interfaces (VPP-848)

there are, intentionally, no validation checks in the ARP/ND code to prevent an ARP/ND entry from being installed for an address that is not local to the interface's sub-net. This is ok, since the adjacency/FIB code is designed to handle this case using the 'refinement' criteria - i.e. only installing a FIB entry for the address if the address 'refines' (i.e. is more specific than) the interface's sub-net.
However, the refinement criteria currently operates on the FIB entry's prefix (which is a /32, so on the address) and not on the next-hop in the path.
So, enter multiple ARP entries for the same address on different links, and this refinement criteria uses only the last added path, and so will remove the FIB entry should the ARP entries be added in the 'wrong' order.
This fix updates the refinement criteria to work on each path of the FIB entry. The entry is installed if one of the paths refines the covers and only paths refining the cover contribute forwarding.
Per-path refinement checks are stored in path-extensions. The patch is rather large as path-extension, which were previously used only for out-going MPLS labels, have been generalized.

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

1 files changed, 195 insertions, 47 deletions

diff --git a/src/vnet/fib/fib_entry_src_adj.c b/src/vnet/fib/fib_entry_src_adj.c
index 9990223037a..9ea2b17e6b9 100644
--- a/src/vnet/fib/fib_entry_src_adj.c
+++ b/src/vnet/fib/fib_entry_src_adj.c
@@ -19,9 +19,10 @@
 #include "fib_table.h"
 #include "fib_entry_cover.h"
 #include "fib_attached_export.h"
+#include "fib_path_ext.h"
 
 /**
- * Source initialisation Function 
+ * Source initialisation Function
  */
 static void
 fib_entry_src_adj_init (fib_entry_src_t *src)
@@ -31,12 +32,93 @@ fib_entry_src_adj_init (fib_entry_src_t *src)
 }
 
 static void
+fib_entry_src_adj_path_add (fib_entry_src_t *src,
+                            const fib_entry_t *entry,
+                            fib_path_list_flags_t pl_flags,
+                            const fib_route_path_t *paths)
+{
+    const fib_route_path_t *rpath;
+
+    if (FIB_NODE_INDEX_INVALID == src->fes_pl)
+    {
+        src->fes_pl = fib_path_list_create(pl_flags, paths);
+    }
+    else
+    {
+        src->fes_pl = fib_path_list_copy_and_path_add(src->fes_pl,
+                                                      pl_flags,
+                                                      paths);
+    }
+
+    /*
+     * resolve the existing extensions
+     */
+    fib_path_ext_list_resolve(&src->fes_path_exts, src->fes_pl);
+
+    /*
+     * and new extensions
+     */
+    vec_foreach(rpath, paths)
+    {
+        fib_path_ext_list_insert(&src->fes_path_exts,
+                                 src->fes_pl,
+                                 FIB_PATH_EXT_ADJ,
+                                 rpath);
+    }
+}
+
+static void
+fib_entry_src_adj_path_remove (fib_entry_src_t *src,
+                               fib_path_list_flags_t pl_flags,
+                               const fib_route_path_t *rpaths)
+{
+    const fib_route_path_t *rpath;
+
+    if (FIB_NODE_INDEX_INVALID != src->fes_pl)
+    {
+        src->fes_pl = fib_path_list_copy_and_path_remove(src->fes_pl,
+                                                         pl_flags,
+                                                         rpaths);
+    }
+
+    /*
+     * remove the path-extension for the path
+     */
+    vec_foreach(rpath, rpaths)
+    {
+        fib_path_ext_list_remove(&src->fes_path_exts, FIB_PATH_EXT_ADJ, rpath);
+    };
+    /*
+     * resolve the remaining extensions
+     */
+    fib_path_ext_list_resolve(&src->fes_path_exts, src->fes_pl);
+}
+
+static void
 fib_entry_src_adj_path_swap (fib_entry_src_t *src,
-			     const fib_entry_t *entry,
-			     fib_path_list_flags_t pl_flags,
-			     const fib_route_path_t *paths)
+                             const fib_entry_t *entry,
+                             fib_path_list_flags_t pl_flags,
+                             const fib_route_path_t *paths)
 {
+    const fib_route_path_t *rpath;
+
+    /*
+     * flush all the old extensions before we create a brand new path-list
+     */
+    fib_path_ext_list_flush(&src->fes_path_exts);
+
     src->fes_pl = fib_path_list_create(pl_flags, paths);
+
+    /*
+     * and new extensions
+     */
+    vec_foreach(rpath, paths)
+    {
+        fib_path_ext_list_push_back(&src->fes_path_exts,
+                                    src->fes_pl,
+                                    FIB_PATH_EXT_ADJ,
+                                    rpath);
+    }
 }
 
 static void
@@ -45,14 +127,87 @@ fib_entry_src_adj_remove (fib_entry_src_t *src)
     src->fes_pl = FIB_NODE_INDEX_INVALID;
 }
 
+/*
+ * Add a path-extension indicating whether this path is resolved,
+ * because it passed the refinement check
+ */
+static void
+fib_enty_src_adj_update_path_ext (fib_entry_src_t *src,
+                                  fib_node_index_t path_index,
+                                  fib_path_ext_adj_flags_t flags)
+{
+    fib_path_ext_t *path_ext;
+
+    path_ext = fib_path_ext_list_find_by_path_index(&src->fes_path_exts,
+                                                    path_index);
+
+    if (NULL != path_ext)
+    {
+        path_ext->fpe_adj_flags = flags;
+    }
+    else
+    {
+        ASSERT(!"no path extension");
+    }
+}
+
+typedef struct fib_entry_src_path_list_walk_cxt_t_
+{
+    fib_entry_src_t *src;
+    u32 cover_itf;
+    fib_path_ext_adj_flags_t flags;
+} fib_entry_src_path_list_walk_cxt_t;
+
+static fib_path_list_walk_rc_t
+fib_entry_src_adj_path_list_walk (fib_node_index_t pl_index,
+                                  fib_node_index_t path_index,
+                                  void *arg)
+{
+    fib_entry_src_path_list_walk_cxt_t *ctx;
+    u32 adj_itf;
+
+    ctx = arg;
+    adj_itf = fib_path_get_resolving_interface(path_index);
+
+    if (ctx->cover_itf == adj_itf)
+    {
+        fib_enty_src_adj_update_path_ext(ctx->src, path_index,
+                                         FIB_PATH_EXT_ADJ_FLAG_REFINES_COVER);
+        ctx->flags |= FIB_PATH_EXT_ADJ_FLAG_REFINES_COVER;
+    }
+    else
+    {
+        /*
+         * if the interface the adj is on is unnumbered to the
+         * cover's, then allow that too.
+         */
+        vnet_sw_interface_t *swif;
+
+        swif = vnet_get_sw_interface (vnet_get_main(), adj_itf);
+
+        if (swif->flags & VNET_SW_INTERFACE_FLAG_UNNUMBERED &&
+            ctx->cover_itf == swif->unnumbered_sw_if_index)
+        {
+            fib_enty_src_adj_update_path_ext(ctx->src, path_index,
+                                             FIB_PATH_EXT_ADJ_FLAG_REFINES_COVER);
+            ctx->flags |= FIB_PATH_EXT_ADJ_FLAG_REFINES_COVER;
+        }
+        else
+        {
+            fib_enty_src_adj_update_path_ext(ctx->src, path_index,
+                                             FIB_PATH_EXT_ADJ_FLAG_NONE);
+        }
+    }
+    return (FIB_PATH_LIST_WALK_CONTINUE);
+}
 
 /*
- * Source activate. 
+ * Source activate.
  * Called when the source is the new longer best source on the entry
  */
 static int
 fib_entry_src_adj_activate (fib_entry_src_t *src,
-			    const fib_entry_t *fib_entry)
+                            const fib_entry_t *fib_entry)
 {
     fib_entry_t *cover;
 
@@ -61,7 +216,7 @@ fib_entry_src_adj_activate (fib_entry_src_t *src,
      * there should always be a cover, though it may be the default route.
      */
     src->adj.fesa_cover = fib_table_get_less_specific(fib_entry->fe_fib_index,
-						      &fib_entry->fe_prefix);
+                                                      &fib_entry->fe_prefix);
 
     ASSERT(FIB_NODE_INDEX_INVALID != src->adj.fesa_cover);
     ASSERT(fib_entry_get_index(fib_entry) != src->adj.fesa_cover);
@@ -71,8 +226,8 @@ fib_entry_src_adj_activate (fib_entry_src_t *src,
     ASSERT(cover != fib_entry);
 
     src->adj.fesa_sibling =
-	fib_entry_cover_track(cover,
-			      fib_entry_get_index(fib_entry));
+        fib_entry_cover_track(cover,
+                              fib_entry_get_index(fib_entry));
 
     /*
      * if the cover is attached on the same interface as this adj source then
@@ -88,40 +243,31 @@ fib_entry_src_adj_activate (fib_entry_src_t *src,
      */
     if (FIB_ENTRY_FLAG_ATTACHED & fib_entry_get_flags_i(cover))
     {
-        u32 cover_itf = fib_entry_get_resolving_interface(src->adj.fesa_cover);
-        u32 adj_itf = fib_path_list_get_resolving_interface(src->fes_pl);
-
-        if (cover_itf == adj_itf)
-        {
-            return (1);
-        }
-        else
-        {
-            /*
-             * if the interface the adj is on is unnumbered to the
-             * cover's, then allow that too.
-             */
-            vnet_sw_interface_t *swif;
-
-            swif = vnet_get_sw_interface (vnet_get_main(), adj_itf);
-
-            if (swif->flags & VNET_SW_INTERFACE_FLAG_UNNUMBERED &&
-                cover_itf == swif->unnumbered_sw_if_index)
-            {
-                return (1);
-            }
-        }
+        fib_entry_src_path_list_walk_cxt_t ctx = {
+            .cover_itf = fib_entry_get_resolving_interface(src->adj.fesa_cover),
+            .flags = FIB_PATH_EXT_ADJ_FLAG_NONE,
+            .src = src,
+        };
+
+        fib_path_list_walk(src->fes_pl,
+                           fib_entry_src_adj_path_list_walk,
+                           &ctx);
+
+        /*
+         * active the entry is one of the paths refines the cover.
+         */
+        return (FIB_PATH_EXT_ADJ_FLAG_REFINES_COVER & ctx.flags);
     }
     return (0);
 }
 
 /*
- * Source Deactivate. 
+ * Source Deactivate.
  * Called when the source is no longer best source on the entry
  */
 static void
 fib_entry_src_adj_deactivate (fib_entry_src_t *src,
-			      const fib_entry_t *fib_entry)
+                              const fib_entry_t *fib_entry)
 {
     fib_entry_t *cover;
 
@@ -143,14 +289,14 @@ fib_entry_src_adj_deactivate (fib_entry_src_t *src,
 
 static u8*
 fib_entry_src_adj_format (fib_entry_src_t *src,
-			 u8* s)
+                         u8* s)
 {
     return (format(s, "cover:%d", src->adj.fesa_cover));
 }
 
 static void
 fib_entry_src_adj_installed (fib_entry_src_t *src,
-			     const fib_entry_t *fib_entry)
+                             const fib_entry_t *fib_entry)
 {
     /*
      * The adj source now rules! poke our cover to get exported
@@ -161,16 +307,16 @@ fib_entry_src_adj_installed (fib_entry_src_t *src,
     cover = fib_entry_get(src->adj.fesa_cover);
 
     fib_attached_export_covered_added(cover,
-				      fib_entry_get_index(fib_entry));
+                                      fib_entry_get_index(fib_entry));
 }
 
 static fib_entry_src_cover_res_t
 fib_entry_src_adj_cover_change (fib_entry_src_t *src,
-				const fib_entry_t *fib_entry)
+                                const fib_entry_t *fib_entry)
 {
     fib_entry_src_cover_res_t res = {
-	.install = !0,
-	.bw_reason = FIB_NODE_BW_REASON_FLAG_NONE,
+        .install = !0,
+        .bw_reason = FIB_NODE_BW_REASON_FLAG_NONE,
     };
 
     fib_entry_src_adj_deactivate(src, fib_entry);
@@ -178,10 +324,10 @@ fib_entry_src_adj_cover_change (fib_entry_src_t *src,
     res.install = fib_entry_src_adj_activate(src, fib_entry);
 
     if (res.install) {
-	/*
-	 * ADJ fib can install
-	 */
-	res.bw_reason = FIB_NODE_BW_REASON_FLAG_EVALUATE;
+        /*
+         * ADJ fib can install
+         */
+        res.bw_reason = FIB_NODE_BW_REASON_FLAG_EVALUATE;
     }
 
     return (res);
@@ -196,12 +342,12 @@ fib_entry_src_adj_cover_update (fib_entry_src_t *src,
 {
     /*
      * the cover has updated, i.e. its forwarding or flags
-     * have changed. do'nt decativate/activate here, since this
+     * have changed. don't decativate/activate here, since this
      * prefix is updated during the covers walk.
      */
     fib_entry_src_cover_res_t res = {
-	.install = !0,
-	.bw_reason = FIB_NODE_BW_REASON_FLAG_NONE,
+        .install = !0,
+        .bw_reason = FIB_NODE_BW_REASON_FLAG_NONE,
     };
     fib_entry_t *cover;
 
@@ -217,6 +363,8 @@ fib_entry_src_adj_cover_update (fib_entry_src_t *src,
 const static fib_entry_src_vft_t adj_src_vft = {
     .fesv_init = fib_entry_src_adj_init,
     .fesv_path_swap = fib_entry_src_adj_path_swap,
+    .fesv_path_add = fib_entry_src_adj_path_add,
+    .fesv_path_remove = fib_entry_src_adj_path_remove,
     .fesv_remove = fib_entry_src_adj_remove,
     .fesv_activate = fib_entry_src_adj_activate,
     .fesv_deactivate = fib_entry_src_adj_deactivate,