lib: fix access-list entry update

When an access-list entry is updated, current NB code calls notification
hooks for each updated field. It means that when multiple fields of an
entry are changed in a single commit, the hooks are run with an interim
state of an access-list instead of a final one. To fix the issue, we
should call the hooks once, after all fields of an entry are updated.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

diff --git a/lib/filter.c b/lib/filter.c
index a0adff0e35dc36ba12bb4f3cbe573fe2a82bce84..5a0790f8bfc22d8372c07f1ab7f072c2897c6a36 100644 (file)
--- a/lib/filter.c
+++ b/lib/filter.c
@@ -410,7 +410,10 @@ void access_list_filter_add(struct access_list *access,
                filter->prev = access->tail;
                access->tail = filter;
        }
+}
 
+void access_list_filter_update(struct access_list *access)
+{
        /* Run hook function. */
        if (access->master->add_hook)
                (*access->master->add_hook)(access);

diff --git a/lib/filter.h b/lib/filter.h
index bd9e22d38416a5019f80d763013217be14e5d0e4..4fa482ba4ee7813c2910a1f3a1af0283b044d07d 100644 (file)
--- a/lib/filter.h
+++ b/lib/filter.h
@@ -128,6 +128,7 @@ struct filter *filter_new(void);
 void access_list_filter_add(struct access_list *access, struct filter *filter);
 void access_list_filter_delete(struct access_list *access,
                               struct filter *filter);
+void access_list_filter_update(struct access_list *access);
 int64_t filter_new_seq_get(struct access_list *access);
 
 extern const struct frr_yang_module_info frr_filter_info;

diff --git a/lib/filter_nb.c b/lib/filter_nb.c
index ec31f6d39558b8f42a55ae16a6ba83584722d768..91f80dcb5e86cc7a508df924e982e136f3c7a193 100644 (file)
--- a/lib/filter_nb.c
+++ b/lib/filter_nb.c
@@ -17,23 +17,6 @@
 #include "lib/plist_int.h"
 #include "lib/routemap.h"
 
-/* Helper function. */
-static void acl_notify_route_map(struct access_list *acl, int route_map_event)
-{
-       switch (route_map_event) {
-       case RMAP_EVENT_FILTER_ADDED:
-               if (acl->master->add_hook)
-                       (*acl->master->add_hook)(acl);
-               break;
-       case RMAP_EVENT_FILTER_DELETED:
-               if (acl->master->delete_hook)
-                       (*acl->master->delete_hook)(acl);
-               break;
-       }
-
-       route_map_notify_dependencies(acl->name, route_map_event);
-}
-
 static enum nb_error prefix_list_length_validate(struct nb_cb_modify_args *args)
 {
        int type = yang_dnode_get_enum(args->dnode, "../../type");
@@ -575,6 +558,15 @@ static int lib_access_list_entry_destroy(struct nb_cb_destroy_args *args)
        return NB_OK;
 }
 
+static void
+lib_access_list_entry_apply_finish(struct nb_cb_apply_finish_args *args)
+{
+       struct filter *f;
+
+       f = nb_running_get_entry(args->dnode, NULL, true);
+       access_list_filter_update(f->acl);
+}
+
 /*
  * XPath: /frr-filter:lib/access-list/entry/action
  */
@@ -594,8 +586,6 @@ lib_access_list_entry_action_modify(struct nb_cb_modify_args *args)
        else
                f->type = FILTER_DENY;
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -629,8 +619,6 @@ lib_access_list_entry_ipv4_prefix_modify(struct nb_cb_modify_args *args)
        fz = &f->u.zfilter;
        yang_dnode_get_prefix(&fz->prefix, args->dnode, NULL);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -647,8 +635,6 @@ lib_access_list_entry_ipv4_prefix_destroy(struct nb_cb_destroy_args *args)
        fz = &f->u.zfilter;
        memset(&fz->prefix, 0, sizeof(fz->prefix));
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -681,8 +667,6 @@ lib_access_list_entry_ipv4_exact_match_modify(struct nb_cb_modify_args *args)
        fz = &f->u.zfilter;
        fz->exact = yang_dnode_get_bool(args->dnode, NULL);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -699,8 +683,6 @@ lib_access_list_entry_ipv4_exact_match_destroy(struct nb_cb_destroy_args *args)
        fz = &f->u.zfilter;
        fz->exact = 0;
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -733,8 +715,6 @@ lib_access_list_entry_host_modify(struct nb_cb_modify_args *args)
        yang_dnode_get_ipv4(&fc->addr, args->dnode, NULL);
        fc->addr_mask.s_addr = CISCO_BIN_HOST_WILDCARD_MASK;
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -751,8 +731,6 @@ lib_access_list_entry_host_destroy(struct nb_cb_destroy_args *args)
        fc = &f->u.cfilter;
        cisco_unset_addr_mask(&fc->addr, &fc->addr_mask);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -777,8 +755,6 @@ static int lib_access_list_entry_network_destroy(struct nb_cb_destroy_args *args
        fc = &f->u.cfilter;
        cisco_unset_addr_mask(&fc->addr, &fc->addr_mask);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -810,8 +786,6 @@ lib_access_list_entry_network_address_modify(struct nb_cb_modify_args *args)
        fc = &f->u.cfilter;
        yang_dnode_get_ipv4(&fc->addr, args->dnode, NULL);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -843,8 +817,6 @@ lib_access_list_entry_network_mask_modify(struct nb_cb_modify_args *args)
        fc = &f->u.cfilter;
        yang_dnode_get_ipv4(&fc->addr_mask, args->dnode, NULL);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -877,8 +849,6 @@ lib_access_list_entry_source_any_create(struct nb_cb_create_args *args)
        fc->addr.s_addr = INADDR_ANY;
        fc->addr_mask.s_addr = CISCO_BIN_ANY_WILDCARD_MASK;
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -895,8 +865,6 @@ lib_access_list_entry_source_any_destroy(struct nb_cb_destroy_args *args)
        fc = &f->u.cfilter;
        cisco_unset_addr_mask(&fc->addr, &fc->addr_mask);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -929,8 +897,6 @@ static int lib_access_list_entry_destination_host_modify(
        yang_dnode_get_ipv4(&fc->mask, args->dnode, NULL);
        fc->mask_mask.s_addr = CISCO_BIN_HOST_WILDCARD_MASK;
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -948,8 +914,6 @@ static int lib_access_list_entry_destination_host_destroy(
        fc->extended = 0;
        cisco_unset_addr_mask(&fc->mask, &fc->mask_mask);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -977,8 +941,6 @@ static int lib_access_list_entry_destination_network_destroy(
        fc->extended = 0;
        cisco_unset_addr_mask(&fc->mask, &fc->mask_mask);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -1010,8 +972,6 @@ static int lib_access_list_entry_destination_network_address_modify(
        fc->extended = 1;
        yang_dnode_get_ipv4(&fc->mask, args->dnode, NULL);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -1043,8 +1003,6 @@ static int lib_access_list_entry_destination_network_mask_modify(
        fc->extended = 1;
        yang_dnode_get_ipv4(&fc->mask_mask, args->dnode, NULL);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -1077,8 +1035,6 @@ static int lib_access_list_entry_destination_any_create(
        fc->mask.s_addr = INADDR_ANY;
        fc->mask_mask.s_addr = CISCO_BIN_ANY_WILDCARD_MASK;
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -1096,8 +1052,6 @@ static int lib_access_list_entry_destination_any_destroy(
        fc->extended = 0;
        cisco_unset_addr_mask(&fc->mask, &fc->mask_mask);
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -1144,8 +1098,6 @@ static int lib_access_list_entry_any_create(struct nb_cb_create_args *args)
                break;
        }
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
-
        return NB_OK;
 }
 
@@ -1161,8 +1113,6 @@ static int lib_access_list_entry_any_destroy(struct nb_cb_destroy_args *args)
        fz = &f->u.zfilter;
        fz->prefix.family = AF_UNSPEC;
 
-       acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
-
        return NB_OK;
 }
 
@@ -1652,6 +1602,7 @@ const struct frr_yang_module_info frr_filter_info = {
                        .cbs = {
                                .create = lib_access_list_entry_create,
                                .destroy = lib_access_list_entry_destroy,
+                               .apply_finish = lib_access_list_entry_apply_finish,
                                .cli_cmp = access_list_cmp,
                                .cli_show = access_list_show,
                        }