lib: fix prefix list installation

Based on the function `prefix_list_entry_add` and
`prefix_list_entry_delete` it was created two functions to replicate
its functionality without the assumption we are always alocating a new
prefix list entry.

Since the prefix list entry is stored in the YANG private data
structures, we want to avoid the allocation/free of memory that is
hold by the schema.

Every time a prefix list entry values change we must call
`prefix_list_entry_update_start` to uninstall the entry from prefix
list internal structures and then call
`prefix_list_entry_update_finish` to put them back.

The variable `installed` in the prefix list entry tracks the
installation status of the internal structure. It is possible that a
user unconfigures or forgets to add a `prefix` value and so we can't
install the entry until then.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

diff --git a/lib/filter_nb.c b/lib/filter_nb.c
index 46ea71aeca4de428808158688a0c7fcd16479a18..0f6e5488fc6280a918d579ba06baf0f96fb0679d 100644 (file)
--- a/lib/filter_nb.c
+++ b/lib/filter_nb.c
@@ -838,6 +838,7 @@ static int lib_prefix_list_entry_create(struct nb_cb_create_args *args)
        ple->pl = pl;
        ple->any = 1;
        ple->seq = yang_dnode_get_uint32(args->dnode, "./sequence");
+       nb_running_set_entry(args->dnode, ple);
 
        return NB_OK;
 }
@@ -850,7 +851,10 @@ static int lib_prefix_list_entry_destroy(struct nb_cb_destroy_args *args)
                return NB_OK;
 
        ple = nb_running_unset_entry(args->dnode);
-       prefix_list_entry_delete(ple->pl, ple, 1);
+       if (ple->installed)
+               prefix_list_entry_delete(ple->pl, ple, 0);
+       else
+               prefix_list_entry_free(ple);
 
        return NB_OK;
 }
@@ -867,12 +871,19 @@ static int lib_prefix_list_entry_action_modify(struct nb_cb_modify_args *args)
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        action_str = yang_dnode_get_string(args->dnode, NULL);
        if (strcmp(action_str, "permit") == 0)
                ple->type = PREFIX_PERMIT;
        else
                ple->type = PREFIX_DENY;
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -888,8 +899,15 @@ lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_args *args)
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        yang_dnode_get_prefix(&ple->prefix, args->dnode, NULL);
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -902,9 +920,16 @@ lib_prefix_list_entry_ipv4_prefix_destroy(struct nb_cb_destroy_args *args)
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        memset(&ple->prefix, 0, sizeof(ple->prefix));
        ple->any = 1;
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -924,8 +949,15 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_modify(
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        ple->ge = yang_dnode_get_uint8(args->dnode, NULL);
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -938,8 +970,15 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_destroy(
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        ple->ge = 0;
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -959,8 +998,15 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_modify(
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        ple->le = yang_dnode_get_uint8(args->dnode, NULL);
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -973,8 +1019,15 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy(
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        ple->le = 0;
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -1038,9 +1091,16 @@ static int lib_prefix_list_entry_any_create(struct nb_cb_create_args *args)
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        memset(&ple->prefix, 0, sizeof(ple->prefix));
        ple->any = 1;
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 
@@ -1052,9 +1112,16 @@ static int lib_prefix_list_entry_any_destroy(struct nb_cb_destroy_args *args)
                return NB_OK;
 
        ple = nb_running_get_entry(args->dnode, NULL, true);
+
+       /* Start prefix entry update procedure. */
+       prefix_list_entry_update_start(ple);
+
        memset(&ple->prefix, 0, sizeof(ple->prefix));
        ple->any = 1;
 
+       /* Finish prefix entry update procedure. */
+       prefix_list_entry_update_finish(ple);
+
        return NB_OK;
 }
 

diff --git a/lib/plist.c b/lib/plist.c
index aa420da927731620671bf72ba7cc75f4d58447d9..9567fde372b8b428c212cdd409e73811899556b7 100644 (file)
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -187,7 +187,7 @@ struct prefix_list_entry *prefix_list_entry_new(void)
        return new;
 }
 
-static void prefix_list_entry_free(struct prefix_list_entry *pentry)
+void prefix_list_entry_free(struct prefix_list_entry *pentry)
 {
        XFREE(MTYPE_PREFIX_LIST_ENTRY, pentry);
 }
@@ -646,6 +646,58 @@ static void prefix_list_entry_add(struct prefix_list *plist,
        plist->master->recent = plist;
 }
 
+/**
+ * Prefix list entry update start procedure:
+ * Remove entry from previosly installed tries and notify observers..
+ *
+ * \param[in] ple prefix list entry.
+ */
+void prefix_list_entry_update_start(struct prefix_list_entry *ple)
+{
+       struct prefix_list *pl = ple->pl;
+
+       /* Not installed, nothing to do. */
+       if (!ple->installed)
+               return;
+
+       prefix_list_trie_del(pl, ple);
+       route_map_notify_pentry_dependencies(pl->name, ple,
+                                            RMAP_EVENT_PLIST_DELETED);
+       pl->count--;
+
+       ple->installed = false;
+}
+
+/**
+ * Prefix list entry update finish procedure:
+ * Add entry back to trie, notify observers and call master hook.
+ *
+ * \param[in] ple prefix list entry.
+ */
+void prefix_list_entry_update_finish(struct prefix_list_entry *ple)
+{
+       struct prefix_list *pl = ple->pl;
+
+       /* Already installed, nothing to do. */
+       if (ple->installed)
+               return;
+
+       prefix_list_trie_add(pl, ple);
+       pl->count++;
+
+       route_map_notify_pentry_dependencies(pl->name, ple,
+                                            RMAP_EVENT_PLIST_ADDED);
+
+       /* Run hook function. */
+       if (pl->master->add_hook)
+               (*pl->master->add_hook)(pl);
+
+       route_map_notify_dependencies(pl->name, RMAP_EVENT_PLIST_ADDED);
+       pl->master->recent = pl;
+
+       ple->installed = true;
+}
+
 /* Return string of prefix_list_type. */
 static const char *prefix_list_type_str(struct prefix_list_entry *pentry)
 {

diff --git a/lib/plist_int.h b/lib/plist_int.h
index b180d18081cf59d8c23f181d5ccb63a19c421067..cef78a3ef7a089764776279b603b87b4a7a07efa 100644 (file)
--- a/lib/plist_int.h
+++ b/lib/plist_int.h
@@ -72,8 +72,15 @@ struct prefix_list_entry {
 
        /* up the chain for best match search */
        struct prefix_list_entry *next_best;
+
+       /* Flag to track trie/list installation status. */
+       bool installed;
 };
 
+extern void prefix_list_entry_free(struct prefix_list_entry *pentry);
+extern void prefix_list_entry_update_start(struct prefix_list_entry *ple);
+extern void prefix_list_entry_update_finish(struct prefix_list_entry *ple);
+
 #ifdef __cplusplus
 }
 #endif