Blob Blame History Raw
From 667dcc277c15c0bddc785f9b949d658f8d815818 Mon Sep 17 00:00:00 2001
From: Igor Ryzhov <iryzhov@nfware.com>
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 <iryzhov@nfware.com>
---
 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);
 }