aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNeale Ranns <nranns@cisco.com>2017-05-13 05:52:58 -0700
committerDave Barach <openvpp@barachs.net>2017-05-15 11:36:33 +0000
commit08b1648194e0076220c52aebe27ee1eb5615fd6a (patch)
tree00ff881719d0d47a49898b333a00d0945b323893
parent572825df79e27a8baebcfc11d91bdb6f9776c2c7 (diff)
Fix FIB recursion loops via cover (VPP-842)
Change-Id: Ia91c3e8cb27b9e4c1cccefc0a4857dd9995450ab Signed-off-by: Neale Ranns <nranns@cisco.com>
-rw-r--r--src/vnet/fib/fib_entry_src_rr.c78
-rw-r--r--src/vnet/fib/fib_path.c11
-rw-r--r--src/vnet/fib/fib_test.c30
3 files changed, 81 insertions, 38 deletions
diff --git a/src/vnet/fib/fib_entry_src_rr.c b/src/vnet/fib/fib_entry_src_rr.c
index c145aaa2915..d66ef7b1108 100644
--- a/src/vnet/fib/fib_entry_src_rr.c
+++ b/src/vnet/fib/fib_entry_src_rr.c
@@ -69,6 +69,47 @@ fib_entry_src_rr_init (fib_entry_src_t *src)
src->rr.fesr_sibling = FIB_NODE_INDEX_INVALID;
}
+
+/*
+ * 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.
+ */
+static void
+fib_entry_src_rr_use_covers_pl (fib_entry_src_t *src,
+ const fib_entry_t *fib_entry,
+ const fib_entry_t *cover)
+{
+ 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);
+}
+
/*
* Source activation. Called when the source is the new best source on the entry
*/
@@ -112,40 +153,7 @@ fib_entry_src_rr_activate (fib_entry_src_t *src,
}
else
{
- /*
- * 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_entry_src_rr_use_covers_pl(src, fib_entry, cover);
}
fib_path_list_lock(src->fes_pl);
@@ -256,7 +264,7 @@ fib_entry_src_rr_cover_update (fib_entry_src_t *src,
}
else
{
- src->fes_pl = cover->fe_parent;
+ fib_entry_src_rr_use_covers_pl(src, fib_entry, cover);
}
fib_path_list_lock(src->fes_pl);
fib_path_list_unlock(old_path_list);
diff --git a/src/vnet/fib/fib_path.c b/src/vnet/fib/fib_path.c
index 889317fd83c..b47d51f8d51 100644
--- a/src/vnet/fib/fib_path.c
+++ b/src/vnet/fib/fib_path.c
@@ -1716,7 +1716,11 @@ fib_path_get_resolving_interface (fib_node_index_t path_index)
case FIB_PATH_TYPE_RECEIVE:
return (path->receive.fp_interface);
case FIB_PATH_TYPE_RECURSIVE:
- return (fib_entry_get_resolving_interface(path->fp_via_fib));
+ if (fib_path_is_resolved(path_index))
+ {
+ return (fib_entry_get_resolving_interface(path->fp_via_fib));
+ }
+ break;
case FIB_PATH_TYPE_INTF_RX:
case FIB_PATH_TYPE_SPECIAL:
case FIB_PATH_TYPE_DEAG:
@@ -1781,11 +1785,12 @@ fib_path_contribute_urpf (fib_node_index_t path_index,
break;
case FIB_PATH_TYPE_RECURSIVE:
- if (FIB_NODE_INDEX_INVALID != path->fp_via_fib)
+ if (FIB_NODE_INDEX_INVALID != path->fp_via_fib &&
+ !fib_path_is_looped(path_index))
{
/*
* there's unresolved due to constraints, and there's unresolved
- * due to ain't go no via. can't do nowt w'out via.
+ * due to ain't got no via. can't do nowt w'out via.
*/
fib_entry_contribute_urpf(path->fp_via_fib, urpf);
}
diff --git a/src/vnet/fib/fib_test.c b/src/vnet/fib/fib_test.c
index ddea6b86e7e..486e5616798 100644
--- a/src/vnet/fib/fib_test.c
+++ b/src/vnet/fib/fib_test.c
@@ -2977,6 +2977,36 @@ fib_test_v4 (void)
fib_entry_pool_size());
/*
+ * Make the default route recursive via a unknown next-hop. Thus the
+ * next hop's cover would be the default route
+ */
+ fei = fib_table_entry_path_add(fib_index,
+ &pfx_0_0_0_0_s_0,
+ FIB_SOURCE_API,
+ FIB_ENTRY_FLAG_NONE,
+ FIB_PROTOCOL_IP4,
+ &pfx_23_23_23_23_s_32.fp_addr,
+ ~0, // recursive
+ fib_index,
+ 1,
+ NULL,
+ FIB_ROUTE_PATH_FLAG_NONE);
+ dpo = fib_entry_contribute_ip_forwarding(fei);
+ FIB_TEST(load_balance_is_drop(dpo),
+ "0.0.0.0.0/0 via is DROP");
+ FIB_TEST((fib_entry_get_resolving_interface(fei) == ~0),
+ "no resolving interface for looped 0.0.0.0/0");
+
+ fei = fib_table_lookup_exact_match(fib_index, &pfx_23_23_23_23_s_32);
+ dpo = fib_entry_contribute_ip_forwarding(fei);
+ FIB_TEST(load_balance_is_drop(dpo),
+ "23.23.23.23/32 via is DROP");
+ FIB_TEST((fib_entry_get_resolving_interface(fei) == ~0),
+ "no resolving interface for looped 23.23.23.23/32");
+
+ fib_table_entry_delete(fib_index, &pfx_0_0_0_0_s_0, FIB_SOURCE_API);
+
+ /*
* A recursive route with recursion constraints.
* 200.200.200.200/32 via 1.1.1.1 is recurse via host constrained
*/