lib, ripd: rework API for converted CLI commands

When editing the candidate configuration, the northbound must ensure
that either all changes made by a command are accepted or none are.
This is done to prevent inconsistent states where only parts of a
command are applied in the event any error happens.

The previous API for converted commands, the nb_cli_cfg_change()
function, required callers to pass an array containing all changes
that needed to be applied in the candidate configuration. The
problem with this API is that it was very inconvenient for complex
commands, which change different configuration options depending
on several factors.  This required users to manipulate the array
of configuration changes using low-level primitives, making it
complicated to implement some commands.

To solve this problem, introduce a new API based on the two following
functions:
- nb_cli_enqueue_change()
- nb_cli_apply_changes()

The first function is used to enqueue configuration changes, one
at time. Then the nb_cli_apply_changes() function is used to apply
all the enqueued configuration changes.

To implement this, a static-sized array was allocated in the "vty"
structure, along with a counter of enqueued changes. This eliminates
the need to declare an array of configuration changes in every
converted CLI command, simplifying things quite considerably.

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

lib/command.c

@@ -1051,8 +1051,13 @@ static int cmd_execute_command_real(vector vline, enum filter_type filter,
 	int ret;
 	if (matched_element->daemon)
 		ret = CMD_SUCCESS_DAEMON;
-	else
+	else {
+		/* Clear enqueued configuration changes. */
+		vty->num_cfg_changes = 0;
+		memset(&vty->cfg_changes, 0, sizeof(vty->cfg_changes));
+
 		ret = matched_element->func(matched_element, vty, argc, argv);
+	}
 
 	// delete list and cmd_token's in it
 	list_delete(&argv_list);

lib/if.c

@@ -1086,12 +1086,6 @@ DEFPY_NOSH (interface,
        VRF_CMD_HELP_STR)
 {
 	char xpath_list[XPATH_MAXLEN];
-	struct cli_config_change changes[] = {
-		{
-			.xpath = ".",
-			.operation = NB_OP_CREATE,
-		},
-	};
 	vrf_id_t vrf_id;
 	struct interface *ifp;
 	int ret;
@@ -1136,7 +1130,8 @@ DEFPY_NOSH (interface,
 		 "/frr-interface:lib/interface[name='%s'][vrf='%s']", ifname,
 		 vrfname);
 
-	ret = nb_cli_cfg_change(vty, xpath_list, changes, array_size(changes));
+	nb_cli_enqueue_change(vty, ".", NB_OP_CREATE, NULL);
+	ret = nb_cli_apply_changes(vty, xpath_list);
 	if (ret == CMD_SUCCESS) {
 		VTY_PUSH_XPATH(INTERFACE_NODE, xpath_list);
 
@@ -1162,22 +1157,14 @@ DEFPY (no_interface,
        "Interface's name\n"
        VRF_CMD_HELP_STR)
 {
-	char xpath_list[XPATH_MAXLEN];
-	struct cli_config_change changes[] = {
-		{
-			.xpath = ".",
-			.operation = NB_OP_DELETE,
-		},
-	};
-
 	if (!vrfname)
 		vrfname = VRF_DEFAULT_NAME;
 
-	snprintf(xpath_list, sizeof(xpath_list),
+	nb_cli_enqueue_change(vty, ".", NB_OP_DELETE, NULL);
-		 "/frr-interface:lib/interface[name='%s'][vrf='%s']", ifname,
-		 vrfname);
 
-	return nb_cli_cfg_change(vty, xpath_list, changes, array_size(changes));
+	return nb_cli_apply_changes(
+		vty, "/frr-interface:lib/interface[name='%s'][vrf='%s']",
+		ifname, vrfname);
 }
 
 static void cli_show_interface(struct vty *vty, struct lyd_node *dnode,
@@ -1203,18 +1190,12 @@ DEFPY (interface_desc,
        "Interface specific description\n"
        "Characters describing this interface\n")
 {
-	struct cli_config_change changes[] = {
-		{
-			.xpath = "./description",
-			.operation = NB_OP_MODIFY,
-		},
-	};
 	char *desc;
 	int ret;
 
 	desc = argv_concat(argv, argc, 1);
-	changes[0].value = desc;
+	nb_cli_enqueue_change(vty, "./description", NB_OP_MODIFY, desc);
-	ret = nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	ret = nb_cli_apply_changes(vty, NULL);
 	XFREE(MTYPE_TMP, desc);
 
 	return ret;
@@ -1226,14 +1207,9 @@ DEFPY  (no_interface_desc,
 	NO_STR
 	"Interface specific description\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./description", NB_OP_DELETE, NULL);
-		{
-			.xpath = "./description",
-			.operation = NB_OP_DELETE,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 static void cli_show_interface_desc(struct vty *vty, struct lyd_node *dnode,

lib/northbound_cli.c

@@ -56,10 +56,30 @@ static void vty_show_libyang_errors(struct vty *vty, struct ly_ctx *ly_ctx)
 	ly_err_clean(ly_ctx, NULL);
 }
 
-int nb_cli_cfg_change(struct vty *vty, char *xpath_base,
+void nb_cli_enqueue_change(struct vty *vty, const char *xpath,
-		      struct cli_config_change changes[], size_t size)
+			   enum nb_operation operation, const char *value)
+{
+	struct vty_cfg_change *change;
+
+	if (vty->num_cfg_changes == VTY_MAXCFGCHANGES) {
+		/* Not expected to happen. */
+		vty_out(vty,
+			"%% Exceeded the maximum number of changes (%u) for a single command\n\n",
+			VTY_MAXCFGCHANGES);
+		return;
+	}
+
+	change = &vty->cfg_changes[vty->num_cfg_changes++];
+	change->xpath = xpath;
+	change->operation = operation;
+	change->value = value;
+}
+
+int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
 {
 	struct nb_config *candidate_transitory;
+	char xpath_base[XPATH_MAXLEN];
+	va_list ap;
 	bool error = false;
 	int ret;
 
@@ -72,9 +92,14 @@ int nb_cli_cfg_change(struct vty *vty, char *xpath_base,
 	 */
 	candidate_transitory = nb_config_dup(vty->candidate_config);
 
+	/* Parse the base XPath format string. */
+	va_start(ap, xpath_base_fmt);
+	vsnprintf(xpath_base, sizeof(xpath_base), xpath_base_fmt, ap);
+	va_end(ap);
+
 	/* Edit candidate configuration. */
-	for (size_t i = 0; i < size; i++) {
+	for (size_t i = 0; i < vty->num_cfg_changes; i++) {
-		struct cli_config_change *change = &changes[i];
+		struct vty_cfg_change *change = &vty->cfg_changes[i];
 		struct nb_node *nb_node;
 		char xpath[XPATH_MAXLEN];
 		struct yang_data *data;
@@ -82,12 +107,13 @@ int nb_cli_cfg_change(struct vty *vty, char *xpath_base,
 		/* Handle relative XPaths. */
 		memset(xpath, 0, sizeof(xpath));
 		if (vty->xpath_index > 0
-		    && ((xpath_base && xpath_base[0] == '.')
+		    && ((xpath_base_fmt && xpath_base[0] == '.')
 			|| change->xpath[0] == '.'))
 			strlcpy(xpath, VTY_CURR_XPATH, sizeof(xpath));
-		if (xpath_base) {
+		if (xpath_base_fmt) {
 			if (xpath_base[0] == '.')
-				xpath_base++;
+				strlcat(xpath, xpath_base + 1, sizeof(xpath));
+			else
 				strlcat(xpath, xpath_base, sizeof(xpath));
 		}
 		if (change->xpath[0] == '.')
@@ -95,6 +121,7 @@ int nb_cli_cfg_change(struct vty *vty, char *xpath_base,
 		else
 			strlcpy(xpath, change->xpath, sizeof(xpath));
 
+		/* Find the northbound node associated to the data path. */
 		nb_node = nb_node_find(xpath);
 		if (!nb_node) {
 			flog_warn(EC_LIB_YANG_UNKNOWN_DATA_PATH,

lib/northbound_cli.h

@@ -22,27 +22,6 @@
 
 #include "northbound.h"
 
-struct cli_config_change {
-	/*
-	 * XPath (absolute or relative) of the configuration option being
-	 * edited.
-	 */
-	char xpath[XPATH_MAXLEN];
-
-	/*
-	 * Operation to apply (either NB_OP_CREATE, NB_OP_MODIFY or
-	 * NB_OP_DELETE).
-	 */
-	enum nb_operation operation;
-
-	/*
-	 * New value of the configuration option. Should be NULL for typeless
-	 * YANG data (e.g. presence-containers). For convenience, NULL can also
-	 * be used to restore a leaf to its default value.
-	 */
-	const char *value;
-};
-
 /* Possible formats in which a configuration can be displayed. */
 enum nb_cfg_format {
 	NB_CFG_FMT_CMDS = 0,
@@ -52,13 +31,80 @@ enum nb_cfg_format {
 
 extern struct nb_config *vty_shared_candidate_config;
 
-/* Prototypes. */
+/*
-extern int nb_cli_cfg_change(struct vty *vty, char *xpath_list,
+ * Enqueue change to be applied in the candidate configuration.
-			     struct cli_config_change changes[], size_t size);
+ *
+ * vty
+ *    The vty context.
+ *
+ * xpath
+ *    XPath (absolute or relative) of the configuration option being edited.
+ *
+ * operation
+ *    Operation to apply (either NB_OP_CREATE, NB_OP_MODIFY or NB_OP_DELETE).
+ *
+ * value
+ *    New value of the configuration option. Should be NULL for typeless YANG
+ *    data (e.g. presence-containers). For convenience, NULL can also be used
+ *    to restore a leaf to its default value.
+ */
+extern void nb_cli_enqueue_change(struct vty *vty, const char *xpath,
+				  enum nb_operation operation,
+				  const char *value);
+
+/*
+ * Apply enqueued changes to the candidate configuration.
+ *
+ * vty
+ *    The vty context.
+ *
+ * xpath_base_fmt
+ *    Prepend the given XPath (absolute or relative) to all enqueued
+ *    configuration changes.
+ *
+ * Returns:
+ *    CMD_SUCCESS on success, CMD_WARNING_CONFIG_FAILED otherwise.
+ */
+extern int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt,
+				...);
+
+/*
+ * Execute a YANG RPC or Action.
+ *
+ * xpath
+ *    XPath of the YANG RPC or Action node.
+ *
+ * input
+ *    List of 'yang_data' structures containing the RPC input parameters. It
+ *    can be set to NULL when there are no input parameters.
+ *
+ * output
+ *    List of 'yang_data' structures used to retrieve the RPC output parameters.
+ *    It can be set to NULL when it's known that the given YANG RPC or Action
+ *    doesn't have any output parameters.
+ *
+ * Returns:
+ *    CMD_SUCCESS on success, CMD_WARNING otherwise.
+ */
 extern int nb_cli_rpc(const char *xpath, struct list *input,
 		      struct list *output);
+
+/*
+ * Show CLI commands associated to the given YANG data node.
+ *
+ * vty
+ *    The vty terminal to dump the configuration to.
+ *
+ * dnode
+ *    libyang data node that should be shown in the form of CLI commands.
+ *
+ * show_defaults
+ *    Specify whether to display default configuration values or not.
+ */
 extern void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *dnode,
 				   bool show_defaults);
+
+/* Prototypes of internal functions. */
 extern void nb_cli_install_default(int node);
 extern void nb_cli_init(void);
 extern void nb_cli_terminate(void);

lib/vty.h

@@ -35,11 +35,19 @@
 #define VTY_MAXHIST 20
 #define VTY_MAXDEPTH 8
 
+#define VTY_MAXCFGCHANGES 8
+
 struct vty_error {
 	char error_buf[VTY_BUFSIZ];
 	uint32_t line_num;
 };
 
+struct vty_cfg_change {
+	const char *xpath;
+	enum nb_operation operation;
+	const char *value;
+};
+
 /* VTY struct. */
 struct vty {
 	/* File descripter of this vty. */
@@ -98,6 +106,10 @@ struct vty {
 	/* History insert end point */
 	int hindex;
 
+	/* Changes enqueued to be applied in the candidate configuration. */
+	size_t num_cfg_changes;
+	struct vty_cfg_change cfg_changes[VTY_MAXCFGCHANGES];
+
 	/* XPath of the current node */
 	int xpath_index;
 	char xpath[VTY_MAXDEPTH][XPATH_MAXLEN];

ripd/rip_cli.c

@@ -45,17 +45,12 @@ DEFPY_NOSH (router_rip,
 {
 	int ret;
 
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "/frr-ripd:ripd/instance", NB_OP_CREATE,
-		{
+			      NULL);
-			.xpath = "/frr-ripd:ripd/instance",
-			.operation = NB_OP_CREATE,
-			.value = NULL,
-		},
-	};
 
-	ret = nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	ret = nb_cli_apply_changes(vty, NULL);
 	if (ret == CMD_SUCCESS)
-		VTY_PUSH_XPATH(RIP_NODE, changes[0].xpath);
+		VTY_PUSH_XPATH(RIP_NODE, "/frr-ripd:ripd/instance");
 
 	return ret;
 }
@@ -67,15 +62,10 @@ DEFPY (no_router_rip,
        "Enable a routing process\n"
        "Routing Information Protocol (RIP)\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "/frr-ripd:ripd/instance", NB_OP_DELETE,
-		{
+			      NULL);
-			.xpath = "/frr-ripd:ripd/instance",
-			.operation = NB_OP_DELETE,
-			.value = NULL,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_router_rip(struct vty *vty, struct lyd_node *dnode,
@@ -94,15 +84,10 @@ DEFPY (rip_allow_ecmp,
        NO_STR
        "Allow Equal Cost MultiPath\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./allow-ecmp", NB_OP_MODIFY,
-		{
+			      no ? "false" : "true");
-			.xpath = "./allow-ecmp",
-			.operation = NB_OP_MODIFY,
-			.value = no ? "false" : "true",
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_allow_ecmp(struct vty *vty, struct lyd_node *dnode,
@@ -124,15 +109,10 @@ DEFPY (rip_default_information_originate,
        "Control distribution of default route\n"
        "Distribute a default route\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./default-information-originate",
-		{
+			      NB_OP_MODIFY, no ? "false" : "true");
-			.xpath = "./default-information-originate",
-			.operation = NB_OP_MODIFY,
-			.value = no ? "false" : "true",
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_default_information_originate(struct vty *vty,
@@ -154,15 +134,10 @@ DEFPY (rip_default_metric,
        "Set a metric of redistribute routes\n"
        "Default metric\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./default-metric", NB_OP_MODIFY,
-		{
+			      default_metric_str);
-			.xpath = "./default-metric",
-			.operation = NB_OP_MODIFY,
-			.value = default_metric_str,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 DEFPY (no_rip_default_metric,
@@ -172,15 +147,9 @@ DEFPY (no_rip_default_metric,
        "Set a metric of redistribute routes\n"
        "Default metric\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./default-metric", NB_OP_MODIFY, NULL);
-		{
-			.xpath = "./default-metric",
-			.operation = NB_OP_MODIFY,
-			.value = NULL,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_default_metric(struct vty *vty, struct lyd_node *dnode,
@@ -199,15 +168,10 @@ DEFPY (rip_distance,
        "Administrative distance\n"
        "Distance value\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./distance/default", NB_OP_MODIFY,
-		{
+			      distance_str);
-			.xpath = "./distance/default",
-			.operation = NB_OP_MODIFY,
-			.value = distance_str,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 DEFPY (no_rip_distance,
@@ -217,15 +181,9 @@ DEFPY (no_rip_distance,
        "Administrative distance\n"
        "Distance value\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./distance/default", NB_OP_MODIFY, NULL);
-		{
-			.xpath = "./distance/default",
-			.operation = NB_OP_MODIFY,
-			.value = NULL,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_distance(struct vty *vty, struct lyd_node *dnode,
@@ -243,57 +201,23 @@ void cli_show_rip_distance(struct vty *vty, struct lyd_node *dnode,
  */
 DEFPY (rip_distance_source,
        rip_distance_source_cmd,
-       "distance (1-255) A.B.C.D/M$prefix [WORD$acl]",
+       "[no] distance (1-255) A.B.C.D/M$prefix [WORD$acl]",
-       "Administrative distance\n"
-       "Distance value\n"
-       "IP source prefix\n"
-       "Access list name\n")
-{
-	char xpath_list[XPATH_MAXLEN];
-	struct cli_config_change changes[] = {
-		{
-			.xpath = ".",
-			.operation = NB_OP_CREATE,
-		},
-		{
-			.xpath = "./distance",
-			.operation = NB_OP_MODIFY,
-			.value = distance_str,
-		},
-		{
-			.xpath = "./access-list",
-			.operation = acl ? NB_OP_MODIFY : NB_OP_DELETE,
-			.value = acl,
-		},
-	};
-
-	snprintf(xpath_list, sizeof(xpath_list),
-		 "./distance/source[prefix='%s']", prefix_str);
-
-	return nb_cli_cfg_change(vty, xpath_list, changes, array_size(changes));
-}
-
-DEFPY (no_rip_distance_source,
-       no_rip_distance_source_cmd,
-       "no distance (1-255) A.B.C.D/M$prefix [WORD$acl]",
        NO_STR
        "Administrative distance\n"
        "Distance value\n"
        "IP source prefix\n"
        "Access list name\n")
 {
-	char xpath_list[XPATH_MAXLEN];
+	if (!no) {
-	struct cli_config_change changes[] = {
+		nb_cli_enqueue_change(vty, ".", NB_OP_CREATE, NULL);
-		{
+		nb_cli_enqueue_change(vty, "./distance", NB_OP_MODIFY, NULL);
-			.xpath = ".",
+		nb_cli_enqueue_change(vty, "./access-list",
-			.operation = NB_OP_DELETE,
+				      acl ? NB_OP_MODIFY : NB_OP_DELETE, acl);
-		},
+	} else
-	};
+		nb_cli_enqueue_change(vty, ".", NB_OP_DELETE, NULL);
 
-	snprintf(xpath_list, sizeof(xpath_list),
+	return nb_cli_apply_changes(vty, "./distance/source[prefix='%s']",
-		 "./distance/source[prefix='%s']", prefix_str);
+				    prefix_str);
-
-	return nb_cli_cfg_change(vty, xpath_list, changes, 1);
 }
 
 void cli_show_rip_distance_source(struct vty *vty, struct lyd_node *dnode,
@@ -318,15 +242,10 @@ DEFPY (rip_neighbor,
        "Specify a neighbor router\n"
        "Neighbor address\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./explicit-neighbor",
-		{
+			      no ? NB_OP_DELETE : NB_OP_CREATE, neighbor_str);
-			.xpath = "./explicit-neighbor",
-			.operation = no ? NB_OP_DELETE : NB_OP_CREATE,
-			.value = neighbor_str,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_neighbor(struct vty *vty, struct lyd_node *dnode,
@@ -345,15 +264,10 @@ DEFPY (rip_network_prefix,
        "Enable routing on an IP network\n"
        "IP prefix <network>/<length>, e.g., 35.0.0.0/8\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./network",
-		{
+			      no ? NB_OP_DELETE : NB_OP_CREATE, network_str);
-			.xpath = "./network",
-			.operation = no ? NB_OP_DELETE : NB_OP_CREATE,
-			.value = network_str,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_network_prefix(struct vty *vty, struct lyd_node *dnode,
@@ -372,15 +286,10 @@ DEFPY (rip_network_if,
        "Enable routing on an IP network\n"
        "Interface name\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./interface",
-		{
+			      no ? NB_OP_DELETE : NB_OP_CREATE, network);
-			.xpath = "./interface",
-			.operation = no ? NB_OP_DELETE : NB_OP_CREATE,
-			.value = network,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_network_interface(struct vty *vty, struct lyd_node *dnode,
@@ -394,42 +303,7 @@ void cli_show_rip_network_interface(struct vty *vty, struct lyd_node *dnode,
  */
 DEFPY (rip_offset_list,
        rip_offset_list_cmd,
-       "offset-list WORD$acl <in|out>$direction (0-16)$metric [IFNAME]",
+       "[no] offset-list WORD$acl <in|out>$direction (0-16)$metric [IFNAME]",
-       "Modify RIP metric\n"
-       "Access-list name\n"
-       "For incoming updates\n"
-       "For outgoing updates\n"
-       "Metric value\n"
-       "Interface to match\n")
-{
-	char xpath_list[XPATH_MAXLEN];
-	struct cli_config_change changes[] = {
-		{
-			.xpath = ".",
-			.operation = NB_OP_CREATE,
-		},
-		{
-			.xpath = "./access-list",
-			.operation = NB_OP_MODIFY,
-			.value = acl,
-		},
-		{
-			.xpath = "./metric",
-			.operation = NB_OP_MODIFY,
-			.value = metric_str,
-		},
-	};
-
-	snprintf(xpath_list, sizeof(xpath_list),
-		 "./offset-list[interface='%s'][direction='%s']",
-		 ifname ? ifname : "*", direction);
-
-	return nb_cli_cfg_change(vty, xpath_list, changes, array_size(changes));
-}
-
-DEFPY (no_rip_offset_list,
-       no_rip_offset_list_cmd,
-       "no offset-list WORD$acl <in|out>$direction (0-16)$metric [IFNAME]",
        NO_STR
        "Modify RIP metric\n"
        "Access-list name\n"
@@ -438,19 +312,17 @@ DEFPY (no_rip_offset_list,
        "Metric value\n"
        "Interface to match\n")
 {
-	char xpath_list[XPATH_MAXLEN];
+	if (!no) {
-	struct cli_config_change changes[] = {
+		nb_cli_enqueue_change(vty, ".", NB_OP_CREATE, NULL);
-		{
+		nb_cli_enqueue_change(vty, "./access-list", NB_OP_MODIFY, acl);
-			.xpath = ".",
+		nb_cli_enqueue_change(vty, "./metric", NB_OP_MODIFY,
-			.operation = NB_OP_DELETE,
+				      metric_str);
-		},
+	} else
-	};
+		nb_cli_enqueue_change(vty, ".", NB_OP_DELETE, NULL);
 
-	snprintf(xpath_list, sizeof(xpath_list),
+	return nb_cli_apply_changes(
-		 "./offset-list[interface='%s'][direction='%s']",
+		vty, "./offset-list[interface='%s'][direction='%s']",
 		ifname ? ifname : "*", direction);
-
-	return nb_cli_cfg_change(vty, xpath_list, changes, array_size(changes));
 }
 
 void cli_show_rip_offset_list(struct vty *vty, struct lyd_node *dnode,
@@ -479,15 +351,10 @@ DEFPY (rip_passive_default,
        "Suppress routing updates on an interface\n"
        "default for all interfaces\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./passive-default", NB_OP_MODIFY,
-		{
+			      no ? "false" : "true");
-			.xpath = "./passive-default",
-			.operation = NB_OP_MODIFY,
-			.value = no ? "false" : "true",
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_passive_default(struct vty *vty, struct lyd_node *dnode,
@@ -510,20 +377,12 @@ DEFPY (rip_passive_interface,
        "Suppress routing updates on an interface\n"
        "Interface name\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./passive-interface",
-		{
+			      no ? NB_OP_DELETE : NB_OP_CREATE, ifname);
-			.xpath = "./passive-interface",
+	nb_cli_enqueue_change(vty, "./non-passive-interface",
-			.operation = no ? NB_OP_DELETE : NB_OP_CREATE,
+			      no ? NB_OP_CREATE : NB_OP_DELETE, ifname);
-			.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));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_passive_interface(struct vty *vty, struct lyd_node *dnode,
@@ -545,41 +404,7 @@ void cli_show_rip_non_passive_interface(struct vty *vty, struct lyd_node *dnode,
  */
 DEFPY (rip_redistribute,
        rip_redistribute_cmd,
-       "redistribute " FRR_REDIST_STR_RIPD "$protocol [{metric (0-16)|route-map WORD}]",
+       "[no] redistribute " FRR_REDIST_STR_RIPD "$protocol [{metric (0-16)|route-map WORD}]",
-       REDIST_STR
-       FRR_REDIST_HELP_STR_RIPD
-       "Metric\n"
-       "Metric value\n"
-       "Route map reference\n"
-       "Pointer to route-map entries\n")
-{
-	char xpath_list[XPATH_MAXLEN];
-	struct cli_config_change changes[] = {
-		{
-			.xpath = ".",
-			.operation = NB_OP_CREATE,
-		},
-		{
-			.xpath = "./route-map",
-			.operation = route_map ? NB_OP_MODIFY : NB_OP_DELETE,
-			.value = route_map,
-		},
-		{
-			.xpath = "./metric",
-			.operation = metric_str ? NB_OP_MODIFY : NB_OP_DELETE,
-			.value = metric_str,
-		},
-	};
-
-	snprintf(xpath_list, sizeof(xpath_list),
-		 "./redistribute[protocol='%s']", protocol);
-
-	return nb_cli_cfg_change(vty, xpath_list, changes, array_size(changes));
-}
-
-DEFPY (no_rip_redistribute,
-       no_rip_redistribute_cmd,
-       "no redistribute " FRR_REDIST_STR_RIPD "$protocol [{metric (0-16)|route-map WORD}]",
        NO_STR
        REDIST_STR
        FRR_REDIST_HELP_STR_RIPD
@@ -588,18 +413,19 @@ DEFPY (no_rip_redistribute,
        "Route map reference\n"
        "Pointer to route-map entries\n")
 {
-	char xpath_list[XPATH_MAXLEN];
+	if (!no) {
-	struct cli_config_change changes[] = {
+		nb_cli_enqueue_change(vty, ".", NB_OP_CREATE, NULL);
-		{
+		nb_cli_enqueue_change(vty, "./route-map",
-			.xpath = ".",
+				      route_map ? NB_OP_MODIFY : NB_OP_DELETE,
-			.operation = NB_OP_DELETE,
+				      route_map);
-		},
+		nb_cli_enqueue_change(vty, "./metric",
-	};
+				      metric_str ? NB_OP_MODIFY : NB_OP_DELETE,
+				      metric_str);
+	} else
+		nb_cli_enqueue_change(vty, ".", NB_OP_DELETE, NULL);
 
-	snprintf(xpath_list, sizeof(xpath_list),
+	return nb_cli_apply_changes(vty, "./redistribute[protocol='%s']",
-		 "./redistribute[protocol='%s']", protocol);
+				    protocol);
-
-	return nb_cli_cfg_change(vty, xpath_list, changes, array_size(changes));
 }
 
 void cli_show_rip_redistribute(struct vty *vty, struct lyd_node *dnode,
@@ -626,15 +452,10 @@ DEFPY (rip_route,
        "RIP static route configuration\n"
        "IP prefix <network>/<length>\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./static-route",
-		{
+			      no ? NB_OP_DELETE : NB_OP_CREATE, route_str);
-			.xpath = "./static-route",
-			.operation = no ? NB_OP_DELETE : NB_OP_CREATE,
-			.value = route_str,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_route(struct vty *vty, struct lyd_node *dnode,
@@ -655,25 +476,14 @@ DEFPY (rip_timers,
        "Routing information timeout timer. Default is 180.\n"
        "Garbage collection timer. Default is 120.\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./update-interval", NB_OP_MODIFY,
-		{
+			      update_str);
-			.xpath = "./timers/update-interval",
+	nb_cli_enqueue_change(vty, "./holddown-interval", NB_OP_MODIFY,
-			.operation = NB_OP_MODIFY,
+			      timeout_str);
-			.value = update_str,
+	nb_cli_enqueue_change(vty, "./flush-interval", NB_OP_MODIFY,
-		},
+			      garbage_str);
-		{
-			.xpath = "./timers/holddown-interval",
-			.operation = NB_OP_MODIFY,
-			.value = timeout_str,
-		},
-		{
-			.xpath = "./timers/flush-interval",
-			.operation = NB_OP_MODIFY,
-			.value = garbage_str,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./timers");
 }
 
 DEFPY (no_rip_timers,
@@ -686,25 +496,11 @@ DEFPY (no_rip_timers,
        "Routing information timeout timer. Default is 180.\n"
        "Garbage collection timer. Default is 120.\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./update-interval", NB_OP_MODIFY, NULL);
-		{
+	nb_cli_enqueue_change(vty, "./holddown-interval", NB_OP_MODIFY, NULL);
-			.xpath = "./timers/update-interval",
+	nb_cli_enqueue_change(vty, "./flush-interval", NB_OP_MODIFY, NULL);
-			.operation = NB_OP_MODIFY,
-			.value =  NULL,
-		},
-		{
-			.xpath = "./timers/holddown-interval",
-			.operation = NB_OP_MODIFY,
-			.value =  NULL,
-		},
-		{
-			.xpath = "./timers/flush-interval",
-			.operation = NB_OP_MODIFY,
-			.value =  NULL,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./timers");
 }
 
 void cli_show_rip_timers(struct vty *vty, struct lyd_node *dnode,
@@ -725,20 +521,11 @@ DEFPY (rip_version,
        "Set routing protocol version\n"
        "version\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./version/receive", NB_OP_MODIFY,
-		{
+			      version_str);
-			.xpath = "./version/receive",
+	nb_cli_enqueue_change(vty, "./version/send", NB_OP_MODIFY, version_str);
-			.operation = NB_OP_MODIFY,
-			.value = version_str,
-		},
-		{
-			.xpath = "./version/send",
-			.operation = NB_OP_MODIFY,
-			.value = version_str,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 DEFPY (no_rip_version,
@@ -748,18 +535,10 @@ DEFPY (no_rip_version,
        "Set routing protocol version\n"
        "version\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./version/receive", NB_OP_MODIFY, NULL);
-		{
+	nb_cli_enqueue_change(vty, "./version/send", NB_OP_MODIFY, NULL);
-			.xpath = "./version/receive",
-			.operation = NB_OP_MODIFY,
-		},
-		{
-			.xpath = "./version/send",
-			.operation = NB_OP_MODIFY,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_rip_version(struct vty *vty, struct lyd_node *dnode,
@@ -794,21 +573,18 @@ DEFPY (ip_rip_split_horizon,
        "Perform split horizon\n"
        "With poisoned-reverse\n")
 {
-	struct cli_config_change changes[] = {
+	const char *value;
-		{
-			.xpath = "./frr-ripd:rip/split-horizon",
-			.operation = NB_OP_MODIFY,
-		},
-	};
 
 	if (no)
-		changes[0].value = "disabled";
+		value = "disabled";
 	else if (poisoned_reverse)
-		changes[0].value = "poison-reverse";
+		value = "poison-reverse";
 	else
-		changes[0].value = "simple";
+		value = "simple";
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	nb_cli_enqueue_change(vty, "./split-horizon", NB_OP_MODIFY, value);
+
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 void cli_show_ip_rip_split_horizon(struct vty *vty, struct lyd_node *dnode,
@@ -841,15 +617,10 @@ DEFPY (ip_rip_v2_broadcast,
        "Routing Information Protocol\n"
        "Send ip broadcast v2 update\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./v2-broadcast", NB_OP_MODIFY,
-		{
+			      no ? "false" : "true");
-			.xpath = "./frr-ripd:rip/v2-broadcast",
-			.operation = NB_OP_MODIFY,
-			.value = no ? "false" : "true",
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 void cli_show_ip_rip_v2_broadcast(struct vty *vty, struct lyd_node *dnode,
@@ -875,23 +646,20 @@ DEFPY (ip_rip_receive_version,
        "RIP version 2\n"
        "None\n")
 {
-	struct cli_config_change changes[] = {
+	const char *value;
-		{
-			.xpath = "./frr-ripd:rip/version-receive",
-			.operation = NB_OP_MODIFY,
-		},
-	};
 
 	if (v1 && v2)
-		changes[0].value = "both";
+		value = "both";
 	else if (v1)
-		changes[0].value = "1";
+		value = "1";
 	else if (v2)
-		changes[0].value = "2";
+		value = "2";
 	else
-		changes[0].value = "none";
+		value = "none";
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	nb_cli_enqueue_change(vty, "./version-receive", NB_OP_MODIFY, value);
+
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 DEFPY (no_ip_rip_receive_version,
@@ -906,15 +674,9 @@ DEFPY (no_ip_rip_receive_version,
        "RIP version 2\n"
        "None\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./version-receive", NB_OP_MODIFY, NULL);
-		{
-			.xpath = "./frr-ripd:rip/version-receive",
-			.operation = NB_OP_MODIFY,
-			.value = NULL,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 void cli_show_ip_rip_receive_version(struct vty *vty, struct lyd_node *dnode,
@@ -953,23 +715,20 @@ DEFPY (ip_rip_send_version,
        "RIP version 2\n"
        "None\n")
 {
-	struct cli_config_change changes[] = {
+	const char *value;
-		{
-			.xpath = "./frr-ripd:rip/version-send",
-			.operation = NB_OP_MODIFY,
-		},
-	};
 
 	if (v1 && v2)
-		changes[0].value = "both";
+		value = "both";
 	else if (v1)
-		changes[0].value = "1";
+		value = "1";
 	else if (v2)
-		changes[0].value = "2";
+		value = "2";
 	else
-		changes[0].value = "none";
+		value = "none";
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	nb_cli_enqueue_change(vty, "./version-send", NB_OP_MODIFY, value);
+
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 DEFPY (no_ip_rip_send_version,
@@ -984,15 +743,9 @@ DEFPY (no_ip_rip_send_version,
        "RIP version 2\n"
        "None\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./version-send", NB_OP_MODIFY, NULL);
-		{
-			.xpath = "./frr-ripd:rip/version-send",
-			.operation = NB_OP_MODIFY,
-			.value = NULL,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 void cli_show_ip_rip_send_version(struct vty *vty, struct lyd_node *dnode,
@@ -1033,26 +786,21 @@ DEFPY (ip_rip_authentication_mode,
        "Old ripd compatible\n"
        "Clear text authentication\n")
 {
-	struct cli_config_change changes[] = {
+	const char *value = NULL;
-		{
-			.xpath = "./frr-ripd:rip/authentication-scheme/mode",
-			.operation = NB_OP_MODIFY,
-			.value = strmatch(mode, "md5") ? "md5" : "plain-text",
-		},
-		{
-			.xpath = "./frr-ripd:rip/authentication-scheme/md5-auth-length",
-			.operation = NB_OP_MODIFY,
-		},
-	};
 
 	if (auth_length) {
 		if (strmatch(auth_length, "rfc"))
-			changes[1].value = "16";
+			value = "16";
 		else
-			changes[1].value = "20";
+			value = "20";
 	}
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	nb_cli_enqueue_change(vty, "./authentication-scheme/mode", NB_OP_MODIFY,
+			      strmatch(mode, "md5") ? "md5" : "plain-text");
+	nb_cli_enqueue_change(vty, "./authentication-scheme/md5-auth-length",
+			      NB_OP_MODIFY, value);
+
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 DEFPY (no_ip_rip_authentication_mode,
@@ -1069,18 +817,12 @@ DEFPY (no_ip_rip_authentication_mode,
        "Old ripd compatible\n"
        "Clear text authentication\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./authentication-scheme/mode", NB_OP_MODIFY,
-		{
+			      NULL);
-			.xpath = "./frr-ripd:rip/authentication-scheme/mode",
+	nb_cli_enqueue_change(vty, "./authentication-scheme/md5-auth-length",
-			.operation = NB_OP_MODIFY,
+			      NB_OP_MODIFY, NULL);
-		},
-		{
-			.xpath = "./frr-ripd:rip/authentication-scheme/md5-auth-length",
-			.operation = NB_OP_MODIFY,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 void cli_show_ip_rip_authentication_scheme(struct vty *vty,
@@ -1121,14 +863,6 @@ DEFPY (ip_rip_authentication_string,
        "Authentication string\n"
        "Authentication string\n")
 {
-	struct cli_config_change changes[] = {
-		{
-			.xpath = "./frr-ripd:rip/authentication-password",
-			.operation = NB_OP_MODIFY,
-			.value = password,
-		},
-	};
-
 	if (strlen(password) > 16) {
 		vty_out(vty,
 			"%% RIPv2 authentication string must be shorter than 16\n");
@@ -1142,7 +876,10 @@ DEFPY (ip_rip_authentication_string,
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	nb_cli_enqueue_change(vty, "./authentication-password", NB_OP_MODIFY,
+			      password);
+
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 DEFPY (no_ip_rip_authentication_string,
@@ -1155,14 +892,10 @@ DEFPY (no_ip_rip_authentication_string,
        "Authentication string\n"
        "Authentication string\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./authentication-password", NB_OP_MODIFY,
-		{
+			      NULL);
-			.xpath = "./frr-ripd:rip/authentication-password",
-			.operation = NB_OP_DELETE,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 void cli_show_ip_rip_authentication_string(struct vty *vty,
@@ -1185,14 +918,6 @@ DEFPY (ip_rip_authentication_key_chain,
        "Authentication key-chain\n"
        "name of key-chain\n")
 {
-	struct cli_config_change changes[] = {
-		{
-			.xpath = "./frr-ripd:rip/authentication-key-chain",
-			.operation = NB_OP_MODIFY,
-			.value = keychain,
-		},
-	};
-
 	if (yang_dnode_exists(vty->candidate_config->dnode, "%s%s",
 			      VTY_CURR_XPATH,
 			      "/frr-ripd:rip/authentication-password")) {
@@ -1200,7 +925,10 @@ DEFPY (ip_rip_authentication_key_chain,
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	nb_cli_enqueue_change(vty, "./authentication-key-chain", NB_OP_MODIFY,
+			      keychain);
+
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 DEFPY (no_ip_rip_authentication_key_chain,
@@ -1213,14 +941,10 @@ DEFPY (no_ip_rip_authentication_key_chain,
        "Authentication key-chain\n"
        "name of key-chain\n")
 {
-	struct cli_config_change changes[] = {
+	nb_cli_enqueue_change(vty, "./authentication-key-chain", NB_OP_DELETE,
-		{
+			      NULL);
-			.xpath = "./frr-ripd:rip/authentication-key-chain",
-			.operation = NB_OP_DELETE,
-		},
-	};
 
-	return nb_cli_cfg_change(vty, NULL, changes, array_size(changes));
+	return nb_cli_apply_changes(vty, "./frr-ripd:rip");
 }
 
 void cli_show_ip_rip_authentication_key_chain(struct vty *vty,
@@ -1256,16 +980,13 @@ void rip_cli_init(void)
 	install_element(RIP_NODE, &rip_distance_cmd);
 	install_element(RIP_NODE, &no_rip_distance_cmd);
 	install_element(RIP_NODE, &rip_distance_source_cmd);
-	install_element(RIP_NODE, &no_rip_distance_source_cmd);
 	install_element(RIP_NODE, &rip_neighbor_cmd);
 	install_element(RIP_NODE, &rip_network_prefix_cmd);
 	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);
 	install_element(RIP_NODE, &rip_redistribute_cmd);
-	install_element(RIP_NODE, &no_rip_redistribute_cmd);
 	install_element(RIP_NODE, &rip_route_cmd);
 	install_element(RIP_NODE, &rip_timers_cmd);
 	install_element(RIP_NODE, &no_rip_timers_cmd);