bgpd: fix rpki revalidation for invalid announcements

Announcements that are marked as invalid were previously not revalidated.
This was fixed by replacing the range lookup with a subtree lookup.

Signed-off-by: Marcel Röthke <marcel.roethke@haw-hamburg.de>
This commit is contained in:
Marcel Röthke 2020-06-11 16:11:09 +02:00
parent a633498e0b
commit bac31cb885
5 changed files with 90 additions and 145 deletions

View File

@ -402,25 +402,23 @@ static int bgpd_sync_callback(struct thread *thread)
if (!peer->bgp->rib[afi][safi])
continue;
struct list *matches = list_new();
struct bgp_node *match;
struct bgp_node *node;
matches->del =
(void (*)(void *))bgp_unlock_node;
match = bgp_table_subtree_lookup(
peer->bgp->rib[afi][safi], prefix);
node = match;
bgp_table_range_lookup(
peer->bgp->rib[afi][safi], prefix,
rec.max_len, matches);
while (node) {
if (bgp_node_has_bgp_path_info_data(
node)) {
revalidate_bgp_node(node, afi,
safi);
}
struct bgp_node *bgp_node;
struct listnode *bgp_listnode;
for (ALL_LIST_ELEMENTS_RO(matches, bgp_listnode,
bgp_node))
revalidate_bgp_node(bgp_node, afi,
safi);
list_delete(&matches);
node = bgp_route_next_until(node,
match);
}
}
}
}

View File

@ -164,90 +164,42 @@ void bgp_delete_listnode(struct bgp_node *node)
}
}
static struct bgp_node *
bgp_route_next_until_maxlen(struct bgp_node *node, const struct bgp_node *limit,
const uint8_t maxlen)
{
const struct prefix *p = bgp_node_get_prefix(node);
if (node->l_left) {
const struct prefix *left_p =
bgp_node_get_prefix(bgp_node_from_rnode(node->l_left));
if (p->prefixlen < maxlen && left_p->prefixlen <= maxlen)
return bgp_node_from_rnode(node->l_left);
}
if (node->l_right) {
const struct prefix *right_p =
bgp_node_get_prefix(bgp_node_from_rnode(node->l_right));
if (p->prefixlen < maxlen && right_p->prefixlen <= maxlen)
return bgp_node_from_rnode(node->l_right);
}
while (node->parent && node != limit) {
if (bgp_node_from_rnode(node->parent->l_left) == node
&& node->parent->l_right) {
return bgp_node_from_rnode(node->parent->l_right);
}
node = bgp_node_from_rnode(node->parent);
}
return NULL;
}
void bgp_table_range_lookup(const struct bgp_table *table,
const struct prefix *p,
uint8_t maxlen, struct list *matches)
struct bgp_node *bgp_table_subtree_lookup(const struct bgp_table *table,
const struct prefix *p)
{
struct bgp_node *node = bgp_node_from_rnode(table->route_table->top);
struct bgp_node *matched = NULL;
if (node == NULL)
return;
return NULL;
const struct prefix *node_p = bgp_node_get_prefix(node);
while (node && node_p->prefixlen <= p->prefixlen
&& prefix_match(node_p, p)) {
if (bgp_node_has_bgp_path_info_data(node)
&& node_p->prefixlen == p->prefixlen) {
while (node) {
const struct prefix *node_p = bgp_node_get_prefix(node);
if (node_p->prefixlen >= p->prefixlen) {
if (!prefix_match(p, node_p))
return NULL;
matched = node;
break;
}
if (!prefix_match(node_p, p))
return NULL;
if (node_p->prefixlen == p->prefixlen) {
matched = node;
break;
}
node = bgp_node_from_rnode(node->link[prefix_bit(
&p->u.prefix, node_p->prefixlen)]);
node_p = bgp_node_get_prefix(node);
}
if (!node)
return;
node_p = bgp_node_get_prefix(node);
if (matched == NULL && node_p->prefixlen <= maxlen
&& prefix_match(p, node_p) && node->parent == NULL)
matched = node;
else if ((matched == NULL && node_p->prefixlen > maxlen)
|| !node->parent)
return;
else if (matched == NULL && node->parent)
matched = node = bgp_node_from_rnode(node->parent);
if (!matched)
return;
return NULL;
if (bgp_node_has_bgp_path_info_data(matched)) {
bgp_lock_node(matched);
listnode_add(matches, matched);
}
while ((node = bgp_route_next_until_maxlen(node, matched, maxlen))) {
node_p = bgp_node_get_prefix(node);
if (prefix_match(p, node_p)) {
if (bgp_node_has_bgp_path_info_data(node)) {
bgp_lock_node(node);
listnode_add(matches, node);
}
}
}
bgp_lock_node(matched);
return matched;
}

View File

@ -347,9 +347,15 @@ static inline uint64_t bgp_table_version(struct bgp_table *table)
return table->version;
}
void bgp_table_range_lookup(const struct bgp_table *table,
const struct prefix *p,
uint8_t maxlen, struct list *matches);
/* Find the subtree of the prefix p
*
* This will return the first node that belongs the the subtree of p. Including
* p itself, if it is in the tree.
*
* If the subtree is not present in the table, NULL is returned.
*/
struct bgp_node *bgp_table_subtree_lookup(const struct bgp_table *table,
const struct prefix *p);
static inline struct bgp_aggregate *

View File

@ -73,70 +73,64 @@ static void add_node(struct bgp_table *table, const char *prefix_str)
rn->info = node;
}
static void print_range_result(struct list *list)
static bool prefix_in_array(const struct prefix *p, struct prefix *prefix_array,
size_t prefix_array_size)
{
struct listnode *listnode;
struct bgp_node *bnode;
for (ALL_LIST_ELEMENTS_RO(list, listnode, bnode)) {
char buf[PREFIX2STR_BUFFER];
prefix2str(bgp_node_get_prefix(bnode), buf, PREFIX2STR_BUFFER);
printf("%s\n", buf);
for (size_t i = 0; i < prefix_array_size; ++i) {
if (prefix_same(p, &prefix_array[i]))
return true;
}
return false;
}
static void check_lookup_result(struct list *list, va_list arglist)
static void check_lookup_result(struct bgp_node *match, va_list arglist)
{
char *prefix_str;
unsigned int prefix_count = 0;
struct prefix *prefixes = NULL;
size_t prefix_count = 0;
printf("Searching results\n");
while ((prefix_str = va_arg(arglist, char *))) {
struct listnode *listnode;
struct bgp_node *bnode;
struct prefix p;
bool found = false;
++prefix_count;
prefixes = realloc(prefixes, sizeof(*prefixes) * prefix_count);
prefix_count++;
printf("Searching for %s\n", prefix_str);
if (str2prefix(prefix_str, &p) <= 0)
if (str2prefix(prefix_str, &prefixes[prefix_count - 1]) <= 0)
assert(0);
for (ALL_LIST_ELEMENTS_RO(list, listnode, bnode)) {
if (prefix_same(bgp_node_get_prefix(bnode), &p))
found = true;
}
assert(found);
}
printf("Checking for unexpected result items\n");
printf("Expecting %d found %d\n", prefix_count, listcount(list));
assert(prefix_count == listcount(list));
/* check if the result is empty and if it is allowd to be empty */
assert((prefix_count == 0 && !match) || prefix_count > 0);
if (!match)
return;
struct bgp_node *node = match;
while ((node = bgp_route_next_until(node, match))) {
const struct prefix *node_p = bgp_node_get_prefix(node);
if (bgp_node_has_bgp_path_info_data(node)
&& !prefix_in_array(node_p, prefixes, prefix_count)) {
char buf[PREFIX2STR_BUFFER];
prefix2str(node_p, buf, PREFIX2STR_BUFFER);
printf("prefix %s was not expected!\n", buf);
assert(0);
}
}
}
static void do_test(struct bgp_table *table, const char *prefix,
uint32_t maxlen, ...)
static void do_test(struct bgp_table *table, const char *prefix, ...)
{
va_list arglist;
struct list *list = list_new();
struct prefix p;
list->del = (void (*)(void *))bgp_unlock_node;
va_start(arglist, maxlen);
printf("\nDoing lookup for %s-%d\n", prefix, maxlen);
va_start(arglist, prefix);
printf("\nDoing lookup for %s\n", prefix);
if (str2prefix(prefix, &p) <= 0)
assert(0);
bgp_table_range_lookup(table, &p, maxlen, list);
print_range_result(list);
struct bgp_node *node = bgp_table_subtree_lookup(table, &p);
check_lookup_result(list, arglist);
list_delete(&list);
check_lookup_result(node, arglist);
va_end(arglist);
@ -163,27 +157,22 @@ static void test_range_lookup(void)
for (int i = 0; i < num_prefixes; i++)
add_node(table, prefixes[i]);
do_test(table, "1.16.0.0/17", 20, "1.16.64.0/19", "1.16.32.0/20", NULL);
do_test(table, "1.16.128.0/17", 20, "1.16.128.0/18", "1.16.192.0/18",
do_test(table, "1.16.0.0/17", "1.16.64.0/19", "1.16.32.0/20",
"1.16.32.0/20", "1.16.32.0/21", NULL);
do_test(table, "1.16.128.0/17", "1.16.128.0/18", "1.16.192.0/18",
"1.16.160.0/19", NULL);
do_test(table, "1.16.128.0/17", 20, "1.16.128.0/18", "1.16.192.0/18",
"1.16.160.0/19", NULL);
do_test(table, "1.16.0.0/16", 18, "1.16.0.0/16", "1.16.128.0/18",
"1.16.192.0/18", NULL);
do_test(table, "1.16.0.0/16", 21, "1.16.0.0/16", "1.16.128.0/18",
do_test(table, "1.16.0.0/16", "1.16.0.0/16", "1.16.128.0/18",
"1.16.192.0/18", "1.16.64.0/19", "1.16.160.0/19",
"1.16.32.0/20", "1.16.32.0/21", NULL);
do_test(table, "1.17.0.0/16", 20, NULL);
do_test(table, "1.17.0.0/16", NULL);
do_test(table, "128.0.0.0/8", 16, NULL);
do_test(table, "128.0.0.0/8", NULL);
do_test(table, "16.0.0.0/8", 16, "16.0.0.0/16", NULL);
do_test(table, "16.0.0.0/8", "16.0.0.0/16", NULL);
do_test(table, "0.0.0.0/2", 21, "1.16.0.0/16", "1.16.128.0/18",
do_test(table, "0.0.0.0/2", "1.16.0.0/16", "1.16.128.0/18",
"1.16.192.0/18", "1.16.64.0/19", "1.16.160.0/19",
"1.16.32.0/20", "1.16.32.0/21", "16.0.0.0/16", NULL);
}

View File

@ -3,5 +3,5 @@ import frrtest
class TestTable(frrtest.TestMultiOut):
program = './test_bgp_table'
for i in range(9):
for i in range(7):
TestTable.onesimple('Checks successfull')