From: Stephen Worley <sworley@cumulusnetworks.com>
Date: Thu, 19 Mar 2020 16:32:13 +0000 (-0400)
Subject: pbrd: properly handle duplicate set vrf XX configs
X-Git-Tag: base_7.4~177^2~1
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=9bf1b0f74a27cd6c697c7daf4fb31e5b55c48ef7;p=mirror%2Ffrr.git

pbrd: properly handle duplicate set vrf XX configs

Properly handle the case where we are sent the same `set vrf`
configs for a pbr map repeatedly. If we are sent the same
config, we return successfully without doing anyting.

If the config is different and its not a [no], then return failure
as we did before since we don't support atomic replace yet.

Before, we would fail anytime even if the config sent was the same
as is already there. This would cause frr-reload to mark as a
failure when it tried to re-apply the same config.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
---

diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c
index e395b7831d..03fbec3b1c 100644
--- a/pbrd/pbr_vty.c
+++ b/pbrd/pbr_vty.c
@@ -444,12 +444,37 @@ DEFPY(pbr_map_vrf, pbr_map_vrf_cmd,
 		goto done;
 	}
 
-	if (pbrms->vrf_lookup || pbrms->vrf_unchanged) {
-		vty_out(vty, SET_VRF_EXISTS_STR);
-		ret = CMD_WARNING_CONFIG_FAILED;
+	/*
+	 * Determine if a set vrf * command already exists.
+	 *
+	 * If its equivalent, just return success.
+	 *
+	 * Else, return failure, we don't allow atomic swaps yet.
+	 */
+	if (vrf_name && pbrms->vrf_lookup) {
+		/* New vrf specified and one already exists */
+
+		/* Is this vrf different from one already configured? */
+		if (strncmp(pbrms->vrf_name, vrf_name, sizeof(pbrms->vrf_name))
+		    != 0)
+			goto vrf_exists;
+
+		goto done;
+
+	} else if (!vrf_name && pbrms->vrf_unchanged) {
+		/* Unchanged specified and unchanged already exists */
 		goto done;
+
+	} else if (vrf_name && pbrms->vrf_unchanged) {
+		/* New vrf specified and unchanged is already set */
+		goto vrf_exists;
+
+	} else if (!vrf_name && pbrms->vrf_lookup) {
+		/* Unchanged specified and vrf to lookup already exists */
+		goto vrf_exists;
 	}
 
+	/* Create new lookup VRF or Unchanged */
 	if (vrf_name) {
 		if (!pbr_vrf_lookup_by_name(vrf_name)) {
 			vty_out(vty, "Specified: %s is non-existent\n",
@@ -467,6 +492,11 @@ DEFPY(pbr_map_vrf, pbr_map_vrf_cmd,
 
 done:
 	return ret;
+
+vrf_exists:
+	vty_out(vty, SET_VRF_EXISTS_STR);
+	ret = CMD_WARNING_CONFIG_FAILED;
+	return ret;
 }
 
 DEFPY (pbr_policy,