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);

 }