From 18abe2b91ef41d4c6ccdd19ac505f6705db64f87 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 29 Mar 2021 21:26:28 +0300 Subject: [PATCH 1/4] lib: fix checking for duplicated access-list entries Restore the behavior that was before the NB conversion. Signed-off-by: Igor Ryzhov --- lib/filter.h | 3 +++ lib/filter_cli.c | 5 +++++ lib/filter_nb.c | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/lib/filter.h b/lib/filter.h index 091a5197f6..337639c4b7 100644 --- a/lib/filter.h +++ b/lib/filter.h @@ -182,6 +182,9 @@ struct acl_dup_args { /** Access list name. */ const char *ada_name; + /** Entry action. */ + const char *ada_action; + #define ADA_MAX_VALUES 4 /** Entry XPath for value. */ const char *ada_xpath[ADA_MAX_VALUES]; diff --git a/lib/filter_cli.c b/lib/filter_cli.c index 5d66a9fc73..0c49665fd4 100644 --- a/lib/filter_cli.c +++ b/lib/filter_cli.c @@ -173,6 +173,7 @@ DEFPY_YANG( if (seq_str == NULL) { ada.ada_type = "ipv4"; ada.ada_name = name; + ada.ada_action = action; if (host_str && mask_str == NULL) { ada.ada_xpath[0] = "./host"; ada.ada_value[0] = host_str; @@ -309,6 +310,7 @@ DEFPY_YANG( if (seq_str == NULL) { ada.ada_type = "ipv4"; ada.ada_name = name; + ada.ada_action = action; if (src_str && src_mask_str == NULL) { ada.ada_xpath[idx] = "./host"; ada.ada_value[idx] = src_str; @@ -504,6 +506,7 @@ DEFPY_YANG( if (seq_str == NULL) { ada.ada_type = "ipv4"; ada.ada_name = name; + ada.ada_action = action; if (prefix_str) { ada.ada_xpath[0] = "./ipv4-prefix"; @@ -701,6 +704,7 @@ DEFPY_YANG( if (seq_str == NULL) { ada.ada_type = "ipv6"; ada.ada_name = name; + ada.ada_action = action; if (prefix_str) { ada.ada_xpath[0] = "./ipv6-prefix"; @@ -902,6 +906,7 @@ DEFPY_YANG( if (seq_str == NULL) { ada.ada_type = "mac"; ada.ada_name = name; + ada.ada_action = action; if (mac_str) { ada.ada_xpath[0] = "./mac"; diff --git a/lib/filter_nb.c b/lib/filter_nb.c index c83738e729..36ce5429c6 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -238,6 +238,9 @@ static int _acl_is_dup(const struct lyd_node *dnode, void *arg) && ada->ada_entry_dnode == dnode) return YANG_ITER_CONTINUE; + if (strcmp(yang_dnode_get_string(dnode, "action"), ada->ada_action)) + return YANG_ITER_CONTINUE; + /* Check if all values match. */ for (idx = 0; idx < ADA_MAX_VALUES; idx++) { /* No more values. */ @@ -292,6 +295,7 @@ static bool acl_cisco_is_dup(const struct lyd_node *dnode) /* Initialize. */ ada.ada_type = "ipv4"; ada.ada_name = yang_dnode_get_string(entry_dnode, "../name"); + ada.ada_action = yang_dnode_get_string(entry_dnode, "action"); ada.ada_entry_dnode = entry_dnode; /* Load all values/XPaths. */ @@ -341,6 +345,7 @@ static bool acl_zebra_is_dup(const struct lyd_node *dnode, break; } ada.ada_name = yang_dnode_get_string(entry_dnode, "../name"); + ada.ada_action = yang_dnode_get_string(entry_dnode, "action"); ada.ada_entry_dnode = entry_dnode; /* Load all values/XPaths. */ From 4c53bdb1b90e2b60445cb6c33a83ff12f01b6c33 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 29 Mar 2021 22:53:36 +0300 Subject: [PATCH 2/4] lib: finish consolidation of prefix-list nb callbacks There was an attempt to consolidate the code in commit fae60215, but the work was not actually finished and some necessary checks were missed. Let's finish it. Signed-off-by: Igor Ryzhov --- lib/filter_nb.c | 83 ++++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 36ce5429c6..bb34efd34c 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -152,6 +152,27 @@ prefix_list_nb_validate_v6_af_type(const struct lyd_node *plist_dnode, return NB_OK; } +static int lib_prefix_list_entry_prefix_length_greater_or_equal_modify( + struct nb_cb_modify_args *args) +{ + struct prefix_list_entry *ple; + + if (args->event != NB_EV_APPLY) + 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; +} + static int lib_prefix_list_entry_prefix_length_lesser_or_equal_modify( struct nb_cb_modify_args *args) { @@ -1321,26 +1342,20 @@ lib_prefix_list_entry_ipv6_prefix_destroy(struct nb_cb_destroy_args *args) static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_modify( struct nb_cb_modify_args *args) { - struct prefix_list_entry *ple; - - if (args->event == NB_EV_VALIDATE && - prefix_list_length_validate(args) != NB_OK) + if (args->event == NB_EV_VALIDATE + && prefix_list_length_validate(args) != NB_OK) return NB_ERR_VALIDATION; - if (args->event != NB_EV_APPLY) - return NB_OK; + if (args->event == NB_EV_VALIDATE) { + const struct lyd_node *plist_dnode = + yang_dnode_get_parent(args->dnode, "prefix-list"); - ple = nb_running_get_entry(args->dnode, NULL, true); + return prefix_list_nb_validate_v4_af_type( + plist_dnode, args->errmsg, args->errmsg_len); + } - /* 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; + return lib_prefix_list_entry_prefix_length_greater_or_equal_modify( + args); } static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_destroy( @@ -1376,7 +1391,8 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_modify( plist_dnode, args->errmsg, args->errmsg_len); } - return lib_prefix_list_entry_prefix_length_lesser_or_equal_modify(args); + return lib_prefix_list_entry_prefix_length_lesser_or_equal_modify( + args); } static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy( @@ -1400,8 +1416,6 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy( static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( struct nb_cb_modify_args *args) { - struct prefix_list_entry *ple; - if (args->event == NB_EV_VALIDATE && prefix_list_length_validate(args) != NB_OK) return NB_ERR_VALIDATION; @@ -1414,20 +1428,8 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( plist_dnode, args->errmsg, args->errmsg_len); } - if (args->event != NB_EV_APPLY) - 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; + return lib_prefix_list_entry_prefix_length_greater_or_equal_modify( + args); } static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_destroy( @@ -1463,24 +1465,19 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify( plist_dnode, args->errmsg, args->errmsg_len); } - return lib_prefix_list_entry_prefix_length_lesser_or_equal_modify(args); + return lib_prefix_list_entry_prefix_length_lesser_or_equal_modify( + args); } static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_destroy( struct nb_cb_destroy_args *args) { - int af_type; - if (args->event == NB_EV_VALIDATE) { const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); - af_type = yang_dnode_get_enum(plist_dnode, "./type"); - if (af_type != YPLT_IPV6) { - snprintf(args->errmsg, args->errmsg_len, - "prefix-list type %u is mismatched.", af_type); - return NB_ERR_VALIDATION; - } - return NB_OK; + + return prefix_list_nb_validate_v6_af_type( + plist_dnode, args->errmsg, args->errmsg_len); } return lib_prefix_list_entry_prefix_length_lesser_or_equal_destroy( From bf79e923167c79b2e813b7e7efda7211949d6378 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 30 Mar 2021 00:25:19 +0300 Subject: [PATCH 3/4] lib: restore checks for duplicated prefix-list entries The checks were incorrectly removed in commit 4d2f546f under the assumption that it is needed only in CLI. Actually the checks are needed for the case when the sequence number is explicitly set by a user. Signed-off-by: Igor Ryzhov --- lib/filter_nb.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/lib/filter_nb.c b/lib/filter_nb.c index bb34efd34c..50325a39f9 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -429,6 +429,45 @@ bool plist_is_dup(const struct lyd_node *dnode, struct plist_dup_args *pda) return pda->pda_found; } +static bool plist_is_dup_nb(const struct lyd_node *dnode) +{ + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(dnode, "entry"); + struct plist_dup_args pda = {}; + int idx = 0, arg_idx = 0; + static const char *entries[] = { + "./ipv4-prefix", + "./ipv4-prefix-length-greater-or-equal", + "./ipv4-prefix-length-lesser-or-equal", + "./ipv6-prefix", + "./ipv6-prefix-length-greater-or-equal", + "./ipv6-prefix-length-lesser-or-equal", + "./any", + NULL + }; + + /* Initialize. */ + pda.pda_type = yang_dnode_get_string(entry_dnode, "../type"); + pda.pda_name = yang_dnode_get_string(entry_dnode, "../name"); + pda.pda_entry_dnode = entry_dnode; + + /* Load all values/XPaths. */ + while (entries[idx] != NULL) { + if (!yang_dnode_exists(entry_dnode, entries[idx])) { + idx++; + continue; + } + + pda.pda_xpath[arg_idx] = entries[idx]; + pda.pda_value[arg_idx] = + yang_dnode_get_string(entry_dnode, entries[idx]); + arg_idx++; + idx++; + } + + return plist_is_dup(entry_dnode, &pda); +} + /* * XPath: /frr-filter:lib/access-list */ @@ -1291,6 +1330,13 @@ lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_args *args) const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); + if (plist_is_dup_nb(args->dnode)) { + snprintf(args->errmsg, args->errmsg_len, + "duplicated prefix list value: %s", + yang_dnode_get_string(args->dnode, NULL)); + return NB_ERR_VALIDATION; + } + return prefix_list_nb_validate_v4_af_type( plist_dnode, args->errmsg, args->errmsg_len); } @@ -1319,6 +1365,13 @@ lib_prefix_list_entry_ipv6_prefix_modify(struct nb_cb_modify_args *args) const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); + if (plist_is_dup_nb(args->dnode)) { + snprintf(args->errmsg, args->errmsg_len, + "duplicated prefix list value: %s", + yang_dnode_get_string(args->dnode, NULL)); + return NB_ERR_VALIDATION; + } + return prefix_list_nb_validate_v6_af_type( plist_dnode, args->errmsg, args->errmsg_len); } @@ -1350,6 +1403,13 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_modify( const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); + if (plist_is_dup_nb(args->dnode)) { + snprintf(args->errmsg, args->errmsg_len, + "duplicated prefix list value: %s", + yang_dnode_get_string(args->dnode, NULL)); + return NB_ERR_VALIDATION; + } + return prefix_list_nb_validate_v4_af_type( plist_dnode, args->errmsg, args->errmsg_len); } @@ -1387,6 +1447,13 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_modify( const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); + if (plist_is_dup_nb(args->dnode)) { + snprintf(args->errmsg, args->errmsg_len, + "duplicated prefix list value: %s", + yang_dnode_get_string(args->dnode, NULL)); + return NB_ERR_VALIDATION; + } + return prefix_list_nb_validate_v4_af_type( plist_dnode, args->errmsg, args->errmsg_len); } @@ -1424,6 +1491,13 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); + if (plist_is_dup_nb(args->dnode)) { + snprintf(args->errmsg, args->errmsg_len, + "duplicated prefix list value: %s", + yang_dnode_get_string(args->dnode, NULL)); + return NB_ERR_VALIDATION; + } + return prefix_list_nb_validate_v6_af_type( plist_dnode, args->errmsg, args->errmsg_len); } @@ -1461,6 +1535,13 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify( const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); + if (plist_is_dup_nb(args->dnode)) { + snprintf(args->errmsg, args->errmsg_len, + "duplicated prefix list value: %s", + yang_dnode_get_string(args->dnode, NULL)); + return NB_ERR_VALIDATION; + } + return prefix_list_nb_validate_v6_af_type( plist_dnode, args->errmsg, args->errmsg_len); } @@ -1492,6 +1573,17 @@ static int lib_prefix_list_entry_any_create(struct nb_cb_create_args *args) struct prefix_list_entry *ple; int type; + if (args->event == NB_EV_VALIDATE) { + if (plist_is_dup_nb(args->dnode)) { + snprintf(args->errmsg, args->errmsg_len, + "duplicated prefix list value: %s", + yang_dnode_get_string(args->dnode, NULL)); + return NB_ERR_VALIDATION; + } + + return NB_OK; + } + if (args->event != NB_EV_APPLY) return NB_OK; From 4179f151fe80584167dbeeb260144d0cfa171f16 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 30 Mar 2021 00:37:52 +0300 Subject: [PATCH 4/4] lib: fix checking for duplicated prefix-list entries Restore the behavior that was before the NB conversion. Signed-off-by: Igor Ryzhov --- lib/filter.h | 3 +++ lib/filter_cli.c | 2 ++ lib/filter_nb.c | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/lib/filter.h b/lib/filter.h index 337639c4b7..b1bf1d67ba 100644 --- a/lib/filter.h +++ b/lib/filter.h @@ -212,6 +212,9 @@ struct plist_dup_args { /** Access list name. */ const char *pda_name; + /** Entry action. */ + const char *pda_action; + #define PDA_MAX_VALUES 4 /** Entry XPath for value. */ const char *pda_xpath[PDA_MAX_VALUES]; diff --git a/lib/filter_cli.c b/lib/filter_cli.c index 0c49665fd4..24980f7858 100644 --- a/lib/filter_cli.c +++ b/lib/filter_cli.c @@ -1336,6 +1336,7 @@ DEFPY_YANG( if (seq_str == NULL) { pda.pda_type = "ipv4"; pda.pda_name = name; + pda.pda_action = action; if (prefix_str) { pda.pda_xpath[arg_idx] = "./ipv4-prefix"; pda.pda_value[arg_idx] = prefix_str; @@ -1531,6 +1532,7 @@ DEFPY_YANG( if (seq_str == NULL) { pda.pda_type = "ipv6"; pda.pda_name = name; + pda.pda_action = action; if (prefix_str) { pda.pda_xpath[arg_idx] = "./ipv6-prefix"; pda.pda_value[arg_idx] = prefix_str; diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 50325a39f9..3b650742f3 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -396,6 +396,9 @@ static int _plist_is_dup(const struct lyd_node *dnode, void *arg) && pda->pda_entry_dnode == dnode) return YANG_ITER_CONTINUE; + if (strcmp(yang_dnode_get_string(dnode, "action"), pda->pda_action)) + return YANG_ITER_CONTINUE; + /* Check if all values match. */ for (idx = 0; idx < PDA_MAX_VALUES; idx++) { /* No more values. */ @@ -449,6 +452,7 @@ static bool plist_is_dup_nb(const struct lyd_node *dnode) /* Initialize. */ pda.pda_type = yang_dnode_get_string(entry_dnode, "../type"); pda.pda_name = yang_dnode_get_string(entry_dnode, "../name"); + pda.pda_action = yang_dnode_get_string(entry_dnode, "action"); pda.pda_entry_dnode = entry_dnode; /* Load all values/XPaths. */