1 files changed, 145 insertions, 167 deletions

diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c
index dfc8bec1bc..a52c2d1e30 100644
--- a/pbrd/pbr_vty.c
+++ b/pbrd/pbr_vty.c
@@ -142,18 +142,14 @@ DEFPY(pbr_map_match_src, pbr_map_match_src_cmd,
 		if (pbrms->src) {
 			if (prefix_same(pbrms->src, prefix))
 				return CMD_SUCCESS;
+		} else
+			pbrms->src = prefix_new();
 
-			vty_out(vty,
-				"A `match src-ip XX` command already exists, please remove that first\n");
-			return CMD_WARNING_CONFIG_FAILED;
-		}
-
-		pbrms->src = prefix_new();
 		prefix_copy(pbrms->src, prefix);
 	} else
 		prefix_free(&pbrms->src);
 
-	pbr_map_check(pbrms);
+	pbr_map_check(pbrms, true);
 
 	return CMD_SUCCESS;
 }
@@ -174,18 +170,14 @@ DEFPY(pbr_map_match_dst, pbr_map_match_dst_cmd,
 		if (pbrms->dst) {
 			if (prefix_same(pbrms->dst, prefix))
 				return CMD_SUCCESS;
+		} else
+			pbrms->dst = prefix_new();
 
-			vty_out(vty,
-				"A `match dst-ip XX` command already exists, please remove that first\n");
-			return CMD_WARNING_CONFIG_FAILED;
-		}
-
-		pbrms->dst = prefix_new();
 		prefix_copy(pbrms->dst, prefix);
 	} else
 		prefix_free(&pbrms->dst);
 
-	pbr_map_check(pbrms);
+	pbr_map_check(pbrms, true);
 
 	return CMD_SUCCESS;
 }
@@ -205,30 +197,52 @@ DEFPY(pbr_map_match_mark, pbr_map_match_mark_cmd,
 #endif
 
 	if (!no) {
-		if (pbrms->mark) {
+		if (pbrms->mark)
 			if (pbrms->mark == (uint32_t)mark)
 				return CMD_SUCCESS;
 
-			vty_out(vty,
-				"A `match mark XX` command already exists, please remove that first\n");
-			return CMD_WARNING_CONFIG_FAILED;
-		}
-
 		pbrms->mark = (uint32_t)mark;
 	} else
 		pbrms->mark = 0;
 
-	pbr_map_check(pbrms);
+	pbr_map_check(pbrms, true);
 
 	return CMD_SUCCESS;
 }
 
-#define SET_VRF_EXISTS_STR                                                     \
-	"A `set vrf XX` command already exists, please remove that first\n"
+static void pbrms_clear_set_vrf_config(struct pbr_map_sequence *pbrms)
+{
+	if (pbrms->vrf_lookup || pbrms->vrf_unchanged) {
+		pbr_map_delete_vrf(pbrms);
+		pbrms->vrf_name[0] = '\0';
+		pbrms->vrf_lookup = false;
+		pbrms->vrf_unchanged = false;
+	}
+}
+
+static void pbrms_clear_set_nhg_config(struct pbr_map_sequence *pbrms)
+{
+	if (pbrms->nhgrp_name)
+		pbr_map_delete_nexthops(pbrms);
+}
+
+static void pbrms_clear_set_nexthop_config(struct pbr_map_sequence *pbrms)
+{
+	if (pbrms->nhg)
+		pbr_nht_delete_individual_nexthop(pbrms);
+}
+
+static void pbrms_clear_set_config(struct pbr_map_sequence *pbrms)
+{
+	pbrms_clear_set_vrf_config(pbrms);
+	pbrms_clear_set_nhg_config(pbrms);
+	pbrms_clear_set_nexthop_config(pbrms);
+
+	pbrms->nhs_installed = false;
+}
 
 DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd,
-      "[no] set nexthop-group NHGNAME$name",
-      NO_STR
+      "set nexthop-group NHGNAME$name",
       "Set for the PBR-MAP\n"
       "nexthop-group to use\n"
       "The name of the nexthop-group\n")
@@ -236,17 +250,6 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd,
 	struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence);
 	struct nexthop_group_cmd *nhgc;
 
-	if (pbrms->nhg) {
-		vty_out(vty,
-			"A `set nexthop XX` command already exists, please remove that first\n");
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
-	if (pbrms->vrf_lookup || pbrms->vrf_unchanged) {
-		vty_out(vty, SET_VRF_EXISTS_STR);
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
 	nhgc = nhgc_find(name);
 	if (!nhgc) {
 		vty_out(vty, "Specified nexthop-group %s does not exist\n",
@@ -255,40 +258,39 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd,
 			"PBR-MAP will not be applied until it is created\n");
 	}
 
-	if (no) {
-		if (pbrms->nhgrp_name && strcmp(name, pbrms->nhgrp_name) == 0)
-			pbr_map_delete_nexthops(pbrms);
-		else {
-			vty_out(vty,
-				"Nexthop Group specified: %s does not exist to remove\n",
-				name);
-			return CMD_WARNING_CONFIG_FAILED;
-		}
-	} else {
-		if (pbrms->nhgrp_name) {
-			if (strcmp(name, pbrms->nhgrp_name) != 0) {
-				vty_out(vty,
-					"Please delete current nexthop group before modifying current one\n");
-				return CMD_WARNING_CONFIG_FAILED;
-			}
+	if (pbrms->nhgrp_name && strcmp(name, pbrms->nhgrp_name) == 0)
+		return CMD_SUCCESS;
 
-			return CMD_SUCCESS;
-		}
-		pbrms->nhgrp_name = XSTRDUP(MTYPE_TMP, name);
-		pbr_map_check(pbrms);
-	}
+	/* This is new/replacement config */
+	pbrms_clear_set_config(pbrms);
+
+	pbrms->nhgrp_name = XSTRDUP(MTYPE_TMP, name);
+	pbr_map_check(pbrms, true);
+
+	return CMD_SUCCESS;
+}
+
+DEFPY(no_pbr_map_nexthop_group, no_pbr_map_nexthop_group_cmd,
+      "no set nexthop-group [NHGNAME$name]",
+      NO_STR
+      "Set for the PBR-MAP\n"
+      "nexthop-group to use\n"
+      "The name of the nexthop-group\n")
+{
+	struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence);
+
+	pbrms_clear_set_config(pbrms);
 
 	return CMD_SUCCESS;
 }
 
 DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
-      "[no] set nexthop\
+      "set nexthop\
         <\
 	  <A.B.C.D|X:X::X:X>$addr [INTERFACE$intf]\
 	  |INTERFACE$intf\
 	>\
         [nexthop-vrf NAME$vrf_name]",
-      NO_STR
       "Set for the PBR-MAP\n"
       "Specify one of the nexthops in this map\n"
       "v4 Address\n"
@@ -301,18 +303,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
 	struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence);
 	struct vrf *vrf;
 	struct nexthop nhop;
-	struct nexthop *nh;
-
-	if (pbrms->nhgrp_name) {
-		vty_out(vty,
-			"Please unconfigure the nexthop group before adding an individual nexthop\n");
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
-	if (pbrms->vrf_lookup || pbrms->vrf_unchanged) {
-		vty_out(vty, SET_VRF_EXISTS_STR);
-		return CMD_WARNING_CONFIG_FAILED;
-	}
+	struct nexthop *nh = NULL;
 
 	if (vrf_name)
 		vrf = vrf_lookup_by_name(vrf_name);
@@ -362,45 +353,18 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
 
 	if (pbrms->nhg)
 		nh = nexthop_exists(pbrms->nhg, &nhop);
-	else {
-		char buf[PBR_NHC_NAMELEN];
-
-		if (no) {
-			vty_out(vty, "No nexthops to delete\n");
-			return CMD_WARNING_CONFIG_FAILED;
-		}
-
-		pbrms->nhg = nexthop_group_new();
-		pbrms->internal_nhg_name =
-			XSTRDUP(MTYPE_TMP,
-				pbr_nht_nexthop_make_name(pbrms->parent->name,
-							  PBR_NHC_NAMELEN,
-							  pbrms->seqno,
-							  buf));
-		nh = NULL;
-	}
-
-	if (no) {
-		if (nh)
-			pbr_nht_delete_individual_nexthop(pbrms);
-	} else if (!nh) {
 
-		if (pbrms->nhg->nexthop) {
-			vty_out(vty,
-				"If you would like more than one nexthop please use nexthop-groups\n");
-			return CMD_WARNING_CONFIG_FAILED;
-		}
+	if (nh) /* Same config re-entered */
+		goto done;
 
-		/* must be adding new nexthop since !no and !nexthop_exists */
-		nh = nexthop_new();
+	/* This is new/replacement config */
+	pbrms_clear_set_config(pbrms);
 
-		memcpy(nh, &nhop, sizeof(nhop));
-		_nexthop_add(&pbrms->nhg->nexthop, nh);
+	pbr_nht_add_individual_nexthop(pbrms, &nhop);
 
-		pbr_nht_add_individual_nexthop(pbrms);
-		pbr_map_check(pbrms);
-	}
+	pbr_map_check(pbrms, true);
 
+done:
 	if (nhop.type == NEXTHOP_TYPE_IFINDEX
 	    || (nhop.type == NEXTHOP_TYPE_IPV6_IFINDEX
 		&& IN6_IS_ADDR_LINKLOCAL(&nhop.gate.ipv6))) {
@@ -414,84 +378,80 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
 	return CMD_SUCCESS;
 }
 
-DEFPY(pbr_map_vrf, pbr_map_vrf_cmd,
-      "[no] set vrf <NAME$vrf_name|unchanged>",
+DEFPY(no_pbr_map_nexthop, no_pbr_map_nexthop_cmd,
+      "no set nexthop\
+        [<\
+	  <A.B.C.D|X:X::X:X>$addr [INTERFACE$intf]\
+	  |INTERFACE$intf\
+	>\
+        [nexthop-vrf NAME$vrf_name]]",
       NO_STR
       "Set for the PBR-MAP\n"
-      "Specify the VRF for this map\n"
-      "The VRF Name\n"
-      "Use the interface's VRF for lookup\n")
+      "Specify one of the nexthops in this map\n"
+      "v4 Address\n"
+      "v6 Address\n"
+      "Interface to use\n"
+      "Interface to use\n"
+      "If the nexthop is in a different vrf tell us\n"
+      "The nexthop-vrf Name\n")
 {
 	struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence);
 
-	if (no) {
-		pbr_map_delete_vrf(pbrms);
+	pbrms_clear_set_config(pbrms);
 
-		/* Reset all data */
-		pbrms->nhs_installed = false;
-		pbrms->vrf_name[0] = '\0';
-		pbrms->vrf_lookup = false;
-		pbrms->vrf_unchanged = false;
-
-		return CMD_SUCCESS;
-	}
+	return CMD_SUCCESS;
+}
 
-	if (pbrms->nhgrp_name || pbrms->nhg) {
-		vty_out(vty,
-			"A `set nexthop/nexthop-group XX` command already exits, please remove that first\n");
-		return CMD_WARNING_CONFIG_FAILED;
-	}
+DEFPY(pbr_map_vrf, pbr_map_vrf_cmd,
+      "set vrf <NAME$vrf_name|unchanged>",
+      "Set for the PBR-MAP\n"
+      "Specify the VRF for this map\n"
+      "The VRF Name\n"
+      "Use the interface's VRF for lookup\n")
+{
+	struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence);
 
 	/*
-	 * 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 an equivalent set vrf * exists, just return success.
 	 */
-	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;
-
+	if (vrf_name && pbrms->vrf_lookup
+	    && strncmp(pbrms->vrf_name, vrf_name, sizeof(pbrms->vrf_name)) == 0)
 		return CMD_SUCCESS;
-
-	} else if (!vrf_name && pbrms->vrf_unchanged) {
-		/* Unchanged specified and unchanged already exists */
+	else if (!vrf_name && pbrms->vrf_unchanged) /* Unchanged already set */
 		return CMD_SUCCESS;
 
-	} 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;
+	if (vrf_name && !pbr_vrf_lookup_by_name(vrf_name)) {
+		vty_out(vty, "Specified: %s is non-existent\n", vrf_name);
+		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	/* 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",
-				vrf_name);
-			return CMD_WARNING_CONFIG_FAILED;
-		}
+	/* This is new/replacement config */
+	pbrms_clear_set_config(pbrms);
 
+	if (vrf_name) {
 		pbrms->vrf_lookup = true;
 		strlcpy(pbrms->vrf_name, vrf_name, sizeof(pbrms->vrf_name));
 	} else
 		pbrms->vrf_unchanged = true;
 
-	pbr_map_check(pbrms);
+	pbr_map_check(pbrms, true);
 
 	return CMD_SUCCESS;
+}
 
-vrf_exists:
-	vty_out(vty, SET_VRF_EXISTS_STR);
-	return CMD_WARNING_CONFIG_FAILED;
+DEFPY(no_pbr_map_vrf, no_pbr_map_vrf_cmd,
+      "no set vrf [<NAME$vrf_name|unchanged>]",
+      NO_STR
+      "Set for the PBR-MAP\n"
+      "Specify the VRF for this map\n"
+      "The VRF Name\n"
+      "Use the interface's VRF for lookup\n")
+{
+	struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence);
+
+	pbrms_clear_set_config(pbrms);
+
+	return CMD_SUCCESS;
 }
 
 DEFPY (pbr_policy,
@@ -717,7 +677,12 @@ DEFPY (show_pbr_interface,
 
 /* PBR debugging CLI ------------------------------------------------------- */
 
-static struct cmd_node debug_node = {DEBUG_NODE, "", 1};
+static struct cmd_node debug_node = {
+	.name = "debug",
+	.node = DEBUG_NODE,
+	.prompt = "",
+	.config_write = pbr_debug_config_write,
+};
 
 DEFPY(debug_pbr,
       debug_pbr_cmd,
@@ -765,8 +730,13 @@ DEFUN_NOSH(show_debugging_pbr,
 /* ------------------------------------------------------------------------- */
 
 
+static int pbr_interface_config_write(struct vty *vty);
 static struct cmd_node interface_node = {
-	INTERFACE_NODE, "%s(config-if)# ", 1 /* vtysh ? yes */
+	.name = "interface",
+	.node = INTERFACE_NODE,
+	.parent_node = CONFIG_NODE,
+	.prompt = "%s(config-if)# ",
+	.config_write = pbr_interface_config_write,
 };
 
 static int pbr_interface_config_write(struct vty *vty)
@@ -794,8 +764,15 @@ static int pbr_interface_config_write(struct vty *vty)
 	return 1;
 }
 
+static int pbr_vty_map_config_write(struct vty *vty);
 /* PBR map node structure. */
-static struct cmd_node pbr_map_node = {PBRMAP_NODE, "%s(config-pbr-map)# ", 1};
+static struct cmd_node pbr_map_node = {
+	.name = "pbr-map",
+	.node = PBRMAP_NODE,
+	.parent_node = CONFIG_NODE,
+	.prompt = "%s(config-pbr-map)# ",
+	.config_write = pbr_vty_map_config_write,
+};
 
 static int pbr_vty_map_config_write_sequence(struct vty *vty,
 					     struct pbr_map *pbrm,
@@ -873,15 +850,13 @@ void pbr_vty_init(void)
 {
 	cmd_variable_handler_register(pbr_map_name);
 
-	install_node(&interface_node,
-		     pbr_interface_config_write);
+	install_node(&interface_node);
 	if_cmd_init();
 
-	install_node(&pbr_map_node,
-		     pbr_vty_map_config_write);
+	install_node(&pbr_map_node);
 
 	/* debug */
-	install_node(&debug_node, pbr_debug_config_write);
+	install_node(&debug_node);
 	install_element(VIEW_NODE, &debug_pbr_cmd);
 	install_element(CONFIG_NODE, &debug_pbr_cmd);
 	install_element(VIEW_NODE, &show_debugging_pbr_cmd);
@@ -897,8 +872,11 @@ void pbr_vty_init(void)
 	install_element(PBRMAP_NODE, &pbr_map_match_dst_cmd);
 	install_element(PBRMAP_NODE, &pbr_map_match_mark_cmd);
 	install_element(PBRMAP_NODE, &pbr_map_nexthop_group_cmd);
+	install_element(PBRMAP_NODE, &no_pbr_map_nexthop_group_cmd);
 	install_element(PBRMAP_NODE, &pbr_map_nexthop_cmd);
+	install_element(PBRMAP_NODE, &no_pbr_map_nexthop_cmd);
 	install_element(PBRMAP_NODE, &pbr_map_vrf_cmd);
+	install_element(PBRMAP_NODE, &no_pbr_map_vrf_cmd);
 	install_element(VIEW_NODE, &show_pbr_cmd);
 	install_element(VIEW_NODE, &show_pbr_map_cmd);
 	install_element(VIEW_NODE, &show_pbr_interface_cmd);