]> git.puffer.fish Git - mirror/frr.git/commitdiff
lib : fix duplicate prefix list delete
authorSamanvitha B Bhargav <bsamanvitha@vmware.com>
Mon, 24 Apr 2023 08:51:23 +0000 (01:51 -0700)
committerSamanvitha B Bhargav <bsamanvitha@vmware.com>
Tue, 9 May 2023 09:23:44 +0000 (02:23 -0700)
Problem statement:
Step-1:
pl1 - 10.10.10.10/24 with deny sequence 1
Step-2:
pl1 - 10.10.10.10/24 with permit sequence 2
Step-3:
pl1 - 20.20.20.20/24 with deny sequence 1

Now we end up deleting permit sequence 2,
which might blackhole the traffic.

RCA:
Whenever we have multiple prefix lists,
having same prefix and different subnet range,
delete or replace of prefix list
would result in delete of entry in route-map
prefix table.

Fix:
We will skip deleting prefix list entry
from routemap prefix table, if we have
the multiple prefix lists having same prefix.

Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
lib/plist.c

index e286a32f9212ba1b699fba7a5a9eaf4df8f0ac0b..d8ce83d219ead532a474fce9c1078f2d120a4433 100644 (file)
@@ -336,6 +336,22 @@ 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,
@@ -404,12 +420,16 @@ 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)
@@ -421,8 +441,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 +579,15 @@ 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`. */
@@ -574,8 +600,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);