From c2ac2357fb0ff598ca1cb650a5766a552e017833 Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Tue, 2 Jul 2019 14:33:29 +0000 Subject: 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 --- extras/vom/vom/route_cmds.cpp | 3 +- src/vnet/fib/fib_api.c | 64 +++++++++++++++++++++++++------------------ src/vnet/fib/fib_api.h | 12 ++++---- src/vnet/ip/ip_api.c | 15 ++++------ src/vnet/mpls/mpls_api.c | 14 +++++----- test/vpp_ip_route.py | 30 ++++++++++++++------ 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,10 +97,11 @@ 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); 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, -- cgit 1.2.3-korg