linux-cp: Fix add vs update on routes

Linux uses NLM_F_REPLACE in the netlink message to signal a FIB update
The code invariably does a FIB update for IPv4 and a addition for IPv6.
Without this fix, the following:
 ip route add 2001:db8::/48 via 2001:db8::1
 ip route replace 2001:db8::/48 via 2001:db8::2

ends up as two separate FIB entries in VPP. With the fix, there will be one FIB entry (the second one with nexthop ::2).

Type: fix
Change-Id: I8f98d6ded52ae0c60bfddaa7fc39acbbaa19d34a
Signed-off-by: Pim van Pelt <pim@ipng.nl>

3 files changed, 69 insertions, 58 deletions

diff --git a/src/plugins/linux-cp/lcp_nl.c b/src/plugins/linux-cp/lcp_nl.c
index 8f2bffd6e99..b4fef7e0b40 100644
--- a/src/plugins/linux-cp/lcp_nl.c
+++ b/src/plugins/linux-cp/lcp_nl.c
@@ -205,7 +205,10 @@ nl_route_del (struct rtnl_route *rr, void *arg)
 static void
 nl_route_add (struct rtnl_route *rr, void *arg)
 {
-  FOREACH_VFT (nvl_rt_route_add, rr);
+  nl_msg_info_t *msg_info = (nl_msg_info_t *) arg;
+  struct nlmsghdr *nlh = nlmsg_hdr (msg_info->msg);
+
+  FOREACH_VFT_CTX (nvl_rt_route_add, rr, (nlh->nlmsg_flags & NLM_F_REPLACE));
 }
 
 static void
diff --git a/src/plugins/linux-cp/lcp_nl.h b/src/plugins/linux-cp/lcp_nl.h
index 7b2fccc29cd..41757e9b983 100644
--- a/src/plugins/linux-cp/lcp_nl.h
+++ b/src/plugins/linux-cp/lcp_nl.h
@@ -26,7 +26,8 @@ typedef void (*nl_rt_addr_cb_t) (struct rtnl_addr *ra);
 typedef void (*nl_rt_addr_sync_cb_t) (void);
 typedef void (*nl_rt_neigh_cb_t) (struct rtnl_neigh *rr);
 typedef void (*nl_rt_neigh_sync_cb_t) (void);
-typedef void (*nl_rt_route_cb_t) (struct rtnl_route *rn);
+typedef void (*nl_rt_route_add_cb_t) (struct rtnl_route *rn, int is_replace);
+typedef void (*nl_rt_route_del_cb_t) (struct rtnl_route *rn);
 typedef void (*nl_rt_route_sync_cb_t) (void);
 
 #define NL_RT_COMMON uword is_mp_safe
@@ -73,12 +74,19 @@ typedef struct nl_rt_neigh_sync_t_
   nl_rt_neigh_sync_cb_t cb;
 } nl_rt_neigh_sync_t;
 
-typedef struct nl_rt_route_t_
+typedef struct nl_rt_route_add_t_
 {
   NL_RT_COMMON;
 
-  nl_rt_route_cb_t cb;
-} nl_rt_route_t;
+  nl_rt_route_add_cb_t cb;
+} nl_rt_route_add_t;
+
+typedef struct nl_rt_route_del_t_
+{
+  NL_RT_COMMON;
+
+  nl_rt_route_del_cb_t cb;
+} nl_rt_route_del_t;
 
 typedef struct nl_rt_route_sync_t_
 {
@@ -103,8 +111,8 @@ typedef struct nl_vft_t_
   nl_rt_neigh_t nvl_rt_neigh_del;
   nl_rt_neigh_sync_t nvl_rt_neigh_sync_begin;
   nl_rt_neigh_sync_t nvl_rt_neigh_sync_end;
-  nl_rt_route_t nvl_rt_route_add;
-  nl_rt_route_t nvl_rt_route_del;
+  nl_rt_route_add_t nvl_rt_route_add;
+  nl_rt_route_del_t nvl_rt_route_del;
   nl_rt_route_sync_t nvl_rt_route_sync_begin;
   nl_rt_route_sync_t nvl_rt_route_sync_end;
 } nl_vft_t;
diff --git a/src/plugins/linux-cp/lcp_router.c b/src/plugins/linux-cp/lcp_router.c
index ad701c5ceda..88b7c537afc 100644
--- a/src/plugins/linux-cp/lcp_router.c
+++ b/src/plugins/linux-cp/lcp_router.c
@@ -1184,7 +1184,7 @@ lcp_router_route_del (struct rtnl_route *rr)
 }
 
 static void
-lcp_router_route_add (struct rtnl_route *rr)
+lcp_router_route_add (struct rtnl_route *rr, int is_replace)
 {
   fib_entry_flag_t entry_flags;
   uint32_t table_id;
@@ -1213,71 +1213,71 @@ lcp_router_route_add (struct rtnl_route *rr)
       LCP_ROUTER_DBG ("route skip: %d:%U %U", rtnl_route_get_table (rr),
 		      format_fib_prefix, &pfx, format_fib_entry_flags,
 		      entry_flags);
+      return;
     }
-  else
-    {
-      LCP_ROUTER_DBG ("route add: %d:%U %U", rtnl_route_get_table (rr),
-		      format_fib_prefix, &pfx, format_fib_entry_flags,
-		      entry_flags);
+  LCP_ROUTER_DBG ("route %s: %d:%U %U", is_replace ? "replace" : "add",
+		  rtnl_route_get_table (rr), format_fib_prefix, &pfx,
+		  format_fib_entry_flags, entry_flags);
 
-      lcp_router_route_path_parse_t np = {
-	.route_proto = pfx.fp_proto,
-	.is_mcast = (rtype == RTN_MULTICAST),
-	.type_flags = lcp_router_route_type_frpflags[rtype],
-	.preference = (u8) rtnl_route_get_priority (rr),
-      };
+  lcp_router_route_path_parse_t np = {
+    .route_proto = pfx.fp_proto,
+    .is_mcast = (rtype == RTN_MULTICAST),
+    .type_flags = lcp_router_route_type_frpflags[rtype],
+    .preference = (u8) rtnl_route_get_priority (rr),
+  };
 
-      rtnl_route_foreach_nexthop (rr, lcp_router_route_path_parse, &np);
-      lcp_router_route_path_add_special (rr, &np);
+  rtnl_route_foreach_nexthop (rr, lcp_router_route_path_parse, &np);
+  lcp_router_route_path_add_special (rr, &np);
 
-      if (0 != vec_len (np.paths))
+  if (0 != vec_len (np.paths))
+    {
+      if (rtype == RTN_MULTICAST)
 	{
-	  if (rtype == RTN_MULTICAST)
-	    {
-	      /* it's not clear to me how linux expresses the RPF paramters
-	       * so we'll allow from all interfaces and hope for the best */
-	      mfib_prefix_t mpfx = {};
+	  /* it's not clear to me how linux expresses the RPF paramters
+	   * so we'll allow from all interfaces and hope for the best */
+	  mfib_prefix_t mpfx = {};
 
-	      lcp_router_route_mk_mprefix (rr, &mpfx);
+	  lcp_router_route_mk_mprefix (rr, &mpfx);
 
-	      mfib_table_entry_update (
-		nlt->nlt_mfib_index, &mpfx, MFIB_SOURCE_PLUGIN_LOW,
-		MFIB_RPF_ID_NONE, MFIB_ENTRY_FLAG_ACCEPT_ALL_ITF);
+	  mfib_table_entry_update (nlt->nlt_mfib_index, &mpfx,
+				   MFIB_SOURCE_PLUGIN_LOW, MFIB_RPF_ID_NONE,
+				   MFIB_ENTRY_FLAG_ACCEPT_ALL_ITF);
 
-	      mfib_table_entry_paths_update (nlt->nlt_mfib_index, &mpfx,
-					     MFIB_SOURCE_PLUGIN_LOW,
-					     MFIB_ENTRY_FLAG_NONE, np.paths);
-	    }
-	  else
-	    {
-	      fib_source_t fib_src;
-	      const fib_route_path_t *rpath;
+	  mfib_table_entry_paths_update (nlt->nlt_mfib_index, &mpfx,
+					 MFIB_SOURCE_PLUGIN_LOW,
+					 MFIB_ENTRY_FLAG_NONE, np.paths);
+	}
+      else
+	{
+	  fib_source_t fib_src;
+	  const fib_route_path_t *rpath;
 
-	      vec_foreach (rpath, np.paths)
+	  vec_foreach (rpath, np.paths)
+	    {
+	      if (fib_route_path_is_attached (rpath))
 		{
-		  if (fib_route_path_is_attached (rpath))
-		    {
-		      entry_flags |= FIB_ENTRY_FLAG_ATTACHED;
-		      break;
-		    }
+		  entry_flags |= FIB_ENTRY_FLAG_ATTACHED;
+		  break;
 		}
+	    }
 
-	      fib_src = lcp_router_proto_fib_source (rproto);
+	  fib_src = lcp_router_proto_fib_source (rproto);
 
-	      if (pfx.fp_proto == FIB_PROTOCOL_IP6)
-		fib_table_entry_path_add2 (nlt->nlt_fib_index, &pfx, fib_src,
-					   entry_flags, np.paths);
-	      else
-		fib_table_entry_update (nlt->nlt_fib_index, &pfx, fib_src,
-					entry_flags, np.paths);
-	    }
+	  if (is_replace)
+	    fib_table_entry_update (nlt->nlt_fib_index, &pfx, fib_src,
+				    entry_flags, np.paths);
+	  else
+	    fib_table_entry_path_add2 (nlt->nlt_fib_index, &pfx, fib_src,
+				       entry_flags, np.paths);
 	}
-      else
-	LCP_ROUTER_DBG ("no paths for route add: %d:%U %U",
-			rtnl_route_get_table (rr), format_fib_prefix, &pfx,
-			format_fib_entry_flags, entry_flags);
-      vec_free (np.paths);
     }
+  else
+    {
+      LCP_ROUTER_DBG ("no paths for route: %d:%U %U",
+		      rtnl_route_get_table (rr), format_fib_prefix, &pfx,
+		      format_fib_entry_flags, entry_flags);
+    }
+  vec_free (np.paths);
 }
 
 static void