fib: allow route delete with no paths and multipath=0 to remove the

whole route

Type: fix
Fixes: 097fa66b

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

6 files changed, 79 insertions, 59 deletions

diff --git a/extras/vom/vom/route_cmds.cpp b/extras/vom/vom/route_cmds.cpp
index fb1fc2f9933..78676c225be 100644
--- a/extras/vom/vom/route_cmds.cpp
+++ b/extras/vom/vom/route_cmds.cpp
@@ -97,11 +97,12 @@ delete_cmd::issue(connection& con)
   msg_t req(con.ctx(), 0, std::ref(*this));
 
   auto& payload = req.get_request().get_payload();
-  payload.route.table_id = m_id;
   payload.is_add = 0;
   payload.is_multipath = 0;
 
   payload.route.table_id = m_id;
+  payload.route.n_paths = 0;
+  payload.route.table_id = m_id;
   payload.route.prefix = to_api(m_prefix);
 
   VAPI_CALL(req.execute());
diff --git a/src/vnet/fib/fib_api.c b/src/vnet/fib/fib_api.c
index 5aa5c4ec875..e776235bc00 100644
--- a/src/vnet/fib/fib_api.c
+++ b/src/vnet/fib/fib_api.c
@@ -449,7 +449,7 @@ fib_api_path_encode (const fib_route_path_t * rpath,
     }
 }
 
-void
+int
 fib_api_route_add_del (u8 is_add,
                        u8 is_multipath,
                        u32 fib_index,
@@ -457,36 +457,46 @@ fib_api_route_add_del (u8 is_add,
                        fib_entry_flag_t entry_flags,
                        fib_route_path_t *rpaths)
 {
-  if (is_multipath)
+    if (is_multipath)
     {
-      /* Iterative path add/remove */
-      if (is_add)
-	fib_table_entry_path_add2 (fib_index,
-				   prefix,
-				   FIB_SOURCE_API,
-                                   entry_flags,
-                                   rpaths);
-      else
-	fib_table_entry_path_remove2 (fib_index,
-				      prefix,
-                                      FIB_SOURCE_API,
-                                      rpaths);
+        if (vec_len(rpaths) == 0)
+            return (VNET_API_ERROR_NO_PATHS_IN_ROUTE);
+
+        /* Iterative path add/remove */
+        if (is_add)
+            fib_table_entry_path_add2 (fib_index,
+                                       prefix,
+                                       FIB_SOURCE_API,
+                                       entry_flags,
+                                       rpaths);
+        else
+            fib_table_entry_path_remove2 (fib_index,
+                                          prefix,
+                                          FIB_SOURCE_API,
+                                          rpaths);
     }
-  else
+    else
     {
-      if (is_add)
-        /* path replacement */
-	fib_table_entry_update (fib_index,
-                                prefix,
-                                FIB_SOURCE_API,
-                                entry_flags,
-                                rpaths);
-      else
-        /* entry delete */
-	fib_table_entry_delete (fib_index,
-                                prefix,
-                                FIB_SOURCE_API);
+        if (is_add)
+        {
+            if (vec_len(rpaths) == 0)
+                return (VNET_API_ERROR_NO_PATHS_IN_ROUTE);
+
+            /* path replacement */
+            fib_table_entry_update (fib_index,
+                                    prefix,
+                                    FIB_SOURCE_API,
+                                    entry_flags,
+                                    rpaths);
+        }
+        else
+            /* entry delete */
+            fib_table_entry_delete (fib_index,
+                                    prefix,
+                                    FIB_SOURCE_API);
     }
+
+    return (0);
 }
 
 u8*
diff --git a/src/vnet/fib/fib_api.h b/src/vnet/fib/fib_api.h
index ffff2289b37..27335211df7 100644
--- a/src/vnet/fib/fib_api.h
+++ b/src/vnet/fib/fib_api.h
@@ -40,12 +40,12 @@ extern int fib_api_table_id_decode(fib_protocol_t fproto,
 /**
  * Adding routes from the API
  */
-extern void fib_api_route_add_del (u8 is_add,
-                                   u8 is_multipath,
-                                   u32 fib_index,
-                                   const fib_prefix_t * prefix,
-                                   fib_entry_flag_t entry_flags,
-                                   fib_route_path_t *rpaths);
+extern int fib_api_route_add_del (u8 is_add,
+                                  u8 is_multipath,
+                                  u32 fib_index,
+                                  const fib_prefix_t * prefix,
+                                  fib_entry_flag_t entry_flags,
+                                  fib_route_path_t *rpaths);
 
 extern u8* format_vl_api_fib_path(u8 * s, va_list * args);
 
diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c
index 4d2f0704ca3..bcb53888621 100644
--- a/src/vnet/ip/ip_api.c
+++ b/src/vnet/ip/ip_api.c
@@ -667,13 +667,8 @@ ip_route_add_del_t_handler (vl_api_ip_route_add_del_t * mp, u32 * stats_index)
   if (0 != rv)
     goto out;
 
-  if (0 == mp->route.n_paths)
-    {
-      rv = VNET_API_ERROR_NO_PATHS_IN_ROUTE;
-      goto out;
-    }
-
-  vec_validate (rpaths, mp->route.n_paths - 1);
+  if (0 != mp->route.n_paths)
+    vec_validate (rpaths, mp->route.n_paths - 1);
 
   for (ii = 0; ii < mp->route.n_paths; ii++)
     {
@@ -690,9 +685,9 @@ ip_route_add_del_t_handler (vl_api_ip_route_add_del_t * mp, u32 * stats_index)
 	goto out;
     }
 
-  fib_api_route_add_del (mp->is_add,
-			 mp->is_multipath,
-			 fib_index, &pfx, entry_flags, rpaths);
+  rv = fib_api_route_add_del (mp->is_add,
+			      mp->is_multipath,
+			      fib_index, &pfx, entry_flags, rpaths);
 
   if (mp->is_add && 0 == rv)
     *stats_index = fib_table_entry_get_stats_index (fib_index, &pfx);
diff --git a/src/vnet/mpls/mpls_api.c b/src/vnet/mpls/mpls_api.c
index cb20df5695b..530ceec31c4 100644
--- a/src/vnet/mpls/mpls_api.c
+++ b/src/vnet/mpls/mpls_api.c
@@ -192,13 +192,13 @@ mpls_route_add_del_t_handler (vnet_main_t * vnm,
 	goto out;
     }
 
-  fib_api_route_add_del (mp->mr_is_add,
-			 mp->mr_is_multipath,
-			 fib_index,
-			 &pfx,
-			 (mp->mr_route.mr_is_multicast ?
-			  FIB_ENTRY_FLAG_MULTICAST :
-			  FIB_ENTRY_FLAG_NONE), rpaths);
+  rv = fib_api_route_add_del (mp->mr_is_add,
+			      mp->mr_is_multipath,
+			      fib_index,
+			      &pfx,
+			      (mp->mr_route.mr_is_multicast ?
+			       FIB_ENTRY_FLAG_MULTICAST :
+			       FIB_ENTRY_FLAG_NONE), rpaths);
 
   if (mp->mr_is_add && 0 == rv)
     *stats_index = fib_table_entry_get_stats_index (fib_index, &pfx);
diff --git a/test/vpp_ip_route.py b/test/vpp_ip_route.py
index 031415e9ee0..864a39aeb34 100644
--- a/test/vpp_ip_route.py
+++ b/test/vpp_ip_route.py
@@ -428,6 +428,7 @@ class VppIpRoute(VppObject):
         self.prefix = VppIpPrefix(dest_addr, dest_addr_len)
         self.register = register
         self.stats_index = None
+        self.modified = False
 
         self.encoded_paths = []
         for path in self.paths:
@@ -444,6 +445,7 @@ class VppIpRoute(VppObject):
         self.encoded_paths = []
         for path in self.paths:
             self.encoded_paths.append(path.encode())
+        self.modified = True
 
         self._test.vapi.ip_route_add_del(route={'table_id': self.table_id,
                                                 'prefix': self.prefix.encode(),
@@ -468,14 +470,26 @@ class VppIpRoute(VppObject):
             self._test.registry.register(self, self._test.logger)
 
     def remove_vpp_config(self):
-        self._test.vapi.ip_route_add_del(route={'table_id': self.table_id,
-                                                'prefix': self.prefix.encode(),
-                                                'n_paths': len(
-                                                    self.encoded_paths),
-                                                'paths': self.encoded_paths,
-                                                },
-                                         is_add=0,
-                                         is_multipath=0)
+        # there's no need to issue different deletes for modified routes
+        # we do this only to test the two different ways to delete routes
+        # eiter by passing all the paths to remove and mutlipath=1 or
+        # passing no paths and multipath=0
+        if self.modified:
+            self._test.vapi.ip_route_add_del(
+                route={'table_id': self.table_id,
+                       'prefix': self.prefix.encode(),
+                       'n_paths': len(
+                           self.encoded_paths),
+                       'paths': self.encoded_paths},
+                is_add=0,
+                is_multipath=1)
+        else:
+            self._test.vapi.ip_route_add_del(
+                route={'table_id': self.table_id,
+                       'prefix': self.prefix.encode(),
+                       'n_paths': 0},
+                is_add=0,
+                is_multipath=0)
 
     def query_vpp_config(self):
         return find_route(self._test,