From b8a2efbf2f062dd5dc6ab0c660b3cba3f2f469f5 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Sat, 11 Nov 2023 02:13:17 +0200 Subject: [PATCH] lib, mgmtd: respect base xpath in mgmtd `nb_cli_apply_changes` can be called with base xpath which should be prepended to xpaths of every change in a transaction. This base xpath is respected by regular northbound CLI but not by mgmtd. This commit fixes the problem. Signed-off-by: Igor Ryzhov --- lib/mgmt_be_client.c | 3 +-- lib/northbound.c | 23 ++++++++--------------- lib/northbound.h | 10 ++-------- lib/northbound_cli.c | 38 ++++++++++++++++++++++++++++++++------ lib/vty.c | 20 ++++++++++++++++++-- lib/vty.h | 3 ++- mgmtd/mgmt_txn.c | 4 ++-- mgmtd/mgmt_vty.c | 4 ++-- 8 files changed, 67 insertions(+), 38 deletions(-) diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c index 762ace136179..a7adcc455d24 100644 --- 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 index 080f3d002eca..b0380e6e7e31 100644 --- 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,25 +776,18 @@ 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]; 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 index 5cf6e85b4bd3..9c0b4d16c355 100644 --- 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 index 8003679ed57b..f2415d3383ca 100644 --- 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,6 +212,9 @@ 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; @@ -202,18 +222,20 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) 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 index f395fd3ea1d8..2cfe34f211dc 100644 --- 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 index 346e44910a01..1a431fa16a43 100644 --- 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 index 765def7bacea..1af0f2749afa 100644 --- 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 index e7d69667103b..54288dfe9c89 100644 --- 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; }