From 50bd1d3e256154212198a31932896b07af5b129f Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Fri, 8 Oct 2021 07:16:12 +0000 Subject: [PATCH] fib: re-evaluate the import/export state of a prefix. Type: fix re-evaluate the import/export state of a prefix when the interface it is attached to rebinds to a different table. Only attached routes have import/export requirements, so we can back walk from the glean adjacency when the interface rebinds tables. There are two cases to consider. 1. the rebind may change the prefix from/to import 2. the import VRF may change Signed-off-by: Neale Ranns Change-Id: I25b6af43b3b2d8f701dfbe7a08710dc56b3f5778 --- src/vnet/adj/adj_glean.c | 60 +++++++++++- src/vnet/fib/fib_entry.c | 214 ++++++++++++++++++++++++++++--------------- src/vnet/fib/fib_entry_src.c | 2 +- src/vnet/fib/fib_node.h | 21 ++++- src/vnet/fib/fib_path.c | 5 + test/test_ip4.py | 135 +++++++++++++++++++++++++++ 6 files changed, 360 insertions(+), 77 deletions(-) diff --git a/src/vnet/adj/adj_glean.c b/src/vnet/adj/adj_glean.c index e8ca043662f..45477649c1a 100644 --- a/src/vnet/adj/adj_glean.c +++ b/src/vnet/adj/adj_glean.c @@ -424,11 +424,59 @@ VNET_SW_INTERFACE_ADD_DEL_FUNCTION(adj_glean_interface_delete); */ static void adj_glean_ethernet_change_mac (ethernet_main_t * em, - u32 sw_if_index, uword opaque) + u32 sw_if_index, + uword opaque) { adj_glean_walk (sw_if_index, adj_glean_update_rewrite_walk, NULL); } +static void +adj_glean_table_bind (fib_protocol_t fproto, + u32 sw_if_index, + u32 itf_fib_index) +{ + /* + * for each glean on the interface trigger a walk back to the children + */ + fib_node_back_walk_ctx_t bw_ctx = { + .fnbw_reason = FIB_NODE_BW_REASON_FLAG_INTERFACE_BIND, + .interface_bind = { + .fnbw_to_fib_index = itf_fib_index, + }, + }; + + adj_glean_walk (sw_if_index, adj_glean_start_backwalk, &bw_ctx); +} + + +/** + * Callback function invoked when an interface's IPv6 Table + * binding changes + */ +static void +adj_glean_ip6_table_bind (ip6_main_t * im, + uword opaque, + u32 sw_if_index, + u32 new_fib_index, + u32 old_fib_index) +{ + adj_glean_table_bind (FIB_PROTOCOL_IP6, sw_if_index, new_fib_index); +} + +/** + * Callback function invoked when an interface's IPv4 Table + * binding changes + */ +static void +adj_glean_ip4_table_bind (ip4_main_t * im, + uword opaque, + u32 sw_if_index, + u32 new_fib_index, + u32 old_fib_index) +{ + adj_glean_table_bind (FIB_PROTOCOL_IP4, sw_if_index, new_fib_index); +} + u8* format_adj_glean (u8* s, va_list *ap) { @@ -519,4 +567,14 @@ adj_glean_module_init (void) .function_opaque = 0, }; vec_add1 (ethernet_main.address_change_callbacks, ctx); + + ip6_table_bind_callback_t cbt6 = { + .function = adj_glean_ip6_table_bind, + }; + vec_add1 (ip6_main.table_bind_callbacks, cbt6); + + ip4_table_bind_callback_t cbt4 = { + .function = adj_glean_ip4_table_bind, + }; + vec_add1 (ip4_main.table_bind_callbacks, cbt4); } diff --git a/src/vnet/fib/fib_entry.c b/src/vnet/fib/fib_entry.c index dfa0cb285b4..6f579696f29 100644 --- a/src/vnet/fib/fib_entry.c +++ b/src/vnet/fib/fib_entry.c @@ -293,58 +293,6 @@ fib_entry_get_flags (fib_node_index_t fib_entry_index) return (fib_entry_get_flags_i(fib_entry_get(fib_entry_index))); } -/* - * fib_entry_back_walk_notify - * - * A back walk has reach this entry. - */ -static fib_node_back_walk_rc_t -fib_entry_back_walk_notify (fib_node_t *node, - fib_node_back_walk_ctx_t *ctx) -{ - fib_entry_t *fib_entry; - - fib_entry = fib_entry_from_fib_node(node); - - if (FIB_NODE_BW_REASON_FLAG_EVALUATE & ctx->fnbw_reason || - FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE & ctx->fnbw_reason || - FIB_NODE_BW_REASON_FLAG_ADJ_DOWN & ctx->fnbw_reason || - FIB_NODE_BW_REASON_FLAG_INTERFACE_UP & ctx->fnbw_reason || - FIB_NODE_BW_REASON_FLAG_INTERFACE_DOWN & ctx->fnbw_reason || - FIB_NODE_BW_REASON_FLAG_INTERFACE_DELETE & ctx->fnbw_reason) - { - fib_entry_src_action_reactivate(fib_entry, - fib_entry_get_best_source( - fib_entry_get_index(fib_entry))); - } - - /* - * all other walk types can be reclassifed to a re-evaluate to - * all recursive dependents. - * By reclassifying we ensure that should any of these walk types meet - * they can be merged. - */ - ctx->fnbw_reason = FIB_NODE_BW_REASON_FLAG_EVALUATE; - - /* - * ... and nothing is forced sync from now on. - */ - ctx->fnbw_flags &= ~FIB_NODE_BW_FLAG_FORCE_SYNC; - - FIB_ENTRY_DBG(fib_entry, "bw:%U", - format_fib_node_bw_reason, ctx->fnbw_reason); - - /* - * propagate the backwalk further if we haven't already reached the - * maximum depth. - */ - fib_walk_sync(FIB_NODE_TYPE_ENTRY, - fib_entry_get_index(fib_entry), - ctx); - - return (FIB_NODE_BACK_WALK_CONTINUE); -} - static void fib_entry_show_memory (void) { @@ -373,16 +321,6 @@ fib_entry_show_memory (void) sizeof(fib_path_ext_t)); } -/* - * The FIB path-list's graph node virtual function table - */ -static const fib_node_vft_t fib_entry_vft = { - .fnv_get = fib_entry_get_node, - .fnv_last_lock = fib_entry_last_lock_gone, - .fnv_back_walk = fib_entry_back_walk_notify, - .fnv_mem_show = fib_entry_show_memory, -}; - /** * @brief Contribute the set of Adjacencies that this entry forwards with * to build the uRPF list of its children @@ -645,7 +583,8 @@ fib_entry_alloc (u32 fib_index, static fib_entry_t* fib_entry_post_flag_update_actions (fib_entry_t *fib_entry, - fib_entry_flag_t old_flags) + fib_entry_flag_t old_flags, + u32 new_fib_index) { fib_node_index_t fei; @@ -670,12 +609,14 @@ fib_entry_post_flag_update_actions (fib_entry_t *fib_entry, * there is an assumption here that the entry resolves via only * one interface and that it is the cross VRF interface. */ - u32 sw_if_index = fib_path_list_get_resolving_interface(fib_entry->fe_parent); - - fib_attached_export_import(fib_entry, - fib_table_get_index_for_sw_if_index( - fib_entry_get_proto(fib_entry), - sw_if_index)); + if (~0 == new_fib_index) + { + u32 sw_if_index = fib_path_list_get_resolving_interface(fib_entry->fe_parent); + new_fib_index = fib_table_get_index_for_sw_if_index( + fib_entry_get_proto(fib_entry), + sw_if_index); + } + fib_attached_export_import(fib_entry, new_fib_index); } else if (was_import && !is_import) { @@ -684,6 +625,14 @@ fib_entry_post_flag_update_actions (fib_entry_t *fib_entry, */ fib_attached_export_purge(fib_entry); } + else if (was_import && is_import && ~0 != new_fib_index) + { + /* + * transition from export from one table to another + */ + fib_attached_export_purge(fib_entry); + fib_attached_export_import(fib_entry, new_fib_index); + } /* * else * no change. nothing to do. @@ -717,8 +666,7 @@ fib_entry_post_install_actions (fib_entry_t *fib_entry, fib_source_t source, fib_entry_flag_t old_flags) { - fib_entry = fib_entry_post_flag_update_actions(fib_entry, - old_flags); + fib_entry = fib_entry_post_flag_update_actions(fib_entry, old_flags, ~0); fib_entry = fib_entry_src_action_installed(fib_entry, source); return (fib_entry); @@ -990,7 +938,7 @@ fib_entry_source_removed (fib_entry_t *fib_entry, /* * no more sources left. this entry is toast. */ - fib_entry = fib_entry_post_flag_update_actions(fib_entry, old_flags); + fib_entry = fib_entry_post_flag_update_actions(fib_entry, old_flags, ~0); fib_entry_src_action_uninstall(fib_entry); return (FIB_ENTRY_SRC_FLAG_NONE); @@ -1164,7 +1112,7 @@ fib_entry_special_remove (fib_node_index_t fib_entry_index, /* * no more sources left. this entry is toast. */ - fib_entry = fib_entry_post_flag_update_actions(fib_entry, bflags); + fib_entry = fib_entry_post_flag_update_actions(fib_entry, bflags, ~0); fib_entry_src_action_uninstall(fib_entry); return (FIB_ENTRY_SRC_FLAG_NONE); } @@ -1481,6 +1429,126 @@ fib_entry_recursive_loop_detect (fib_node_index_t entry_index, return (is_looped); } +/* + * fib_entry_attached_cross_table + * + * Return true if the route is attached via an interface that + * is not in the same table as the route + */ +static int +fib_entry_attached_cross_table (const fib_entry_t *fib_entry, + u32 fib_index) +{ + const fib_prefix_t *pfx = &fib_entry->fe_prefix; + + switch (pfx->fp_proto) + { + case FIB_PROTOCOL_MPLS: + /* MPLS routes are never imported/exported */ + return (0); + case FIB_PROTOCOL_IP6: + /* Ignore link local addresses these also can't be imported/exported */ + if (ip6_address_is_link_local_unicast (&pfx->fp_addr.ip6)) + { + return (0); + } + break; + case FIB_PROTOCOL_IP4: + break; + } + + return (fib_entry->fe_fib_index != fib_index); +} + +/* + * fib_entry_back_walk_notify + * + * A back walk has reach this entry. + */ +static fib_node_back_walk_rc_t +fib_entry_back_walk_notify (fib_node_t *node, + fib_node_back_walk_ctx_t *ctx) +{ + fib_source_t best_source; + fib_entry_t *fib_entry; + fib_entry_src_t *bsrc; + + fib_entry = fib_entry_from_fib_node(node); + bsrc = fib_entry_get_best_src_i(fib_entry); + best_source = fib_entry_src_get_source(bsrc); + + if (FIB_NODE_BW_REASON_FLAG_INTERFACE_BIND & ctx->fnbw_reason) + { + fib_entry_flag_t bflags; + + bflags = fib_entry_src_get_flags(bsrc); + + fib_entry_src_action_reactivate(fib_entry, best_source); + + /* re-evaluate whether the prefix is cross table */ + if (fib_entry_attached_cross_table( + fib_entry, ctx->interface_bind.fnbw_to_fib_index) && + !(bsrc->fes_entry_flags & FIB_ENTRY_FLAG_NO_ATTACHED_EXPORT)) + { + bsrc->fes_entry_flags |= FIB_ENTRY_FLAG_IMPORT; + } + else + { + bsrc->fes_entry_flags &= ~FIB_ENTRY_FLAG_IMPORT; + } + + fib_entry = fib_entry_post_flag_update_actions( + fib_entry, bflags, + ctx->interface_bind.fnbw_to_fib_index); + } + else if (FIB_NODE_BW_REASON_FLAG_EVALUATE & ctx->fnbw_reason || + FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE & ctx->fnbw_reason || + FIB_NODE_BW_REASON_FLAG_ADJ_DOWN & ctx->fnbw_reason || + FIB_NODE_BW_REASON_FLAG_INTERFACE_UP & ctx->fnbw_reason || + FIB_NODE_BW_REASON_FLAG_INTERFACE_DOWN & ctx->fnbw_reason || + FIB_NODE_BW_REASON_FLAG_INTERFACE_BIND & ctx->fnbw_reason || + FIB_NODE_BW_REASON_FLAG_INTERFACE_DELETE & ctx->fnbw_reason) + { + fib_entry_src_action_reactivate(fib_entry, best_source); + } + + /* + * all other walk types can be reclassifed to a re-evaluate to + * all recursive dependents. + * By reclassifying we ensure that should any of these walk types meet + * they can be merged. + */ + ctx->fnbw_reason = FIB_NODE_BW_REASON_FLAG_EVALUATE; + + /* + * ... and nothing is forced sync from now on. + */ + ctx->fnbw_flags &= ~FIB_NODE_BW_FLAG_FORCE_SYNC; + + FIB_ENTRY_DBG(fib_entry, "bw:%U", + format_fib_node_bw_reason, ctx->fnbw_reason); + + /* + * propagate the backwalk further if we haven't already reached the + * maximum depth. + */ + fib_walk_sync(FIB_NODE_TYPE_ENTRY, + fib_entry_get_index(fib_entry), + ctx); + + return (FIB_NODE_BACK_WALK_CONTINUE); +} + +/* + * The FIB path-list's graph node virtual function table + */ +static const fib_node_vft_t fib_entry_vft = { + .fnv_get = fib_entry_get_node, + .fnv_last_lock = fib_entry_last_lock_gone, + .fnv_back_walk = fib_entry_back_walk_notify, + .fnv_mem_show = fib_entry_show_memory, +}; + u32 fib_entry_get_resolving_interface (fib_node_index_t entry_index) { diff --git a/src/vnet/fib/fib_entry_src.c b/src/vnet/fib/fib_entry_src.c index a4a4f1ae0b5..388f08c621f 100644 --- a/src/vnet/fib/fib_entry_src.c +++ b/src/vnet/fib/fib_entry_src.c @@ -1493,7 +1493,7 @@ fib_entry_src_action_remove (fib_entry_t *fib_entry, * Return true the the route is attached via an interface that * is not in the same table as the route */ -static inline int +static int fib_route_attached_cross_table (const fib_entry_t *fib_entry, const fib_route_path_t *rpath) { diff --git a/src/vnet/fib/fib_node.h b/src/vnet/fib/fib_node.h index 27e67b11c87..dd266ee8ff9 100644 --- a/src/vnet/fib/fib_node.h +++ b/src/vnet/fib/fib_node.h @@ -109,6 +109,10 @@ typedef enum fib_node_back_walk_reason_t_ { * A resolving interface has gone down */ FIB_NODE_BW_REASON_INTERFACE_DOWN, + /** + * A resolving interface has been bound to another table + */ + FIB_NODE_BW_REASON_INTERFACE_BIND, /** * A resolving interface has been deleted. */ @@ -138,6 +142,7 @@ typedef enum fib_node_back_walk_reason_t_ { [FIB_NODE_BW_REASON_INTERFACE_UP] = "if-up", \ [FIB_NODE_BW_REASON_INTERFACE_DOWN] = "if-down", \ [FIB_NODE_BW_REASON_INTERFACE_DELETE] = "if-delete", \ + [FIB_NODE_BW_REASON_INTERFACE_BIND] = "if-bind", \ [FIB_NODE_BW_REASON_ADJ_UPDATE] = "adj-update", \ [FIB_NODE_BW_REASON_ADJ_MTU] = "adj-mtu", \ [FIB_NODE_BW_REASON_ADJ_DOWN] = "adj-down", \ @@ -157,14 +162,15 @@ typedef enum fib_node_bw_reason_flag_t_ { FIB_NODE_BW_REASON_FLAG_EVALUATE = (1 << FIB_NODE_BW_REASON_EVALUATE), FIB_NODE_BW_REASON_FLAG_INTERFACE_UP = (1 << FIB_NODE_BW_REASON_INTERFACE_UP), FIB_NODE_BW_REASON_FLAG_INTERFACE_DOWN = (1 << FIB_NODE_BW_REASON_INTERFACE_DOWN), + FIB_NODE_BW_REASON_FLAG_INTERFACE_BIND = (1 << FIB_NODE_BW_REASON_INTERFACE_BIND), FIB_NODE_BW_REASON_FLAG_INTERFACE_DELETE = (1 << FIB_NODE_BW_REASON_INTERFACE_DELETE), FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE = (1 << FIB_NODE_BW_REASON_ADJ_UPDATE), FIB_NODE_BW_REASON_FLAG_ADJ_MTU = (1 << FIB_NODE_BW_REASON_ADJ_MTU), FIB_NODE_BW_REASON_FLAG_ADJ_DOWN = (1 << FIB_NODE_BW_REASON_ADJ_DOWN), } __attribute__ ((packed)) fib_node_bw_reason_flag_t; -STATIC_ASSERT(sizeof(fib_node_bw_reason_flag_t) < 2, - "BW Reason enum < 2 byte. Consequences for cover_upd_res_t"); +STATIC_ASSERT(sizeof(fib_node_bw_reason_flag_t) < 3, + "BW Reason enum < 2 byte. Consequences for fib_entry_src_cover_res_t"); extern u8 *format_fib_node_bw_reason(u8 *s, va_list *args); @@ -229,6 +235,17 @@ typedef struct fib_node_back_walk_ctx_t_ { * in the graph. */ u32 fnbw_depth; + + /** + * Additional data associated with the reason the walk is occuring + */ + union + { + struct { + u32 fnbw_from_fib_index; + u32 fnbw_to_fib_index; + } interface_bind; + }; } fib_node_back_walk_ctx_t; /** diff --git a/src/vnet/fib/fib_path.c b/src/vnet/fib/fib_path.c index 209cf403c6e..d7b6091325b 100644 --- a/src/vnet/fib/fib_path.c +++ b/src/vnet/fib/fib_path.c @@ -1161,6 +1161,11 @@ FIXME comment fib_path_unresolve(path); path->fp_oper_flags |= FIB_PATH_OPER_FLAG_DROP; } + if (FIB_NODE_BW_REASON_FLAG_INTERFACE_BIND & ctx->fnbw_reason) + { + /* bind walks should appear here and pass silently up to + * to the fib_entry */ + } break; case FIB_PATH_TYPE_UDP_ENCAP: { diff --git a/test/test_ip4.py b/test/test_ip4.py index 3f2894d01c1..b337e63a957 100644 --- a/test/test_ip4.py +++ b/test/test_ip4.py @@ -2733,5 +2733,140 @@ class TestIPv4PathMTU(VppTestCase): self.send_and_expect(self.pg0, [p_1k], self.pg1, n_rx=2) +class TestIPv4ItfRebind(VppTestCase): + """ IPv4 Interface Bind w/ attached routes """ + + def setUp(self): + super(TestIPv4ItfRebind, self).setUp() + + self.create_pg_interfaces(range(3)) + + def tearDown(self): + super(TestIPv4ItfRebind, self).tearDown() + + def test_rebind(self): + """ Import to no import """ + + TABLE_ID = 1 + tbl = VppIpTable(self, TABLE_ID).add_vpp_config() + self.pg1.set_table_ip4(TABLE_ID) + + for i in self.pg_interfaces: + i.admin_up() + i.config_ip4() + i.resolve_arp() + + # add an attached route via an pg0 + # in a different table. this prefix should import + rt = VppIpRoute(self, self.pg0.local_ip4, 24, + [VppRoutePath("0.0.0.0", + self.pg0.sw_if_index)], + table_id=TABLE_ID).add_vpp_config() + + p = (Ether(dst=self.pg1.local_mac, + src=self.pg1.remote_mac) / + IP(src=self.pg1.remote_ip4, + dst=self.pg0.remote_ip4) / + UDP(sport=1234, dport=5678) / + Raw(b'0xa' * 640)) + + rx = self.send_and_expect(self.pg1, [p], self.pg0) + self.assertFalse(rx[0].haslayer(ARP)) + + # then bind pg0 to a new table + # so the prefix no longer imports + self.pg0.unconfig_ip4() + self.pg0.set_table_ip4(TABLE_ID) + self.pg0.config_ip4() + self.pg0.resolve_arp() + + rx = self.send_and_expect(self.pg1, [p], self.pg0) + self.assertFalse(rx[0].haslayer(ARP)) + + # revert back to imported + self.pg0.unconfig_ip4() + self.pg0.set_table_ip4(0) + self.pg0.config_ip4() + self.pg0.resolve_arp() + + rx = self.send_and_expect(self.pg1, [p], self.pg0) + self.assertFalse(rx[0].haslayer(ARP)) + + # cleanup + for i in self.pg_interfaces: + i.unconfig_ip4() + i.set_table_ip4(0) + i.admin_down() + + rt.remove_vpp_config() + tbl.remove_vpp_config() + + def test_delete(self): + """ Swap import tables """ + + TABLE_ID1 = 1 + tbl1_4 = VppIpTable(self, TABLE_ID1).add_vpp_config() + tbl1_6 = VppIpTable(self, TABLE_ID1, True).add_vpp_config() + TABLE_ID2 = 2 + tbl2_4 = VppIpTable(self, TABLE_ID2).add_vpp_config() + tbl2_6 = VppIpTable(self, TABLE_ID2, True).add_vpp_config() + + # table mappings + self.pg1.set_table_ip4(TABLE_ID1) + self.pg1.set_table_ip6(TABLE_ID1) + self.pg2.set_table_ip4(TABLE_ID2) + self.pg2.set_table_ip6(TABLE_ID2) + + for i in self.pg_interfaces: + i.admin_up() + i.config_ip4() + i.resolve_arp() + + # add an attached route in the default table via pg0 + # this should import to table 1 + rt4 = VppIpRoute(self, self.pg1.local_ip4, 24, + [VppRoutePath("0.0.0.0", + self.pg1.sw_if_index)]).add_vpp_config() + rt6 = VppIpRoute(self, self.pg1.local_ip6, 64, + [VppRoutePath("0.0.0.0", + self.pg1.sw_if_index)]).add_vpp_config() + + p1 = (Ether(dst=self.pg0.local_mac, + src=self.pg0.remote_mac) / + IP(src=self.pg1.remote_ip4, + dst=self.pg1.remote_ip4) / + UDP(sport=1234, dport=5678) / + Raw(b'0xa' * 640)) + + # inject into table 0 + rx = self.send_and_expect(self.pg0, [p1], self.pg1) + self.assertFalse(rx[0].haslayer(ARP)) + + # swap the attached interface to table 2 + self.pg1.unconfig_ip4() + self.pg1.unconfig_ip6() + self.pg1.set_table_ip4(TABLE_ID2) + self.pg1.set_table_ip6(TABLE_ID2) + self.pg1.config_ip4() + self.pg1.config_ip6() + self.pg1.resolve_arp() + + # delete table 1 + tbl1_4.flush() + tbl1_6.flush() + tbl1_4.remove_vpp_config() + tbl1_6.remove_vpp_config() + + rx = self.send_and_expect(self.pg0, [p1], self.pg1) + self.assertFalse(rx[0].haslayer(ARP)) + + for i in self.pg_interfaces: + i.unconfig_ip4() + i.unconfig_ip6() + i.set_table_ip4(0) + i.set_table_ip6(0) + i.admin_down() + + if __name__ == '__main__': unittest.main(testRunner=VppTestRunner) -- 2.16.6