aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--vnet/vnet/fib/fib_entry_src_rr.c36
-rw-r--r--vnet/vnet/fib/fib_test.c38
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 9219d58e..6d56541d 100644
--- a/vnet/vnet/fib/fib_entry_src_rr.c
+++ b/vnet/vnet/fib/fib_entry_src_rr.c
@@ -17,6 +17,7 @@
#include <vnet/ip/format.h>
#include <vnet/ip/lookup.h>
#include <vnet/adj/adj.h>
+#include <vnet/dpo/drop_dpo.h>
#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 2194fcbc..49fc318c 100644
--- a/vnet/vnet/fib/fib_test.c
+++ b/vnet/vnet/fib/fib_test.c
@@ -2077,6 +2077,42 @@ fib_test_v4 (void)
"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.
*/
FIB_TEST((1 == fib_path_list_db_size()), "path list DB population:%d",
@@ -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