lib: better conditionalize leaf-list predicate xpath addition

If we're in the backend we already have the predicate added by mgmtd -- don't add it again. Signed-off-by: Christian Hopps <[email protected]>
FRRouting · Jan 19, 2024 · ee3ce27 · ee3ce27
1 parent 1e9306d
commit ee3ce27
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 23 deletions.
diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c
@@ -506,7 +506,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, err_buf, sizeof(err_buf), &error);
+				NULL, true, 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
@@ -685,10 +685,9 @@ static int dnode_create(struct nb_config *candidate, const char *xpath,
 	return NB_OK;
 }
 
-int nb_candidate_edit(struct nb_config *candidate,
-		      const struct nb_node *nb_node,
+int nb_candidate_edit(struct nb_config *candidate, const struct nb_node *nb_node,
 		      enum nb_operation operation, const char *xpath,
-		      const struct yang_data *previous,
+		      bool in_backend, const struct yang_data *previous,
 		      const struct yang_data *data)
 {
 	struct lyd_node *dnode, *dep_dnode, *old_dnode, *parent;
@@ -697,8 +696,13 @@ int nb_candidate_edit(struct nb_config *candidate,
 	uint32_t options = 0;
 	LY_ERR err;
 
-	/* Use special notation for leaf-lists (RFC 6020, section 9.13.5). */
-	if (nb_node->snode->nodetype == LYS_LEAFLIST)
+	/*
+	 * Use special notation for leaf-lists (RFC 6020, section 9.13.5).
+	 * if we are in a backend client this notation was already applied
+	 * by mgmtd before sending to us.
+	 */
+	if (!in_backend && nb_node->snode->nodetype == LYS_LEAFLIST &&
+	    (operation == NB_OP_DESTROY || operation == NB_OP_DELETE))
 		snprintf(xpath_edit, sizeof(xpath_edit), "%s[.='%s']", xpath,
 			 data->value);
 	else
@@ -829,10 +833,12 @@ bool nb_is_operation_allowed(struct nb_node *nb_node, enum nb_operation oper)
 	return true;
 }
 
-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, char *err_buf,
-	int err_bufsize, bool *error)
+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, bool in_backend,
+				      char *err_buf, int err_bufsize,
+				      bool *error)
 {
 	if (error)
 		*error = false;
@@ -886,7 +892,8 @@ void nb_candidate_edit_config_changes(
 		 * configuration.
 		 */
 		ret = nb_candidate_edit(candidate_config, nb_node,
-					change->operation, xpath, NULL, data);
+					change->operation, xpath, in_backend,
+					NULL, data);
 		yang_data_free(data);
 		if (ret != NB_OK) {
 			flog_warn(

diff --git a/lib/northbound.h b/lib/northbound.h
@@ -950,6 +950,9 @@ extern bool nb_is_operation_allowed(struct nb_node *nb_node,
  * xpath
  *    XPath of the configuration node being edited.
  *
+ * in_backend
+ *    Specify whether the changes are being applied in the backend or not.
+ *
  * previous
  *    Previous value of the configuration node. Should be used only when the
  *    operation is NB_OP_MOVE, otherwise this parameter is ignored.
@@ -964,7 +967,7 @@ extern bool nb_is_operation_allowed(struct nb_node *nb_node,
 extern int nb_candidate_edit(struct nb_config *candidate,
 			     const struct nb_node *nb_node,
 			     enum nb_operation operation, const char *xpath,
-			     const struct yang_data *previous,
+			     bool in_backend, const struct yang_data *previous,
 			     const struct yang_data *data);
 
 /*
@@ -1009,6 +1012,9 @@ extern bool nb_candidate_needs_update(const struct nb_config *candidate);
  * xpath_base
  *    Base xpath for config.
  *
+ * in_backend
+ *    Specify whether the changes are being applied in the backend or not.
+ *
  * err_buf
  *    Buffer to store human-readable error message in case of error.
  *
@@ -1018,10 +1024,12 @@ extern bool nb_candidate_needs_update(const struct nb_config *candidate);
  * error
  *    TRUE on error, FALSE on success
  */
-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, char *err_buf,
-	int err_bufsize, bool *error);
+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,
+					     bool in_backend, char *err_buf,
+					     int err_bufsize, bool *error);
 
 /*
  * Delete candidate configuration changes.

diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
@@ -146,9 +146,9 @@ static int nb_cli_apply_changes_internal(struct vty *vty,
 
 	VTY_CHECK_XPATH;
 
-	nb_candidate_edit_config_changes(
-		vty->candidate_config, vty->cfg_changes, vty->num_cfg_changes,
-		xpath_base, buf, sizeof(buf), &error);
+	nb_candidate_edit_config_changes(vty->candidate_config, vty->cfg_changes,
+					 vty->num_cfg_changes, xpath_base,
+					 false, buf, sizeof(buf), &error);
 	if (error) {
 		/*
 		 * Failure to edit the candidate configuration should never

diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c
@@ -256,7 +256,7 @@ frr_confd_cdb_diff_iter(confd_hkeypath_t *kp, enum cdb_iter_op cdb_op,
 	/* Edit the candidate configuration. */
 	data = yang_data_new(xpath, value_str);
 	ret = nb_candidate_edit(iter_args->candidate, nb_node, nb_op, xpath,
-				NULL, data);
+				false, NULL, data);
 	yang_data_free(data);
 	if (ret != NB_OK) {
 		flog_warn(

diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
@@ -219,7 +219,7 @@ static int frr_sr_process_change(struct nb_config *candidate,
 	sr_val_to_buff(sr_data, value_str, sizeof(value_str));
 	data = yang_data_new(xpath, value_str);
 
-	ret = nb_candidate_edit(candidate, nb_node, nb_op, xpath, NULL, data);
+	ret = nb_candidate_edit(candidate, nb_node, nb_op, xpath, false, NULL, data);
 	yang_data_free(data);
 	if (ret != NB_OK) {
 		flog_warn(

diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
@@ -587,8 +587,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, err_buf, sizeof(err_buf),
-						 &error);
+						 NULL, false, 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,