ripd: retrofit the 'passive-interface' command to the new northbound model

In ripd, the "passive-interface default" command has the following
behavior:
* All interfaces are converted to the passive mode;
* The "passive-interface IFNAME" command becomes a no-operation and
  "passive-interface IFNAME" statements are removed from the running
  configuration.
* The "no passive-interface IFNAME" can be used to remove interfaces
  from the passive mode.

This command was modeled using the following YANG data nodes in the
frr-ripd module:

  leaf passive-default {
    type boolean;
    default "false";
    description
      "Control whether interfaces are in the passive mode
       by default or not.";
  }
  leaf-list passive-interface {
    when "../passive-default = 'false'";
    type string {
      length "1..16";
    }
    description
      "A list of interfaces where the sending of RIP packets
       is disabled.";
  }
  leaf-list non-passive-interface {
    when "../passive-default = 'true'";
    type string {
      length "1..16";
    }
    description
      "A list of interfaces where the sending of RIP packets
       is enabled.";
  }

The 'when' statements guarantee that the list of passive interfaces
is cleared when the "passive-interface default" command is entered
(likewise, they guarantee that the list of non-passive interfaces is
cleared when the "passive-interface default" command is removed). This
matches exactly the behavior we want to model.

Finally, move the 'passive_default' global variable into the
'rip' structure where it belongs. This fixed the bug where the
"passive-interface default" command was being retained after a "no router
rip" + "router rip".

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

6 files changed, 156 insertions, 79 deletions

diff --git a/ripd/rip_cli.c b/ripd/rip_cli.c
index 10db5d967..ae7e05cc1 100644
--- a/ripd/rip_cli.c
+++ b/ripd/rip_cli.c
@@ -465,6 +465,77 @@ void cli_show_rip_offset_list(struct vty *vty, struct lyd_node *dnode,
 	vty_out(vty, "\n");
 }
 
+/*
+ * XPath: /frr-ripd:ripd/instance/passive-default
+ */
+DEFPY (rip_passive_default,
+       rip_passive_default_cmd,
+       "[no] passive-interface default",
+       NO_STR
+       "Suppress routing updates on an interface\n"
+       "default for all interfaces\n")
+{
+	struct cli_config_change changes[] = {
+		{
+			.xpath = "./passive-default",
+			.operation = NB_OP_MODIFY,
+			.value = no ? "false" : "true",
+		},
+	};
+
+	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+}
+
+void cli_show_rip_passive_default(struct vty *vty, struct lyd_node *dnode,
+				  bool show_defaults)
+{
+	if (!yang_dnode_get_bool(dnode, NULL))
+		vty_out(vty, " no");
+
+	vty_out(vty, " passive-interface default\n");
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/passive-interface
+ *        /frr-ripd:ripd/instance/non-passive-interface
+ */
+DEFPY (rip_passive_interface,
+       rip_passive_interface_cmd,
+       "[no] passive-interface IFNAME",
+       NO_STR
+       "Suppress routing updates on an interface\n"
+       "Interface name\n")
+{
+	struct cli_config_change changes[] = {
+		{
+			.xpath = "./passive-interface",
+			.operation = no ? NB_OP_DELETE : NB_OP_CREATE,
+			.value = ifname,
+		},
+		{
+			.xpath = "./non-passive-interface",
+			.operation = no ? NB_OP_CREATE : NB_OP_DELETE,
+			.value = ifname,
+		},
+	};
+
+	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+}
+
+void cli_show_rip_passive_interface(struct vty *vty, struct lyd_node *dnode,
+				    bool show_defaults)
+{
+	vty_out(vty, " passive-interface %s\n",
+		yang_dnode_get_string(dnode, NULL));
+}
+
+void cli_show_rip_non_passive_interface(struct vty *vty, struct lyd_node *dnode,
+					bool show_defaults)
+{
+	vty_out(vty, " no passive-interface %s\n",
+		yang_dnode_get_string(dnode, NULL));
+}
+
 void rip_cli_init(void)
 {
 	install_element(CONFIG_NODE, &router_rip_cmd);
@@ -483,4 +554,6 @@ void rip_cli_init(void)
 	install_element(RIP_NODE, &rip_network_if_cmd);
 	install_element(RIP_NODE, &rip_offset_list_cmd);
 	install_element(RIP_NODE, &no_rip_offset_list_cmd);
+	install_element(RIP_NODE, &rip_passive_default_cmd);
+	install_element(RIP_NODE, &rip_passive_interface_cmd);
 }
diff --git a/ripd/rip_cli.h b/ripd/rip_cli.h
index 16ed78451..61d379f78 100644
--- a/ripd/rip_cli.h
+++ b/ripd/rip_cli.h
@@ -44,5 +44,14 @@ extern void cli_show_rip_network_interface(struct vty *vty,
 					   bool show_defaults);
 extern void cli_show_rip_offset_list(struct vty *vty, struct lyd_node *dnode,
 				     bool show_defaults);
+extern void cli_show_rip_passive_default(struct vty *vty,
+					 struct lyd_node *dnode,
+					 bool show_defaults);
+extern void cli_show_rip_passive_interface(struct vty *vty,
+					   struct lyd_node *dnode,
+					   bool show_defaults);
+extern void cli_show_rip_non_passive_interface(struct vty *vty,
+					       struct lyd_node *dnode,
+					       bool show_defaults);
 
 #endif /* _FRR_RIP_CLI_H_ */
diff --git a/ripd/rip_interface.c b/ripd/rip_interface.c
index 1e79e6d27..a377b2540 100644
--- a/ripd/rip_interface.c
+++ b/ripd/rip_interface.c
@@ -67,7 +67,6 @@ vector rip_enable_interface;
 struct route_table *rip_enable_network;
 
 /* Vector to store passive-interface name. */
-static int passive_default; /* are we in passive-interface default mode? */
 vector Vrip_passive_nondefault;
 
 /* Join to the RIP version 2 multicast group. */
@@ -1087,11 +1086,14 @@ void rip_passive_interface_apply(struct interface *ifp)
 {
 	struct rip_interface *ri;
 
+	if (rip == NULL)
+		return;
+
 	ri = ifp->info;
 
 	ri->passive = ((rip_passive_nondefault_lookup(ifp->name) < 0)
-			       ? passive_default
-			       : !passive_default);
+			       ? rip->passive_default
+			       : !rip->passive_default);
 
 	if (IS_RIP_DEBUG_ZEBRA)
 		zlog_debug("interface %s: passive = %d", ifp->name,
@@ -1108,27 +1110,35 @@ static void rip_passive_interface_apply_all(void)
 }
 
 /* Passive interface. */
-static int rip_passive_nondefault_set(struct vty *vty, const char *ifname)
+int rip_passive_nondefault_set(const char *ifname)
 {
 	if (rip_passive_nondefault_lookup(ifname) >= 0)
-		return CMD_WARNING_CONFIG_FAILED;
+		/*
+		 * Don't return an error, this can happen after changing
+		 * 'passive-default'.
+		 */
+		return NB_OK;
 
 	vector_set(Vrip_passive_nondefault,
 		   XSTRDUP(MTYPE_RIP_INTERFACE_STRING, ifname));
 
 	rip_passive_interface_apply_all();
 
-	return CMD_SUCCESS;
+	return NB_OK;
 }
 
-static int rip_passive_nondefault_unset(struct vty *vty, const char *ifname)
+int rip_passive_nondefault_unset(const char *ifname)
 {
 	int i;
 	char *str;
 
 	i = rip_passive_nondefault_lookup(ifname);
 	if (i < 0)
-		return CMD_WARNING_CONFIG_FAILED;
+		/*
+		 * Don't return an error, this can happen after changing
+		 * 'passive-default'.
+		 */
+		return NB_OK;
 
 	str = vector_slot(Vrip_passive_nondefault, i);
 	XFREE(MTYPE_RIP_INTERFACE_STRING, str);
@@ -1136,7 +1146,7 @@ static int rip_passive_nondefault_unset(struct vty *vty, const char *ifname)
 
 	rip_passive_interface_apply_all();
 
-	return CMD_SUCCESS;
+	return NB_OK;
 }
 
 /* Free all configured RIP passive-interface settings. */
@@ -1591,43 +1601,6 @@ DEFUN (no_ip_rip_split_horizon_poisoned_reverse,
 	return CMD_SUCCESS;
 }
 
-DEFUN (rip_passive_interface,
-       rip_passive_interface_cmd,
-       "passive-interface <IFNAME|default>",
-       "Suppress routing updates on an interface\n"
-       "Interface name\n"
-       "default for all interfaces\n")
-{
-	if (argv[1]->type == WORD_TKN) { // user passed 'default'
-		passive_default = 1;
-		rip_passive_nondefault_clean();
-		return CMD_SUCCESS;
-	}
-	if (passive_default)
-		return rip_passive_nondefault_unset(vty, argv[1]->arg);
-	else
-		return rip_passive_nondefault_set(vty, argv[1]->arg);
-}
-
-DEFUN (no_rip_passive_interface,
-       no_rip_passive_interface_cmd,
-       "no passive-interface <IFNAME|default>",
-       NO_STR
-       "Suppress routing updates on an interface\n"
-       "Interface name\n"
-       "default for all interfaces\n")
-{
-	if (argv[2]->type == WORD_TKN) {
-		passive_default = 0;
-		rip_passive_nondefault_clean();
-		return CMD_SUCCESS;
-	}
-	if (passive_default)
-		return rip_passive_nondefault_set(vty, argv[2]->arg);
-	else
-		return rip_passive_nondefault_unset(vty, argv[2]->arg);
-}
-
 /* Write rip configuration of each interface. */
 static int rip_interface_config_write(struct vty *vty)
 {
@@ -1712,7 +1685,7 @@ static int rip_interface_config_write(struct vty *vty)
 	return 0;
 }
 
-int config_write_rip_network(struct vty *vty, int config_mode)
+int rip_show_network_config(struct vty *vty)
 {
 	unsigned int i;
 	char *ifname;
@@ -1722,34 +1695,19 @@ int config_write_rip_network(struct vty *vty, int config_mode)
 	for (node = route_top(rip_enable_network); node;
 	     node = route_next(node))
 		if (node->info)
-			vty_out(vty, "%s%s/%d\n",
-				config_mode ? " network " : "    ",
+			vty_out(vty, "    %s/%u\n",
 				inet_ntoa(node->p.u.prefix4),
 				node->p.prefixlen);
 
 	/* Interface name RIP enable statement. */
 	for (i = 0; i < vector_active(rip_enable_interface); i++)
 		if ((ifname = vector_slot(rip_enable_interface, i)) != NULL)
-			vty_out(vty, "%s%s\n",
-				config_mode ? " network " : "    ", ifname);
+			vty_out(vty, "    %s\n", ifname);
 
 	/* RIP neighbors listing. */
 	for (node = route_top(rip->neighbor); node; node = route_next(node))
 		if (node->info)
-			vty_out(vty, "%s%s\n",
-				config_mode ? " neighbor " : "    ",
-				inet_ntoa(node->p.u.prefix4));
-
-	/* RIP passive interface listing. */
-	if (config_mode) {
-		if (passive_default)
-			vty_out(vty, " passive-interface default\n");
-		for (i = 0; i < vector_active(Vrip_passive_nondefault); i++)
-			if ((ifname = vector_slot(Vrip_passive_nondefault, i))
-			    != NULL)
-				vty_out(vty, " %spassive-interface %s\n",
-					(passive_default ? "no " : ""), ifname);
-	}
+			vty_out(vty, "    %s\n", inet_ntoa(node->p.u.prefix4));
 
 	return 0;
 }
@@ -1792,9 +1750,6 @@ void rip_if_init(void)
 	if_cmd_init();
 
 	/* Install commands. */
-	install_element(RIP_NODE, &rip_passive_interface_cmd);
-	install_element(RIP_NODE, &no_rip_passive_interface_cmd);
-
 	install_element(INTERFACE_NODE, &ip_rip_send_version_cmd);
 	install_element(INTERFACE_NODE, &ip_rip_send_version_1_cmd);
 	install_element(INTERFACE_NODE, &no_ip_rip_send_version_cmd);
diff --git a/ripd/rip_northbound.c b/ripd/rip_northbound.c
index bdad4004d..08dcac62c 100644
--- a/ripd/rip_northbound.c
+++ b/ripd/rip_northbound.c
@@ -463,7 +463,12 @@ static int ripd_instance_passive_default_modify(enum nb_event event,
 						const struct lyd_node *dnode,
 						union nb_resource *resource)
 {
-	/* TODO: implement me. */
+	if (event != NB_EV_APPLY)
+		return NB_OK;
+
+	rip->passive_default = yang_dnode_get_bool(dnode, NULL);
+	rip_passive_nondefault_clean();
+
 	return NB_OK;
 }
 
@@ -474,15 +479,27 @@ static int ripd_instance_passive_interface_create(enum nb_event event,
 						  const struct lyd_node *dnode,
 						  union nb_resource *resource)
 {
-	/* TODO: implement me. */
-	return NB_OK;
+	const char *ifname;
+
+	if (event != NB_EV_APPLY)
+		return NB_OK;
+
+	ifname = yang_dnode_get_string(dnode, NULL);
+
+	return rip_passive_nondefault_set(ifname);
 }
 
 static int ripd_instance_passive_interface_delete(enum nb_event event,
 						  const struct lyd_node *dnode)
 {
-	/* TODO: implement me. */
-	return NB_OK;
+	const char *ifname;
+
+	if (event != NB_EV_APPLY)
+		return NB_OK;
+
+	ifname = yang_dnode_get_string(dnode, NULL);
+
+	return rip_passive_nondefault_unset(ifname);
 }
 
 /*
@@ -493,16 +510,28 @@ ripd_instance_non_passive_interface_create(enum nb_event event,
 					   const struct lyd_node *dnode,
 					   union nb_resource *resource)
 {
-	/* TODO: implement me. */
-	return NB_OK;
+	const char *ifname;
+
+	if (event != NB_EV_APPLY)
+		return NB_OK;
+
+	ifname = yang_dnode_get_string(dnode, NULL);
+
+	return rip_passive_nondefault_unset(ifname);
 }
 
 static int
 ripd_instance_non_passive_interface_delete(enum nb_event event,
 					   const struct lyd_node *dnode)
 {
-	/* TODO: implement me. */
-	return NB_OK;
+	const char *ifname;
+
+	if (event != NB_EV_APPLY)
+		return NB_OK;
+
+	ifname = yang_dnode_get_string(dnode, NULL);
+
+	return rip_passive_nondefault_set(ifname);
 }
 
 /*
@@ -981,16 +1010,19 @@ const struct frr_yang_module_info frr_ripd_info = {
 		{
 			.xpath = "/frr-ripd:ripd/instance/passive-default",
 			.cbs.modify = ripd_instance_passive_default_modify,
+			.cbs.cli_show = cli_show_rip_passive_default,
 		},
 		{
 			.xpath = "/frr-ripd:ripd/instance/passive-interface",
 			.cbs.create = ripd_instance_passive_interface_create,
 			.cbs.delete = ripd_instance_passive_interface_delete,
+			.cbs.cli_show = cli_show_rip_passive_interface,
 		},
 		{
 			.xpath = "/frr-ripd:ripd/instance/non-passive-interface",
 			.cbs.create = ripd_instance_non_passive_interface_create,
 			.cbs.delete = ripd_instance_non_passive_interface_delete,
+			.cbs.cli_show = cli_show_rip_non_passive_interface,
 		},
 		{
 			.xpath = "/frr-ripd:ripd/instance/redistribute",
diff --git a/ripd/ripd.c b/ripd/ripd.c
index 24bbf226b..9892c5c41 100644
--- a/ripd/ripd.c
+++ b/ripd/ripd.c
@@ -2674,6 +2674,8 @@ int rip_create(int socket)
 		yang_get_default_uint8("%s/default-metric", RIP_INSTANCE);
 	rip->distance =
 		yang_get_default_uint8("%s/distance/default", RIP_INSTANCE);
+	rip->passive_default =
+		yang_get_default_bool("%s/passive-default", RIP_INSTANCE);
 	rip->garbage_time = yang_get_default_uint32("%s/timers/flush-interval",
 						    RIP_INSTANCE);
 	rip->timeout_time = yang_get_default_uint32(
@@ -3353,7 +3355,7 @@ DEFUN (show_ip_rip_status,
 	}
 
 	vty_out(vty, "  Routing for Networks:\n");
-	config_write_rip_network(vty, 0);
+	rip_show_network_config(vty);
 
 	{
 		int found_passive = 0;
diff --git a/ripd/ripd.h b/ripd/ripd.h
index 4378f75af..307f7934b 100644
--- a/ripd/ripd.h
+++ b/ripd/ripd.h
@@ -149,6 +149,9 @@ struct rip {
 	/* RIP ECMP flag */
 	bool ecmp;
 
+	/* Are we in passive-interface default mode? */
+	bool passive_default;
+
 	/* For redistribute route map. */
 	struct {
 		char *name;
@@ -388,6 +391,8 @@ extern void rip_clean(void);
 extern void rip_clean_network(void);
 extern void rip_interfaces_clean(void);
 extern void rip_interfaces_reset(void);
+extern int rip_passive_nondefault_set(const char *ifname);
+extern int rip_passive_nondefault_unset(const char *ifname);
 extern void rip_passive_nondefault_clean(void);
 extern void rip_if_init(void);
 extern void rip_if_down_all(void);
@@ -426,7 +431,7 @@ extern void rip_interface_multicast_set(int, struct connected *);
 extern void rip_distribute_update_interface(struct interface *);
 extern void rip_if_rmap_update_interface(struct interface *);
 
-extern int config_write_rip_network(struct vty *, int);
+extern int rip_show_network_config(struct vty *);
 extern int config_write_rip_redistribute(struct vty *, int);
 
 extern void rip_peer_init(void);
@@ -473,6 +478,7 @@ DECLARE_HOOK(rip_ifaddr_add, (struct connected * ifc), (ifc))
 DECLARE_HOOK(rip_ifaddr_del, (struct connected * ifc), (ifc))
 
 extern struct route_table *rip_distance_table;
+extern vector Vrip_passive_nondefault;
 
 /* Northbound. */
 extern void rip_cli_init(void);