From 71fb99d22ebe05b18aa01ab02eed957dd1167a97 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Fri, 22 Sep 2023 12:24:16 -0300 Subject: [PATCH 1/2] Revert "lib : fix duplicate prefix list delete" This reverts commit 394ed767e7207805a6d916b01b1f1d4743c03dd1. --- lib/plist.c | 39 ++++++--------------------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/lib/plist.c b/lib/plist.c index d8ce83d219ea..e286a32f9212 100644 --- a/lib/plist.c +++ b/lib/plist.c @@ -336,22 +336,6 @@ prefix_list_entry_lookup(struct prefix_list *plist, struct prefix *prefix, return NULL; } -static bool -prefix_list_entry_lookup_prefix(struct prefix_list *plist, - struct prefix_list_entry *plist_entry) -{ - struct prefix_list_entry *pentry = NULL; - - for (pentry = plist->head; pentry; pentry = pentry->next) { - if (pentry == plist_entry) - continue; - if (prefix_same(&pentry->prefix, &plist_entry->prefix)) - return true; - } - - return false; -} - static void trie_walk_affected(size_t validbits, struct pltrie_table *table, uint8_t byte, struct prefix_list_entry *object, void (*fn)(struct prefix_list_entry *object, @@ -420,16 +404,12 @@ static void prefix_list_trie_del(struct prefix_list *plist, void prefix_list_entry_delete(struct prefix_list *plist, - struct prefix_list_entry *pentry, int update_list) + struct prefix_list_entry *pentry, + int update_list) { - bool duplicate = false; - if (plist == NULL || pentry == NULL) return; - if (prefix_list_entry_lookup_prefix(plist, pentry)) - duplicate = true; - prefix_list_trie_del(plist, pentry); if (pentry->prev) @@ -441,10 +421,8 @@ void prefix_list_entry_delete(struct prefix_list *plist, else plist->tail = pentry->prev; - if (!duplicate) - route_map_notify_pentry_dependencies(plist->name, pentry, - RMAP_EVENT_PLIST_DELETED); - + route_map_notify_pentry_dependencies(plist->name, pentry, + RMAP_EVENT_PLIST_DELETED); prefix_list_entry_free(pentry); plist->count--; @@ -579,15 +557,11 @@ static void prefix_list_entry_add(struct prefix_list *plist, void prefix_list_entry_update_start(struct prefix_list_entry *ple) { struct prefix_list *pl = ple->pl; - bool duplicate = false; /* Not installed, nothing to do. */ if (!ple->installed) return; - if (prefix_list_entry_lookup_prefix(pl, ple)) - duplicate = true; - prefix_list_trie_del(pl, ple); /* List manipulation: shameless copy from `prefix_list_entry_delete`. */ @@ -600,9 +574,8 @@ void prefix_list_entry_update_start(struct prefix_list_entry *ple) else pl->tail = ple->prev; - if (!duplicate) - route_map_notify_pentry_dependencies(pl->name, ple, - RMAP_EVENT_PLIST_DELETED); + route_map_notify_pentry_dependencies(pl->name, ple, + RMAP_EVENT_PLIST_DELETED); pl->count--; route_map_notify_dependencies(pl->name, RMAP_EVENT_PLIST_DELETED); From aed94c80967213124236ae0dfc8eebfbe9f465d5 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Fri, 22 Sep 2023 12:28:38 -0300 Subject: [PATCH 2/2] lib: don't announce prefix delete for duplicates When deleting a duplicated prefix list entry don't announce the change to route map listeners, otherwise they will be removing rules that shouldn't be removed causing the prefix that still exist in the prefix-list to be not evaluated anymore. Signed-off-by: Rafael Zalamena --- lib/plist.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/plist.c b/lib/plist.c index e286a32f9212..2f5827cf4354 100644 --- a/lib/plist.c +++ b/lib/plist.c @@ -402,14 +402,55 @@ static void prefix_list_trie_del(struct prefix_list *plist, } } +/** + * Find duplicated prefix entry (same prefix but different entry) in prefix + * list. + */ +static bool prefix_list_entry_is_duplicated(struct prefix_list *list, + struct prefix_list_entry *entry) +{ + size_t depth, maxdepth = list->master->trie_depth; + uint8_t byte, *bytes = entry->prefix.u.val; + size_t validbits = entry->prefix.prefixlen; + struct pltrie_table *table = list->trie; + struct prefix_list_entry *pentry; + + for (depth = 0; validbits > PLC_BITS && depth < maxdepth - 1; depth++) { + byte = bytes[depth]; + if (!table->entries[byte].next_table) + return NULL; + + table = table->entries[byte].next_table; + validbits -= PLC_BITS; + } + + byte = bytes[depth]; + if (validbits > PLC_BITS) + pentry = table->entries[byte].final_chain; + else + pentry = table->entries[byte].up_chain; + + for (; pentry; pentry = pentry->next_best) { + if (pentry == entry) + continue; + if (prefix_same(&pentry->prefix, &entry->prefix)) + return true; + } + + return false; +} void prefix_list_entry_delete(struct prefix_list *plist, struct prefix_list_entry *pentry, int update_list) { + bool duplicate; + if (plist == NULL || pentry == NULL) return; + duplicate = prefix_list_entry_is_duplicated(plist, pentry); + prefix_list_trie_del(plist, pentry); if (pentry->prev) @@ -421,8 +462,10 @@ void prefix_list_entry_delete(struct prefix_list *plist, else plist->tail = pentry->prev; - route_map_notify_pentry_dependencies(plist->name, pentry, - RMAP_EVENT_PLIST_DELETED); + if (!duplicate) + route_map_notify_pentry_dependencies(plist->name, pentry, + RMAP_EVENT_PLIST_DELETED); + prefix_list_entry_free(pentry); plist->count--; @@ -557,11 +600,14 @@ static void prefix_list_entry_add(struct prefix_list *plist, void prefix_list_entry_update_start(struct prefix_list_entry *ple) { struct prefix_list *pl = ple->pl; + bool duplicate; /* Not installed, nothing to do. */ if (!ple->installed) return; + duplicate = prefix_list_entry_is_duplicated(pl, ple); + prefix_list_trie_del(pl, ple); /* List manipulation: shameless copy from `prefix_list_entry_delete`. */ @@ -574,8 +620,9 @@ void prefix_list_entry_update_start(struct prefix_list_entry *ple) else pl->tail = ple->prev; - route_map_notify_pentry_dependencies(pl->name, ple, - RMAP_EVENT_PLIST_DELETED); + if (!duplicate) + route_map_notify_pentry_dependencies(pl->name, ple, + RMAP_EVENT_PLIST_DELETED); pl->count--; route_map_notify_dependencies(pl->name, RMAP_EVENT_PLIST_DELETED);