From 64407418349f11587382db56c121ef2dea1fb7b3 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Sun, 13 Dec 2020 19:04:05 -0800 Subject: [PATCH 1/6] yang: remove when statement from prefix-list Remove when statements from prefix-list yang OM, and do the same check in frr validation phase. This helps a bit in perfomance of prefix-lists scale config. Ticket:CM-32035 Reviewed By:CCR-11096 Testing Done: Signed-off-by: Chirag Shah --- lib/filter_nb.c | 280 +++++++++++++++++++++++++++++++++++++++---- yang/frr-filter.yang | 4 - 2 files changed, 257 insertions(+), 27 deletions(-) diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 2007b37cdf..cab108e4ba 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -1148,25 +1148,11 @@ static int lib_prefix_list_entry_action_modify(struct nb_cb_modify_args *args) return NB_OK; } -/* - * XPath: /frr-filter:lib/prefix-list/entry/ipv4-prefix - */ -static int -lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_args *args) +static int lib_prefix_list_entry_prefix_modify(struct nb_cb_modify_args *args) { struct prefix_list_entry *ple; struct prefix p; - 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; @@ -1193,8 +1179,7 @@ lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_args *args) return NB_OK; } -static int -lib_prefix_list_entry_ipv4_prefix_destroy(struct nb_cb_destroy_args *args) +static int lib_prefix_list_entry_prefix_destroy(struct nb_cb_destroy_args *args) { struct prefix_list_entry *ple; @@ -1214,6 +1199,67 @@ lib_prefix_list_entry_ipv4_prefix_destroy(struct nb_cb_destroy_args *args) return NB_OK; } +/* + * XPath: /frr-filter:lib/prefix-list/entry/ipv4-prefix + */ +static int +lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_args *args) +{ + + if (args->event == NB_EV_VALIDATE) { + return NB_OK; + } + + if (args->event != NB_EV_APPLY) + return NB_OK; + + return lib_prefix_list_entry_prefix_modify(args); +} + +static int +lib_prefix_list_entry_ipv4_prefix_destroy(struct nb_cb_destroy_args *args) +{ + + if (args->event == NB_EV_VALIDATE) { + return NB_OK; + } + + if (args->event != NB_EV_APPLY) + return NB_OK; + + return lib_prefix_list_entry_prefix_destroy(args); +} + +/* + * XPath: /frr-filter:lib/prefix-list/entry/ipv6-prefix + */ +static int +lib_prefix_list_entry_ipv6_prefix_modify(struct nb_cb_modify_args *args) +{ + + if (args->event == NB_EV_VALIDATE) { + return NB_OK; + } + + if (args->event != NB_EV_APPLY) + return NB_OK; + + return lib_prefix_list_entry_prefix_modify(args); +} + +static int +lib_prefix_list_entry_ipv6_prefix_destroy(struct nb_cb_destroy_args *args) +{ + + if (args->event == NB_EV_VALIDATE) { + } + + if (args->event != NB_EV_APPLY) + return NB_OK; + + return lib_prefix_list_entry_prefix_destroy(args); +} + /* * XPath: /frr-filter:lib/prefix-list/entry/ipv4-prefix-length-greater-or-equal */ @@ -1256,6 +1302,19 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_destroy( struct nb_cb_destroy_args *args) { struct prefix_list_entry *ple; + const char *af_type; + + if (args->event == NB_EV_VALIDATE) { + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(args->dnode, "prefix-list"); + af_type = yang_dnode_get_string(entry_dnode, "./type"); + if (strcmp(af_type, "ipv4")) { + snprintf(args->errmsg, args->errmsg_len, + "prefix-list type %s is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + return NB_OK; + } if (args->event != NB_EV_APPLY) return NB_OK; @@ -1315,7 +1374,182 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy( struct nb_cb_destroy_args *args) { struct prefix_list_entry *ple; + const char *af_type; + if (args->event == NB_EV_VALIDATE) { + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(args->dnode, "prefix-list"); + af_type = yang_dnode_get_string(entry_dnode, "./type"); + if (strcmp(af_type, "ipv4")) { + snprintf(args->errmsg, args->errmsg_len, + "prefix-list type %s is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + return NB_OK; + } + + 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 = 0; + + /* Finish prefix entry update procedure. */ + prefix_list_entry_update_finish(ple); + + return NB_OK; +} + +/* + * XPath: /frr-filter:lib/prefix-list/entry/ipv6-prefix-length-greater-or-equal + */ +static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( + struct nb_cb_modify_args *args) +{ + struct prefix_list_entry *ple; + const char *af_type; + + if (args->event == NB_EV_VALIDATE + && prefix_list_length_validate(args) != NB_OK) + return NB_ERR_VALIDATION; + + if (args->event == NB_EV_VALIDATE) { + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(args->dnode, "prefix-list"); + af_type = yang_dnode_get_string(entry_dnode, "./type"); + if (strcmp(af_type, "ipv6")) { + snprintf(args->errmsg, args->errmsg_len, + "prefix-list type %s is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + + 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; + + 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_ipv6_prefix_length_greater_or_equal_destroy( + struct nb_cb_destroy_args *args) +{ + struct prefix_list_entry *ple; + const char *af_type; + + if (args->event == NB_EV_VALIDATE) { + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(args->dnode, "prefix-list"); + af_type = yang_dnode_get_string(entry_dnode, "./type"); + if (strcmp(af_type, "ipv6")) { + snprintf(args->errmsg, args->errmsg_len, + "prefix-list type %s is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + return NB_OK; + } + + 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->le = 0; + + /* Finish prefix entry update procedure. */ + prefix_list_entry_update_finish(ple); + + return NB_OK; +} + +/* + * XPath: /frr-filter:lib/prefix-list/entry/ipv6-prefix-length-lesser-or-equal + */ +static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify( + struct nb_cb_modify_args *args) +{ + struct prefix_list_entry *ple; + const char *af_type; + + if (args->event == NB_EV_VALIDATE + && prefix_list_length_validate(args) != NB_OK) + return NB_ERR_VALIDATION; + + if (args->event == NB_EV_VALIDATE) { + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(args->dnode, "prefix-list"); + af_type = yang_dnode_get_string(entry_dnode, "./type"); + if (strcmp(af_type, "ipv6")) { + snprintf(args->errmsg, args->errmsg_len, + "prefix-list type %s is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + 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; + + 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; +} + +static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_destroy( + struct nb_cb_destroy_args *args) +{ + struct prefix_list_entry *ple; + const char *af_type; + + if (args->event == NB_EV_VALIDATE) { + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(args->dnode, "prefix-list"); + af_type = yang_dnode_get_string(entry_dnode, "./type"); + if (strcmp(af_type, "ipv6")) { + snprintf(args->errmsg, args->errmsg_len, + "prefix-list type %s is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + return NB_OK; + } if (args->event != NB_EV_APPLY) return NB_OK; @@ -1583,22 +1817,22 @@ const struct frr_yang_module_info frr_filter_info = { { .xpath = "/frr-filter:lib/prefix-list/entry/ipv6-prefix", .cbs = { - .modify = lib_prefix_list_entry_ipv4_prefix_modify, - .destroy = lib_prefix_list_entry_ipv4_prefix_destroy, + .modify = lib_prefix_list_entry_ipv6_prefix_modify, + .destroy = lib_prefix_list_entry_ipv6_prefix_destroy, } }, { .xpath = "/frr-filter:lib/prefix-list/entry/ipv6-prefix-length-greater-or-equal", .cbs = { - .modify = lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_modify, - .destroy = lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_destroy, + .modify = lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify, + .destroy = lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_destroy, } }, { .xpath = "/frr-filter:lib/prefix-list/entry/ipv6-prefix-length-lesser-or-equal", .cbs = { - .modify = lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_modify, - .destroy = lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy, + .modify = lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify, + .destroy = lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_destroy, } }, { diff --git a/yang/frr-filter.yang b/yang/frr-filter.yang index eb84dd7460..9a864213ee 100644 --- a/yang/frr-filter.yang +++ b/yang/frr-filter.yang @@ -292,8 +292,6 @@ module frr-filter { mandatory true; case ipv4-prefix { - when "../type = 'ipv4'"; - leaf ipv4-prefix { description "Configure IPv4 prefix to match"; type inet:ipv4-prefix; @@ -318,8 +316,6 @@ module frr-filter { } } case ipv6-prefix { - when "../type = 'ipv6'"; - leaf ipv6-prefix { description "Configure IPv6 prefix to match"; type inet:ipv6-prefix; From 4d2f546f82860d4680a7b17164d8ce0598c8ebb5 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Thu, 17 Dec 2020 16:18:40 -0800 Subject: [PATCH 2/6] lib: remove prefix-list dup api in validation phase Following patch " lib: disallow access list duplicated values" introduce a libyang dnode iterator for every prefix-list config which adds an overhead of traversal all prefix dnodes and degrades the performance in scaled prefix-list config. This check is not necessary in prefix-list northbound callbacks as there won't be a case where prefix-list config comes to nb callback without sequence number. The dup check is only necessary for the vtysh case for backward compatiblity reason where cli can be accepted without sequence number. Ticket: CM-32035 Reviewed By: CCR-11096 Testing Done: Signed-off-by: Chirag Shah --- lib/filter_nb.c | 82 +------------------------------------------------ 1 file changed, 1 insertion(+), 81 deletions(-) diff --git a/lib/filter_nb.c b/lib/filter_nb.c index cab108e4ba..4e1bde676e 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -308,45 +308,6 @@ 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 */ @@ -1272,16 +1233,6 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_modify( prefix_list_length_validate(args) != NB_OK) return NB_ERR_VALIDATION; - 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; @@ -1344,16 +1295,6 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_modify( prefix_list_length_validate(args) != NB_OK) return NB_ERR_VALIDATION; - 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; @@ -1427,12 +1368,6 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( return NB_ERR_VALIDATION; } - 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; } @@ -1508,12 +1443,7 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify( "prefix-list type %s is mismatched.", af_type); return NB_ERR_VALIDATION; } - 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; } @@ -1574,16 +1504,6 @@ 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 978ca5d5ba9842ad2be0937d696316a343fc9228 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Wed, 3 Feb 2021 09:51:40 -0800 Subject: [PATCH 3/6] lib: plist validation use enum type In prefix-list nortbound callback's validation phase, use type as enum rather string for better performance. Signed-off-by: Chirag Shah --- lib/filter_nb.c | 93 ++++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 4e1bde676e..fabf71adf8 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -1167,13 +1167,20 @@ static int lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_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_IPV4) { + snprintf(args->errmsg, args->errmsg_len, + "prefix-list type %u is mismatched.", af_type); + return NB_ERR_VALIDATION; + } return NB_OK; } - if (args->event != NB_EV_APPLY) - return NB_OK; - return lib_prefix_list_entry_prefix_modify(args); } @@ -1181,10 +1188,6 @@ static int lib_prefix_list_entry_ipv4_prefix_destroy(struct nb_cb_destroy_args *args) { - if (args->event == NB_EV_VALIDATE) { - return NB_OK; - } - if (args->event != NB_EV_APPLY) return NB_OK; @@ -1198,13 +1201,20 @@ static int lib_prefix_list_entry_ipv6_prefix_modify(struct nb_cb_modify_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; } - if (args->event != NB_EV_APPLY) - return NB_OK; - return lib_prefix_list_entry_prefix_modify(args); } @@ -1212,9 +1222,6 @@ static int lib_prefix_list_entry_ipv6_prefix_destroy(struct nb_cb_destroy_args *args) { - if (args->event == NB_EV_VALIDATE) { - } - if (args->event != NB_EV_APPLY) return NB_OK; @@ -1253,15 +1260,15 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_destroy( struct nb_cb_destroy_args *args) { struct prefix_list_entry *ple; - const char *af_type; + int af_type; if (args->event == NB_EV_VALIDATE) { - const struct lyd_node *entry_dnode = + const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); - af_type = yang_dnode_get_string(entry_dnode, "./type"); - if (strcmp(af_type, "ipv4")) { + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV4) { snprintf(args->errmsg, args->errmsg_len, - "prefix-list type %s is mismatched.", af_type); + "prefix-list type %u is mismatched.", af_type); return NB_ERR_VALIDATION; } return NB_OK; @@ -1315,15 +1322,15 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy( struct nb_cb_destroy_args *args) { struct prefix_list_entry *ple; - const char *af_type; + int af_type; if (args->event == NB_EV_VALIDATE) { - const struct lyd_node *entry_dnode = + const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); - af_type = yang_dnode_get_string(entry_dnode, "./type"); - if (strcmp(af_type, "ipv4")) { + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV4) { snprintf(args->errmsg, args->errmsg_len, - "prefix-list type %s is mismatched.", af_type); + "prefix-list type %u is mismatched.", af_type); return NB_ERR_VALIDATION; } return NB_OK; @@ -1352,19 +1359,19 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( struct nb_cb_modify_args *args) { struct prefix_list_entry *ple; - const char *af_type; + int af_type; if (args->event == NB_EV_VALIDATE && prefix_list_length_validate(args) != NB_OK) return NB_ERR_VALIDATION; if (args->event == NB_EV_VALIDATE) { - const struct lyd_node *entry_dnode = + const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); - af_type = yang_dnode_get_string(entry_dnode, "./type"); - if (strcmp(af_type, "ipv6")) { + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV6) { snprintf(args->errmsg, args->errmsg_len, - "prefix-list type %s is mismatched.", af_type); + "prefix-list type %u is mismatched.", af_type); return NB_ERR_VALIDATION; } @@ -1391,15 +1398,15 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_destroy( struct nb_cb_destroy_args *args) { struct prefix_list_entry *ple; - const char *af_type; + int af_type; if (args->event == NB_EV_VALIDATE) { - const struct lyd_node *entry_dnode = + const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); - af_type = yang_dnode_get_string(entry_dnode, "./type"); - if (strcmp(af_type, "ipv6")) { + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV6) { snprintf(args->errmsg, args->errmsg_len, - "prefix-list type %s is mismatched.", af_type); + "prefix-list type %u is mismatched.", af_type); return NB_ERR_VALIDATION; } return NB_OK; @@ -1428,19 +1435,19 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify( struct nb_cb_modify_args *args) { struct prefix_list_entry *ple; - const char *af_type; + int af_type; if (args->event == NB_EV_VALIDATE && prefix_list_length_validate(args) != NB_OK) return NB_ERR_VALIDATION; if (args->event == NB_EV_VALIDATE) { - const struct lyd_node *entry_dnode = + const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); - af_type = yang_dnode_get_string(entry_dnode, "./type"); - if (strcmp(af_type, "ipv6")) { + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV6) { snprintf(args->errmsg, args->errmsg_len, - "prefix-list type %s is mismatched.", af_type); + "prefix-list type %u is mismatched.", af_type); return NB_ERR_VALIDATION; } @@ -1467,15 +1474,15 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_destroy( struct nb_cb_destroy_args *args) { struct prefix_list_entry *ple; - const char *af_type; + int af_type; if (args->event == NB_EV_VALIDATE) { - const struct lyd_node *entry_dnode = + const struct lyd_node *plist_dnode = yang_dnode_get_parent(args->dnode, "prefix-list"); - af_type = yang_dnode_get_string(entry_dnode, "./type"); - if (strcmp(af_type, "ipv6")) { + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV6) { snprintf(args->errmsg, args->errmsg_len, - "prefix-list type %s is mismatched.", af_type); + "prefix-list type %u is mismatched.", af_type); return NB_ERR_VALIDATION; } return NB_OK; From b12bcae46241e95a5207d3bf45aa8633b553a13e Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Wed, 3 Feb 2021 10:45:40 -0800 Subject: [PATCH 4/6] lib: fix plist le ge reset value merge conflict leads to incorrect reset value for prefix-list less than and greater than equal values. Signed-off-by: Chirag Shah --- lib/filter_nb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/filter_nb.c b/lib/filter_nb.c index fabf71adf8..9ca4a62cf5 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -1344,7 +1344,7 @@ static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy( /* Start prefix entry update procedure. */ prefix_list_entry_update_start(ple); - ple->ge = 0; + ple->le = 0; /* Finish prefix entry update procedure. */ prefix_list_entry_update_finish(ple); @@ -1420,7 +1420,7 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_destroy( /* Start prefix entry update procedure. */ prefix_list_entry_update_start(ple); - ple->le = 0; + ple->ge = 0; /* Finish prefix entry update procedure. */ prefix_list_entry_update_finish(ple); From f7f101156eb0e225f375f12cf4f863ebbe3fed03 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Mon, 25 Jan 2021 11:44:56 -0800 Subject: [PATCH 5/6] lib: fix a crash in plist update Problem: Prefix-list with mulitiple rules, an update to a rule/sequence with different prefix/prefixlen reset prefix-list next-base pointer to avoid having stale value. In some case the old next-bast's reference leads to an assert in tri (trie_install_fn ) add. bt: (object=0x55576a4c8a00, updptr=0x55576a4b97e0) at lib/plist.c:560 (plist=0x55576a4a1770, pentry=0x55576a4c8a00) at lib/plist.c:585 (ple=0x55576a4c8a00) at lib/plist.c:745 (args=0x7fffe04beb50) at lib/filter_nb.c:1181 Solution: Reset prefix-list next-base pointer whenver a sequence/rule is updated. Ticket:CM-33109 Testing Done: Signed-off-by: Chirag Shah Signed-off-by: Rafael Zalamena --- lib/plist.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/plist.c b/lib/plist.c index 4588dfe1d3..fe4689becd 100644 --- a/lib/plist.c +++ b/lib/plist.c @@ -684,6 +684,7 @@ void prefix_list_entry_update_start(struct prefix_list_entry *ple) if (pl->head || pl->tail || pl->desc) pl->master->recent = pl; + ple->next_best = NULL; ple->installed = false; } From fae60215832253871ccc8e72a047add7dcc89360 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Thu, 11 Feb 2021 21:22:06 -0800 Subject: [PATCH 6/6] lib: consolidate plist nb callbacks Signed-off-by: Chirag Shah --- lib/filter_nb.c | 285 ++++++++++++++++++++++-------------------------- 1 file changed, 132 insertions(+), 153 deletions(-) diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 9ca4a62cf5..c83738e729 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -120,6 +120,101 @@ static void prefix_list_entry_set_empty(struct prefix_list_entry *ple) ple->le = 0; } +static int +prefix_list_nb_validate_v4_af_type(const struct lyd_node *plist_dnode, + char *errmsg, size_t errmsg_len) +{ + int af_type; + + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV4) { + snprintf(errmsg, errmsg_len, + "prefix-list type %u is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + + return NB_OK; +} + +static int +prefix_list_nb_validate_v6_af_type(const struct lyd_node *plist_dnode, + char *errmsg, size_t errmsg_len) +{ + int af_type; + + af_type = yang_dnode_get_enum(plist_dnode, "./type"); + if (af_type != YPLT_IPV6) { + snprintf(errmsg, errmsg_len, + "prefix-list type %u is mismatched.", af_type); + return NB_ERR_VALIDATION; + } + + return NB_OK; +} + +static int lib_prefix_list_entry_prefix_length_lesser_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->le = 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_greater_or_equal_destroy( + struct nb_cb_destroy_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 = 0; + + /* 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_destroy( + struct nb_cb_destroy_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->le = 0; + + /* Finish prefix entry update procedure. */ + prefix_list_entry_update_finish(ple); + + return NB_OK; +} + /** * Unsets the cisco style rule for addresses so it becomes disabled (the * equivalent of setting: `0.0.0.0/32`). @@ -1166,19 +1261,12 @@ static int lib_prefix_list_entry_prefix_destroy(struct nb_cb_destroy_args *args) static int lib_prefix_list_entry_ipv4_prefix_modify(struct nb_cb_modify_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_IPV4) { - 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_v4_af_type( + plist_dnode, args->errmsg, args->errmsg_len); } return lib_prefix_list_entry_prefix_modify(args); @@ -1201,18 +1289,12 @@ static int lib_prefix_list_entry_ipv6_prefix_modify(struct nb_cb_modify_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_modify(args); @@ -1259,35 +1341,16 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_modify( static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_destroy( struct nb_cb_destroy_args *args) { - struct prefix_list_entry *ple; - 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_IPV4) { - 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_v4_af_type( + 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 = 0; - - /* Finish prefix entry update procedure. */ - prefix_list_entry_update_finish(ple); - - return NB_OK; + return lib_prefix_list_entry_prefix_length_greater_or_equal_destroy( + args); } /* @@ -1296,60 +1359,34 @@ static int lib_prefix_list_entry_ipv4_prefix_length_greater_or_equal_destroy( static int lib_prefix_list_entry_ipv4_prefix_length_lesser_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->le = 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_lesser_or_equal_modify(args); } static int lib_prefix_list_entry_ipv4_prefix_length_lesser_or_equal_destroy( struct nb_cb_destroy_args *args) { - struct prefix_list_entry *ple; - 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_IPV4) { - 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_v4_af_type( + 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->le = 0; - - /* Finish prefix entry update procedure. */ - prefix_list_entry_update_finish(ple); - - return NB_OK; + return lib_prefix_list_entry_prefix_length_lesser_or_equal_destroy( + args); } /* @@ -1359,7 +1396,6 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( struct nb_cb_modify_args *args) { struct prefix_list_entry *ple; - int af_type; if (args->event == NB_EV_VALIDATE && prefix_list_length_validate(args) != NB_OK) @@ -1368,14 +1404,9 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( 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); } if (args->event != NB_EV_APPLY) @@ -1397,35 +1428,16 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_modify( static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_destroy( struct nb_cb_destroy_args *args) { - struct prefix_list_entry *ple; - 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); } - 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 = 0; - - /* Finish prefix entry update procedure. */ - prefix_list_entry_update_finish(ple); - - return NB_OK; + return lib_prefix_list_entry_prefix_length_greater_or_equal_destroy( + args); } /* @@ -1434,9 +1446,6 @@ static int lib_prefix_list_entry_ipv6_prefix_length_greater_or_equal_destroy( static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify( struct nb_cb_modify_args *args) { - struct prefix_list_entry *ple; - int af_type; - if (args->event == NB_EV_VALIDATE && prefix_list_length_validate(args) != NB_OK) return NB_ERR_VALIDATION; @@ -1444,36 +1453,17 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_modify( 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); } - 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->le = 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_lesser_or_equal_modify(args); } static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_destroy( struct nb_cb_destroy_args *args) { - struct prefix_list_entry *ple; int af_type; if (args->event == NB_EV_VALIDATE) { @@ -1487,20 +1477,9 @@ static int lib_prefix_list_entry_ipv6_prefix_length_lesser_or_equal_destroy( } return NB_OK; } - 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->le = 0; - - /* Finish prefix entry update procedure. */ - prefix_list_entry_update_finish(ple); - - return NB_OK; + return lib_prefix_list_entry_prefix_length_lesser_or_equal_destroy( + args); } /*