From 667dcc277c15c0bddc785f9b949d658f8d815818 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 10 Aug 2021 21:46:37 +0300 Subject: [PATCH] lib: fix prefix-list duplication check Currently, when we check the new prefix-list entry for duplication, we only take filled in fields into account and ignore optional fields. For example, if we already have `ip prefix-list A 0.0.0.0/0 le 32` and we try to add `ip prefix-list A 0.0.0.0/0`, it is treated as duplicate. We should always compare all prefix-list fields when doing the check. Fixes #9355. Signed-off-by: Igor Ryzhov --- lib/filter.h | 9 ++--- lib/filter_cli.c | 102 ++++++++++------------------------------------- lib/filter_nb.c | 85 ++++++++++++++++++++++----------------- 3 files changed, 74 insertions(+), 122 deletions(-) diff --git a/lib/filter.h b/lib/filter.h index 941fabd38b8..d1956ec019f 100644 --- a/lib/filter.h +++ b/lib/filter.h @@ -207,11 +207,10 @@ struct plist_dup_args { /** Entry action. */ const char *pda_action; -#define PDA_MAX_VALUES 4 - /** Entry XPath for value. */ - const char *pda_xpath[PDA_MAX_VALUES]; - /** Entry value to match. */ - const char *pda_value[PDA_MAX_VALUES]; + bool any; + struct prefix prefix; + int ge; + int le; /** Duplicated entry found in list? */ bool pda_found; diff --git a/lib/filter_cli.c b/lib/filter_cli.c index f030ce1b335..45c7544a3b4 100644 --- a/lib/filter_cli.c +++ b/lib/filter_cli.c @@ -1196,11 +1196,9 @@ static int plist_remove_if_empty(struct vty *vty, const char *iptype, static int plist_remove(struct vty *vty, const char *iptype, const char *name, const char *seq, const char *action, - const char *prefix_str, const char *ge_str, - const char *le_str) + union prefixconstptr prefix, int ge, int le) { int64_t sseq; - int arg_idx = 0; struct plist_dup_args pda = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 32]; @@ -1225,43 +1223,13 @@ static int plist_remove(struct vty *vty, const char *iptype, const char *name, pda.pda_type = iptype; pda.pda_name = name; pda.pda_action = action; - if (prefix_str) { - if (strmatch(iptype, "ipv4")) { - pda.pda_xpath[arg_idx] = "./ipv4-prefix"; - pda.pda_value[arg_idx] = prefix_str; - arg_idx++; - if (ge_str) { - pda.pda_xpath[arg_idx] = - "./ipv4-prefix-length-greater-or-equal"; - pda.pda_value[arg_idx] = ge_str; - arg_idx++; - } - if (le_str) { - pda.pda_xpath[arg_idx] = - "./ipv4-prefix-length-lesser-or-equal"; - pda.pda_value[arg_idx] = le_str; - arg_idx++; - } - } else { - pda.pda_xpath[arg_idx] = "./ipv6-prefix"; - pda.pda_value[arg_idx] = prefix_str; - arg_idx++; - if (ge_str) { - pda.pda_xpath[arg_idx] = - "./ipv6-prefix-length-greater-or-equal"; - pda.pda_value[arg_idx] = ge_str; - arg_idx++; - } - if (le_str) { - pda.pda_xpath[arg_idx] = - "./ipv6-prefix-length-lesser-or-equal"; - pda.pda_value[arg_idx] = le_str; - arg_idx++; - } - } + if (prefix.p) { + prefix_copy(&pda.prefix, prefix); + apply_mask(&pda.prefix); + pda.ge = ge; + pda.le = le; } else { - pda.pda_xpath[0] = "./any"; - pda.pda_value[0] = ""; + pda.any = true; } if (plist_is_dup(vty->candidate_config->dnode, &pda)) @@ -1298,7 +1266,6 @@ DEFPY_YANG( "Maximum prefix length\n") { int64_t sseq; - int arg_idx = 0; struct plist_dup_args pda = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 128]; @@ -1312,24 +1279,11 @@ DEFPY_YANG( 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; - arg_idx++; - if (ge_str) { - pda.pda_xpath[arg_idx] = - "./ipv4-prefix-length-greater-or-equal"; - pda.pda_value[arg_idx] = ge_str; - arg_idx++; - } - if (le_str) { - pda.pda_xpath[arg_idx] = - "./ipv4-prefix-length-lesser-or-equal"; - pda.pda_value[arg_idx] = le_str; - arg_idx++; - } + prefix_copy(&pda.prefix, prefix); + pda.ge = ge; + pda.le = le; } else { - pda.pda_xpath[0] = "./any"; - pda.pda_value[0] = ""; + pda.any = true; } /* Duplicated entry without sequence, just quit. */ @@ -1408,8 +1362,8 @@ DEFPY_YANG( "Maximum prefix length to be matched\n" "Maximum prefix length\n") { - return plist_remove(vty, "ipv4", name, seq_str, action, prefix_str, - ge_str, le_str); + return plist_remove(vty, "ipv4", name, seq_str, action, + prefix_str ? prefix : NULL, ge, le); } DEFPY_YANG( @@ -1421,7 +1375,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_SEQ_STR) { - return plist_remove(vty, "ipv4", name, seq_str, NULL, NULL, NULL, NULL); + return plist_remove(vty, "ipv4", name, seq_str, NULL, NULL, 0, 0); } DEFPY_YANG( @@ -1516,7 +1470,6 @@ DEFPY_YANG( "Minimum prefix length\n") { int64_t sseq; - int arg_idx = 0; struct plist_dup_args pda = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 128]; @@ -1530,24 +1483,11 @@ DEFPY_YANG( 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; - arg_idx++; - if (ge_str) { - pda.pda_xpath[arg_idx] = - "./ipv6-prefix-length-greater-or-equal"; - pda.pda_value[arg_idx] = ge_str; - arg_idx++; - } - if (le_str) { - pda.pda_xpath[arg_idx] = - "./ipv6-prefix-length-lesser-or-equal"; - pda.pda_value[arg_idx] = le_str; - arg_idx++; - } + prefix_copy(&pda.prefix, prefix); + pda.ge = ge; + pda.le = le; } else { - pda.pda_xpath[0] = "./any"; - pda.pda_value[0] = ""; + pda.any = true; } /* Duplicated entry without sequence, just quit. */ @@ -1626,8 +1566,8 @@ DEFPY_YANG( "Minimum prefix length to be matched\n" "Minimum prefix length\n") { - return plist_remove(vty, "ipv6", name, seq_str, action, prefix_str, - ge_str, le_str); + return plist_remove(vty, "ipv6", name, seq_str, action, + prefix_str ? prefix : NULL, ge, le); } DEFPY_YANG( @@ -1639,7 +1579,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_SEQ_STR) { - return plist_remove(vty, "ipv6", name, seq_str, NULL, NULL, NULL, NULL); + return plist_remove(vty, "ipv6", name, seq_str, NULL, NULL, 0, 0); } DEFPY_YANG( diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 85805ffa47c..80ea7a57cb2 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -387,10 +387,50 @@ static bool acl_zebra_is_dup(const struct lyd_node *dnode, return acl_is_dup(entry_dnode, &ada); } +static void plist_dnode_to_prefix(const struct lyd_node *dnode, bool *any, + struct prefix *p, int *ge, int *le) +{ + *any = false; + *ge = 0; + *le = 0; + + if (yang_dnode_exists(dnode, "./any")) { + *any = true; + return; + } + + switch (yang_dnode_get_enum(dnode, "../type")) { + case YPLT_IPV4: + yang_dnode_get_prefix(p, dnode, "./ipv4-prefix"); + if (yang_dnode_exists(dnode, + "./ipv4-prefix-length-greater-or-equal")) + *ge = yang_dnode_get_uint8( + dnode, "./ipv4-prefix-length-greater-or-equal"); + if (yang_dnode_exists(dnode, + "./ipv4-prefix-length-lesser-or-equal")) + *le = yang_dnode_get_uint8( + dnode, "./ipv4-prefix-length-lesser-or-equal"); + break; + case YPLT_IPV6: + yang_dnode_get_prefix(p, dnode, "./ipv6-prefix"); + if (yang_dnode_exists(dnode, + "./ipv6-prefix-length-greater-or-equal")) + *ge = yang_dnode_get_uint8( + dnode, "./ipv6-prefix-length-greater-or-equal"); + if (yang_dnode_exists(dnode, + "./ipv6-prefix-length-lesser-or-equal")) + *le = yang_dnode_get_uint8( + dnode, "./ipv6-prefix-length-lesser-or-equal"); + break; + } +} + static int _plist_is_dup(const struct lyd_node *dnode, void *arg) { struct plist_dup_args *pda = arg; - int idx; + struct prefix p; + int ge, le; + bool any; /* This entry is the caller, so skip it. */ if (pda->pda_entry_dnode @@ -400,19 +440,14 @@ static int _plist_is_dup(const struct lyd_node *dnode, void *arg) 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. */ - if (pda->pda_xpath[idx] == NULL) - break; + plist_dnode_to_prefix(dnode, &any, &p, &ge, &le); - /* Not same type, just skip it. */ - if (!yang_dnode_exists(dnode, pda->pda_xpath[idx])) + if (pda->any) { + if (!any) return YANG_ITER_CONTINUE; - - /* Check if different value. */ - if (strcmp(yang_dnode_get_string(dnode, pda->pda_xpath[idx]), - pda->pda_value[idx])) + } else { + if (!prefix_same(&pda->prefix, &p) || pda->ge != ge + || pda->le != le) return YANG_ITER_CONTINUE; } @@ -439,17 +474,6 @@ 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"); @@ -457,19 +481,8 @@ static bool plist_is_dup_nb(const struct lyd_node *dnode) pda.pda_action = yang_dnode_get_string(entry_dnode, "action"); 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++; - } + plist_dnode_to_prefix(entry_dnode, &pda.any, &pda.prefix, &pda.ge, + &pda.le); return plist_is_dup(entry_dnode, &pda); }