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 <iryzhov@nfware.com>

diff --git a/lib/filter.h b/lib/filter.h
index 941fabd38b8a7d77b311ac17032804a3bed0c6e0..d1956ec019fe8daff17f0b1acd824e7419ebef5e 100644 (file)
--- 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 f030ce1b335a6433a0b2922c59e0c9e0f4ef0a9f..45c7544a3b4031bfc0df5b4c9c695d5bf139a1fa 100644 (file)
--- 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 85805ffa47c190436e9e39b633e2d9a5e6baa3b6..80ea7a57cb22faef874736f867513bc157692a4d 100644 (file)
--- 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);
 }