Merge pull request FRRouting#14768 from idryzhov/mgmtd-base-xpath

lib, mgmtd: respect base xpath in mgmtd
donaldsharp · Nov 13, 2023 · c62c018 · c62c018
2 parents 5dcd9f6 + b8a2efb
commit c62c018
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 38 deletions.
diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c
@@ -426,8 +426,7 @@ static int mgmt_be_txn_cfg_prepare(struct mgmt_be_txn_ctx *txn)
 				client_ctx->candidate_config,
 				txn_req->req.set_cfg.cfg_changes,
 				(size_t)txn_req->req.set_cfg.num_cfg_changes,
-				NULL, NULL, 0, err_buf, sizeof(err_buf),
-				&error);
+				NULL, err_buf, sizeof(err_buf), &error);
 			if (error) {
 				err_buf[sizeof(err_buf) - 1] = 0;
 				MGMTD_BE_CLIENT_ERR(

diff --git a/lib/northbound.c b/lib/northbound.c
@@ -763,8 +763,8 @@ static bool nb_is_operation_allowed(struct nb_node *nb_node,
 
 void nb_candidate_edit_config_changes(
 	struct nb_config *candidate_config, struct nb_cfg_change cfg_changes[],
-	size_t num_cfg_changes, const char *xpath_base, const char *curr_xpath,
-	int xpath_index, char *err_buf, int err_bufsize, bool *error)
+	size_t num_cfg_changes, const char *xpath_base, char *err_buf,
+	int err_bufsize, bool *error)
 {
 	if (error)
 		*error = false;
@@ -776,26 +776,19 @@ void nb_candidate_edit_config_changes(
 	for (size_t i = 0; i < num_cfg_changes; i++) {
 		struct nb_cfg_change *change = &cfg_changes[i];
 		struct nb_node *nb_node;
+		char *change_xpath = change->xpath;
 		char xpath[XPATH_MAXLEN];
 		const char *value;
 		struct yang_data *data;
 		int ret;
 
-		/* Handle relative XPaths. */
 		memset(xpath, 0, sizeof(xpath));
-		if (xpath_index > 0 &&
-		    (xpath_base[0] == '.' || change->xpath[0] == '.'))
-			strlcpy(xpath, curr_xpath, sizeof(xpath));
-		if (xpath_base[0]) {
-			if (xpath_base[0] == '.')
-				strlcat(xpath, xpath_base + 1, sizeof(xpath));
-			else
-				strlcat(xpath, xpath_base, sizeof(xpath));
+		/* If change xpath is relative, prepend base xpath. */
+		if (change_xpath[0] == '.') {
+			strlcpy(xpath, xpath_base, sizeof(xpath));
+			change_xpath++; /* skip '.' */
 		}
-		if (change->xpath[0] == '.')
-			strlcat(xpath, change->xpath + 1, sizeof(xpath));
-		else
-			strlcpy(xpath, change->xpath, sizeof(xpath));
+		strlcat(xpath, change_xpath, sizeof(xpath));
 
 		/* Find the northbound node associated to the data path. */
 		nb_node = nb_node_find(xpath);

diff --git a/lib/northbound.h b/lib/northbound.h
@@ -916,12 +916,6 @@ extern bool nb_candidate_needs_update(const struct nb_config *candidate);
  * xpath_base
  *    Base xpath for config.
  *
- * curr_xpath
- *    Current xpath for config.
- *
- * xpath_index
- *    Index of xpath being processed.
- *
  * err_buf
  *    Buffer to store human-readable error message in case of error.
  *
@@ -933,8 +927,8 @@ extern bool nb_candidate_needs_update(const struct nb_config *candidate);
  */
 extern void nb_candidate_edit_config_changes(
 	struct nb_config *candidate_config, struct nb_cfg_change cfg_changes[],
-	size_t num_cfg_changes, const char *xpath_base, const char *curr_xpath,
-	int xpath_index, char *err_buf, int err_bufsize, bool *error);
+	size_t num_cfg_changes, const char *xpath_base, char *err_buf,
+	int err_bufsize, bool *error);
 
 /*
  * Delete candidate configuration changes.

diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
@@ -147,8 +147,7 @@ static int nb_cli_apply_changes_internal(struct vty *vty,
 
 	nb_candidate_edit_config_changes(
 		vty->candidate_config, vty->cfg_changes, vty->num_cfg_changes,
-		xpath_base, VTY_CURR_XPATH, vty->xpath_index, buf, sizeof(buf),
-		&error);
+		xpath_base, buf, sizeof(buf), &error);
 	if (error) {
 		/*
 		 * Failure to edit the candidate configuration should never
@@ -180,8 +179,26 @@ static int nb_cli_apply_changes_internal(struct vty *vty,
 	return CMD_SUCCESS;
 }
 
+static void create_xpath_base_abs(struct vty *vty, char *xpath_base_abs,
+				  size_t xpath_base_abs_size,
+				  const char *xpath_base)
+{
+	memset(xpath_base_abs, 0, xpath_base_abs_size);
+
+	if (xpath_base[0] == 0)
+		xpath_base = ".";
+
+	/* If base xpath is relative, prepend current vty xpath. */
+	if (vty->xpath_index > 0 && xpath_base[0] == '.') {
+		strlcpy(xpath_base_abs, VTY_CURR_XPATH, xpath_base_abs_size);
+		xpath_base++; /* skip '.' */
+	}
+	strlcat(xpath_base_abs, xpath_base, xpath_base_abs_size);
+}
+
 int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
 {
+	char xpath_base_abs[XPATH_MAXLEN] = {};
 	char xpath_base[XPATH_MAXLEN] = {};
 	bool implicit_commit;
 	int ret;
@@ -195,25 +212,30 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
 		va_end(ap);
 	}
 
+	create_xpath_base_abs(vty, xpath_base_abs, sizeof(xpath_base_abs),
+			      xpath_base);
+
 	if (vty_mgmt_should_process_cli_apply_changes(vty)) {
 		VTY_CHECK_XPATH;
 
 		if (vty->type == VTY_FILE)
 			return CMD_SUCCESS;
 
 		implicit_commit = vty_needs_implicit_commit(vty);
-		ret = vty_mgmt_send_config_data(vty, implicit_commit);
+		ret = vty_mgmt_send_config_data(vty, xpath_base_abs,
+						implicit_commit);
 		if (ret >= 0 && !implicit_commit)
 			vty->mgmt_num_pending_setcfg++;
 		return ret;
 	}
 
-	return nb_cli_apply_changes_internal(vty, xpath_base, false);
+	return nb_cli_apply_changes_internal(vty, xpath_base_abs, false);
 }
 
 int nb_cli_apply_changes_clear_pending(struct vty *vty,
 				       const char *xpath_base_fmt, ...)
 {
+	char xpath_base_abs[XPATH_MAXLEN] = {};
 	char xpath_base[XPATH_MAXLEN] = {};
 	bool implicit_commit;
 	int ret;
@@ -227,6 +249,9 @@ int nb_cli_apply_changes_clear_pending(struct vty *vty,
 		va_end(ap);
 	}
 
+	create_xpath_base_abs(vty, xpath_base_abs, sizeof(xpath_base_abs),
+			      xpath_base);
+
 	if (vty_mgmt_should_process_cli_apply_changes(vty)) {
 		VTY_CHECK_XPATH;
 		/*
@@ -238,13 +263,14 @@ int nb_cli_apply_changes_clear_pending(struct vty *vty,
 		 * conversions to mgmtd require full proper implementations.
 		 */
 		implicit_commit = vty_needs_implicit_commit(vty);
-		ret = vty_mgmt_send_config_data(vty, implicit_commit);
+		ret = vty_mgmt_send_config_data(vty, xpath_base_abs,
+						implicit_commit);
 		if (ret >= 0 && !implicit_commit)
 			vty->mgmt_num_pending_setcfg++;
 		return ret;
 	}
 
-	return nb_cli_apply_changes_internal(vty, xpath_base, true);
+	return nb_cli_apply_changes_internal(vty, xpath_base_abs, true);
 }
 
 int nb_cli_rpc(struct vty *vty, const char *xpath, struct list *input,

diff --git a/lib/vty.c b/lib/vty.c
@@ -3718,12 +3718,15 @@ int vty_mgmt_send_lockds_req(struct vty *vty, Mgmtd__DatastoreId ds_id,
 	return 0;
 }
 
-int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)
+int vty_mgmt_send_config_data(struct vty *vty, const char *xpath_base,
+			      bool implicit_commit)
 {
 	Mgmtd__YangDataValue value[VTY_MAXCFGCHANGES];
 	Mgmtd__YangData cfg_data[VTY_MAXCFGCHANGES];
 	Mgmtd__YangCfgDataReq cfg_req[VTY_MAXCFGCHANGES];
 	Mgmtd__YangCfgDataReq *cfgreq[VTY_MAXCFGCHANGES] = {0};
+	char xpath[VTY_MAXCFGCHANGES][XPATH_MAXLEN];
+	char *change_xpath;
 	size_t indx;
 
 	if (vty->type == VTY_FILE) {
@@ -3759,6 +3762,9 @@ int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)
 		}
 	}
 
+	if (xpath_base == NULL)
+		xpath_base = "";
+
 	for (indx = 0; indx < vty->num_cfg_changes; indx++) {
 		mgmt_yang_data_init(&cfg_data[indx]);
 
@@ -3771,7 +3777,17 @@ int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)
 			cfg_data[indx].value = &value[indx];
 		}
 
-		cfg_data[indx].xpath = vty->cfg_changes[indx].xpath;
+		change_xpath = vty->cfg_changes[indx].xpath;
+
+		memset(xpath[indx], 0, sizeof(xpath[indx]));
+		/* If change xpath is relative, prepend base xpath. */
+		if (change_xpath[0] == '.') {
+			strlcpy(xpath[indx], xpath_base, sizeof(xpath[indx]));
+			change_xpath++; /* skip '.' */
+		}
+		strlcat(xpath[indx], change_xpath, sizeof(xpath[indx]));
+
+		cfg_data[indx].xpath = xpath[indx];
 
 		mgmt_yang_cfg_data_req_init(&cfg_req[indx]);
 		cfg_req[indx].data = &cfg_data[indx];

diff --git a/lib/vty.h b/lib/vty.h
@@ -412,7 +412,8 @@ extern bool vty_mgmt_fe_enabled(void);
 extern bool vty_mgmt_should_process_cli_apply_changes(struct vty *vty);
 
 extern bool mgmt_vty_read_configs(void);
-extern int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit);
+extern int vty_mgmt_send_config_data(struct vty *vty, const char *xpath_base,
+				     bool implicit_commit);
 extern int vty_mgmt_send_commit_config(struct vty *vty, bool validate_only,
 				       bool abort);
 extern int vty_mgmt_send_get_req(struct vty *vty, bool is_config,

diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
@@ -664,8 +664,8 @@ static void mgmt_txn_process_set_cfg(struct event *thread)
 						 txn_req->req.set_cfg->cfg_changes,
 						 (size_t)txn_req->req.set_cfg
 							 ->num_cfg_changes,
-						 NULL, NULL, 0, err_buf,
-						 sizeof(err_buf), &error);
+						 NULL, err_buf, sizeof(err_buf),
+						 &error);
 		if (error) {
 			mgmt_fe_send_set_cfg_reply(txn->session_id, txn->txn_id,
 						   txn_req->req.set_cfg->ds_id,

diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c
@@ -157,7 +157,7 @@ DEFPY(mgmt_set_config_data, mgmt_set_config_data_cmd,
 	vty->cfg_changes[0].operation = NB_OP_CREATE;
 	vty->num_cfg_changes = 1;
 
-	vty_mgmt_send_config_data(vty, false);
+	vty_mgmt_send_config_data(vty, NULL, false);
 	return CMD_SUCCESS;
 }
 
@@ -174,7 +174,7 @@ DEFPY(mgmt_delete_config_data, mgmt_delete_config_data_cmd,
 	vty->cfg_changes[0].operation = NB_OP_DESTROY;
 	vty->num_cfg_changes = 1;
 
-	vty_mgmt_send_config_data(vty, false);
+	vty_mgmt_send_config_data(vty, NULL, false);
 	return CMD_SUCCESS;
 }