From 3dffb1e4c628f0698e369d1cbb3cb2068a3a698c Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Tue, 1 Nov 2016 20:38:53 +0000 Subject: Fix recursion loop - recurse through cover Change-Id: I07870122f90e41fbb216b2f426bccbfd94049cd6 Signed-off-by: Neale Ranns --- vnet/vnet/fib/fib_entry_src_rr.c | 36 +++++++++++++++++++++++++++++++++++- vnet/vnet/fib/fib_test.c | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/vnet/vnet/fib/fib_entry_src_rr.c b/vnet/vnet/fib/fib_entry_src_rr.c index 9219d58e46e..6d56541dec2 100644 --- a/vnet/vnet/fib/fib_entry_src_rr.c +++ b/vnet/vnet/fib/fib_entry_src_rr.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "fib_entry_src.h" #include "fib_entry_cover.h" @@ -100,7 +101,40 @@ fib_entry_src_rr_activate (fib_entry_src_t *src, } else { - src->fes_pl = cover->fe_parent; + /* + * use the path-list of the cover, unless it would form a loop. + * that is unless the cover is via this entry. + * If a loop were to form it would be a 1 level loop (i.e. X via X), + * and there would be 2 locks on the path-list; one since its used + * by the cover, and 1 from here. The first lock will go when the + * cover is removed, the second, and last, when the covered walk + * occurs during the cover's removel - this is not a place where + * we can handle last lock gone. + * In short, don't let the loop form. The usual rules of 'we must + * let it form so we know when it breaks' don't apply here, since + * the loop will break when the cover changes, and this function + * will be called again when that happens. + */ + fib_node_index_t *entries = NULL; + fib_protocol_t proto; + + proto = fib_entry->fe_prefix.fp_proto; + vec_add1(entries, fib_entry_get_index(fib_entry)); + + if (fib_path_list_recursive_loop_detect(cover->fe_parent, + &entries)) + { + src->fes_pl = fib_path_list_create_special( + proto, + FIB_PATH_LIST_FLAG_DROP, + drop_dpo_get(fib_proto_to_dpo(proto))); + } + else + { + src->fes_pl = cover->fe_parent; + } + vec_free(entries); + } fib_path_list_lock(src->fes_pl); diff --git a/vnet/vnet/fib/fib_test.c b/vnet/vnet/fib/fib_test.c index 2194fcbc2bf..49fc318cbb7 100644 --- a/vnet/vnet/fib/fib_test.c +++ b/vnet/vnet/fib/fib_test.c @@ -2076,6 +2076,42 @@ fib_test_v4 (void) fib_table_lookup_exact_match(fib_index, &pfx_5_5_5_6_s_32), "1-level 5.5.5.6/32 loop is removed"); + /* + * A recursive route whose next-hop is covered by the prefix. + * This would mean the via-fib, which inherits forwarding from its + * cover, thus picks up forwarding from the prfix, which is via the + * via-fib, and we have a loop. + */ + fib_prefix_t pfx_23_23_23_0_s_24 = { + .fp_len = 24, + .fp_proto = FIB_PROTOCOL_IP4, + .fp_addr = { + .ip4.as_u32 = clib_host_to_net_u32(0x17171700), + }, + }; + fib_prefix_t pfx_23_23_23_23_s_32 = { + .fp_len = 32, + .fp_proto = FIB_PROTOCOL_IP4, + .fp_addr = { + .ip4.as_u32 = clib_host_to_net_u32(0x17171717), + }, + }; + fei = fib_table_entry_path_add(fib_index, + &pfx_23_23_23_0_s_24, + FIB_SOURCE_API, + FIB_ENTRY_FLAG_NONE, + FIB_PROTOCOL_IP4, + &pfx_23_23_23_23_s_32.fp_addr, + ~0, // recursive + fib_index, + 1, + MPLS_LABEL_INVALID, + FIB_ROUTE_PATH_FLAG_NONE); + dpo = fib_entry_contribute_ip_forwarding(fei); + FIB_TEST(load_balance_is_drop(dpo), + "23.23.23.0/24 via covered is DROP"); + fib_table_entry_delete_index(fei, FIB_SOURCE_API); + /* * add-remove test. no change. */ @@ -2800,7 +2836,7 @@ fib_test_v4 (void) FIB_TEST((0 == adj_nbr_db_size()), "ADJ DB size is %d", adj_nbr_db_size()); - + /* * CLEANUP * remove the interface prefixes -- cgit 1.2.3-korg