From 408ee24e411714596115d15c831dc59b22dec9f8 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Fri, 13 Oct 2023 22:51:11 -0400 Subject: [PATCH] lib: create and use libyang tree during oper-state walk Signed-off-by: Christian Hopps --- lib/mgmt_be_client.c | 49 +--------- lib/northbound.c | 203 ++++++++++++++++++++++++++++----------- lib/northbound.h | 6 +- lib/northbound_cli.c | 2 +- lib/northbound_sysrepo.c | 2 +- 5 files changed, 159 insertions(+), 103 deletions(-) diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c index dc8005bf95..b0d55d2015 100644 --- a/lib/mgmt_be_client.c +++ b/lib/mgmt_be_client.c @@ -770,45 +770,6 @@ static int mgmt_be_client_handle_msg(struct mgmt_be_client *client_ctx, return 0; } - -static int be_client_oper_data_cb(const struct lysc_node *snode, - struct yang_translator *translator, - struct yang_data *data, void *arg) -{ - struct be_oper_iter_arg *iarg = arg; - struct ly_ctx *ly_ctx = ly_native_ctx; - struct lyd_node *hint = iarg->hint; - struct lyd_node *dnode = NULL; - LY_ERR err; - - if (hint && - (snode == hint->schema || snode->parent == hint->schema->parent)) { - /* This node and the previous node share the same parent, use - * this fact to create the sibling node directly in the tree. - */ - err = lyd_new_term_canon(&hint->parent->node, snode->module, - snode->name, data->value, true, &dnode); - } else if (hint && snode->parent == hint->schema) { - /* This node is a child of the previous added element (e.g., a list) */ - err = lyd_new_term_canon(hint, snode->module, snode->name, - data->value, true, &dnode); - } else { - /* Use the generic xpath parsing create function. This is - * required for creating list entries (along with their child - * key leafs) and other multiple node adding operations. - */ - err = lyd_new_path(iarg->root, ly_ctx, data->xpath, - (void *)data->value, LYD_NEW_PATH_UPDATE, - &dnode); - } - if (err) - flog_warn(EC_LIB_LIBYANG, "%s: failed creating node: %s: %s", - __func__, data->xpath, ly_errmsg(ly_native_ctx)); - iarg->hint = dnode; - yang_data_free(data); - return err ? NB_ERR : NB_OK; -} - /* * Process the get-tree request on our local oper state */ @@ -818,8 +779,7 @@ static void be_client_handle_get_tree(struct mgmt_be_client *client, { struct mgmt_msg_get_tree *get_tree_msg = msgbuf; struct mgmt_msg_tree_data *tree_msg = NULL; - struct be_oper_iter_arg iter_arg = {}; - struct lyd_node *dnode; + struct lyd_node *dnode = NULL; uint8_t *buf = NULL; int ret; @@ -832,13 +792,12 @@ static void be_client_handle_get_tree(struct mgmt_be_client *client, */ /* Obtain data. */ - dnode = yang_dnode_new(ly_native_ctx, false); - iter_arg.root = dnode; ret = nb_oper_data_iterate(get_tree_msg->xpath, NULL, 0, - be_client_oper_data_cb, &iter_arg); + NULL, NULL, &dnode); if (ret != NB_OK) { fail: - yang_dnode_free(dnode); + if (dnode) + yang_dnode_free(dnode); darr_free(buf); be_client_send_error(client, get_tree_msg->txn_id, get_tree_msg->req_id, false, -EINVAL, diff --git a/lib/northbound.c b/lib/northbound.c index e70d05cea9..d55255a046 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -73,9 +73,9 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction, static int nb_oper_data_iter_node(const struct lysc_node *snode, const char *xpath, const void *list_entry, const struct yang_list_keys *list_keys, - struct yang_translator *translator, - bool first, uint32_t flags, - nb_oper_data_cb cb, void *arg); + struct yang_translator *translator, bool first, + uint32_t flags, nb_oper_data_cb cb, void *arg, + struct lyd_node *pdnode); static int nb_node_check_config_only(const struct lysc_node *snode, void *arg) { @@ -1772,7 +1772,8 @@ static int nb_oper_data_iter_children(const struct lysc_node *snode, const struct yang_list_keys *list_keys, struct yang_translator *translator, bool first, uint32_t flags, - nb_oper_data_cb cb, void *arg) + nb_oper_data_cb cb, void *arg, + struct lyd_node *pdnode) { const struct lysc_node *child; @@ -1781,7 +1782,7 @@ static int nb_oper_data_iter_children(const struct lysc_node *snode, ret = nb_oper_data_iter_node(child, xpath, list_entry, list_keys, translator, false, - flags, cb, arg); + flags, cb, arg, pdnode); if (ret != NB_OK) return ret; } @@ -1793,15 +1794,19 @@ static int nb_oper_data_iter_leaf(const struct nb_node *nb_node, const char *xpath, const void *list_entry, const struct yang_list_keys *list_keys, struct yang_translator *translator, - uint32_t flags, nb_oper_data_cb cb, void *arg) + uint32_t flags, nb_oper_data_cb cb, void *arg, + struct lyd_node *pdnode) { + const struct lysc_node *snode = nb_node->snode; struct yang_data *data; + LY_ERR err = LY_SUCCESS; - if (CHECK_FLAG(nb_node->snode->flags, LYS_CONFIG_W)) + + if (CHECK_FLAG(snode->flags, LYS_CONFIG_W)) return NB_OK; /* Ignore list keys. */ - if (lysc_is_key(nb_node->snode)) + if (lysc_is_key(snode)) return NB_OK; data = nb_callback_get_elem(nb_node, xpath, list_entry); @@ -1809,50 +1814,89 @@ static int nb_oper_data_iter_leaf(const struct nb_node *nb_node, /* Leaf of type "empty" is not present. */ return NB_OK; - return (*cb)(nb_node->snode, translator, data, arg); + /* + * Add a dnode to our tree + */ + err = lyd_new_term(pdnode, snode->module, snode->name, data->value, + false, NULL); + if (err) + return NB_ERR_RESOURCE; + + if (cb) + return (*cb)(nb_node->snode, translator, data, arg); + return NB_OK; } static int nb_oper_data_iter_container(const struct nb_node *nb_node, - const char *xpath, + const char *xpath, bool first, const void *list_entry, const struct yang_list_keys *list_keys, struct yang_translator *translator, uint32_t flags, nb_oper_data_cb cb, - void *arg) + void *arg, struct lyd_node *pdnode) { const struct lysc_node *snode = nb_node->snode; + struct lyd_node *cnode = NULL; + bool presence = false; + LY_ERR err; + int ret; if (CHECK_FLAG(nb_node->flags, F_NB_NODE_CONFIG_ONLY)) return NB_OK; + if (pdnode->schema == snode) + assert(first); + else + assert(!first); + /* Read-only presence containers. */ if (nb_node->cbs.get_elem) { struct yang_data *data; int ret; + presence = true; data = nb_callback_get_elem(nb_node, xpath, list_entry); if (data == NULL) /* Presence container is not present. */ return NB_OK; - ret = (*cb)(snode, translator, data, arg); - if (ret != NB_OK) - return ret; - } + if (!first) { + err = lyd_new_inner(pdnode, snode->module, snode->name, + false, &cnode); + if (err) + return NB_ERR_RESOURCE; + } - /* Read-write presence containers. */ - if (CHECK_FLAG(snode->flags, LYS_CONFIG_W)) { - struct lysc_node_container *scontainer; + if (cb) { + ret = (*cb)(snode, translator, data, arg); + if (ret != NB_OK) + return ret; + } + } - scontainer = (struct lysc_node_container *)snode; - if (CHECK_FLAG(scontainer->flags, LYS_PRESENCE) - && !yang_dnode_get(running_config->dnode, xpath)) - return NB_OK; + if (first) + cnode = pdnode; + else if (!cnode) { + /* Add a node in for this container in-case we have children. */ + err = lyd_new_inner(pdnode, snode->module, snode->name, false, + &cnode); + if (err) + return NB_ERR_RESOURCE; } /* Iterate over the child nodes. */ - return nb_oper_data_iter_children(snode, xpath, list_entry, list_keys, - translator, false, flags, cb, arg); + ret = nb_oper_data_iter_children(snode, xpath, list_entry, list_keys, + translator, false, flags, cb, arg, + cnode); + + /* TODO: here we are freeing only if we created; however, we may want to + * also free if pdnode was cnode on entry to cleanup the data tree + */ + /* If we aren't presence container and we gained no children remove */ + if (!presence && !first && !lyd_child(cnode)) + lyd_free_tree(cnode); + + return ret; } static int @@ -1860,11 +1904,14 @@ nb_oper_data_iter_leaflist(const struct nb_node *nb_node, const char *xpath, const void *parent_list_entry, const struct yang_list_keys *parent_list_keys, struct yang_translator *translator, uint32_t flags, - nb_oper_data_cb cb, void *arg) + nb_oper_data_cb cb, void *arg, + struct lyd_node *pdnode) { + const struct lysc_node *snode = nb_node->snode; const void *list_entry = NULL; + LY_ERR err; - if (CHECK_FLAG(nb_node->snode->flags, LYS_CONFIG_W)) + if (CHECK_FLAG(snode->flags, LYS_CONFIG_W)) return NB_OK; do { @@ -1881,9 +1928,19 @@ nb_oper_data_iter_leaflist(const struct nb_node *nb_node, const char *xpath, if (data == NULL) continue; - ret = (*cb)(nb_node->snode, translator, data, arg); - if (ret != NB_OK) - return ret; + /* + * Add a dnode to our tree + */ + err = lyd_new_term(pdnode, snode->module, snode->name, + data->value, false, NULL); + if (err) + return NB_ERR_RESOURCE; + + if (cb) { + ret = (*cb)(nb_node->snode, translator, data, arg); + if (ret != NB_OK) + return ret; + } } while (list_entry); return NB_OK; @@ -1894,11 +1951,16 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node, const void *parent_list_entry, const struct yang_list_keys *parent_list_keys, struct yang_translator *translator, - uint32_t flags, nb_oper_data_cb cb, void *arg) + uint32_t flags, nb_oper_data_cb cb, void *arg, + struct lyd_node *pdnode) { + char xpath[XPATH_MAXLEN * 2]; const struct lysc_node *snode = nb_node->snode; const void *list_entry = NULL; + struct lyd_node *list_node = NULL; + const char *key_preds = NULL; uint32_t position = 1; + LY_ERR err; if (CHECK_FLAG(nb_node->flags, F_NB_NODE_CONFIG_ONLY)) return NB_OK; @@ -1907,8 +1969,7 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node, do { const struct lysc_node_leaf *skey; struct yang_list_keys list_keys = {}; - char xpath[XPATH_MAXLEN * 2]; - int ret; + int len, len2, ret; /* Obtain list entry. */ list_entry = nb_callback_get_next(nb_node, parent_list_entry, @@ -1930,16 +1991,23 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node, /* Build XPath of the list entry. */ strlcpy(xpath, xpath_list, sizeof(xpath)); + len = strlen(xpath); + key_preds = &xpath[len]; + unsigned int i = 0; LY_FOR_KEYS (snode, skey) { assert(i < list_keys.num); - snprintf(xpath + strlen(xpath), - sizeof(xpath) - strlen(xpath), - "[%s='%s']", skey->name, - list_keys.key[i]); + len2 = snprintf(xpath + len, sizeof(xpath) - len, + "[%s='%s']", skey->name, + list_keys.key[i]); + if (len2 > (ssize_t)sizeof(xpath) - len) + len = sizeof(xpath); + else + len += len2; i++; } assert(i == list_keys.num); + } else { /* * Keyless list - build XPath using a positional index. @@ -1949,10 +2017,20 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node, position++; } + /* + * `pdnode` needs to point at lib - and it does for + * "/frr-vrf:lib/vrf" need to test "/frr-vrf:lib" too though + */ + err = lyd_new_list2(pdnode, snode->module, snode->name, + key_preds, false, &list_node); + if (err) + return NB_ERR_RESOURCE; + /* Iterate over the child nodes. */ - ret = nb_oper_data_iter_children( - nb_node->snode, xpath, list_entry, &list_keys, - translator, false, flags, cb, arg); + ret = nb_oper_data_iter_children(nb_node->snode, xpath, + list_entry, &list_keys, + translator, false, flags, cb, + arg, list_node); if (ret != NB_OK) return ret; } while (list_entry); @@ -1964,9 +2042,9 @@ static int nb_oper_data_iter_node(const struct lysc_node *snode, const char *xpath_parent, const void *list_entry, const struct yang_list_keys *list_keys, - struct yang_translator *translator, - bool first, uint32_t flags, - nb_oper_data_cb cb, void *arg) + struct yang_translator *translator, bool first, + uint32_t flags, nb_oper_data_cb cb, void *arg, + struct lyd_node *pdnode) { struct nb_node *nb_node; char xpath[XPATH_MAXLEN]; @@ -1976,6 +2054,10 @@ static int nb_oper_data_iter_node(const struct lysc_node *snode, && CHECK_FLAG(snode->nodetype, LYS_CONTAINER | LYS_LIST)) return NB_OK; + /* + * would be nice to just be building a libyang data tree here as well + */ + /* Update XPath. */ strlcpy(xpath, xpath_parent, sizeof(xpath)); if (!first && snode->nodetype != LYS_USES) { @@ -2001,29 +2083,31 @@ static int nb_oper_data_iter_node(const struct lysc_node *snode, nb_node = snode->priv; switch (snode->nodetype) { case LYS_CONTAINER: - ret = nb_oper_data_iter_container(nb_node, xpath, list_entry, - list_keys, translator, flags, - cb, arg); + ret = nb_oper_data_iter_container(nb_node, xpath, first, + list_entry, list_keys, + translator, flags, cb, arg, + pdnode); + break; case LYS_LEAF: ret = nb_oper_data_iter_leaf(nb_node, xpath, list_entry, list_keys, translator, flags, cb, - arg); + arg, pdnode); break; case LYS_LEAFLIST: ret = nb_oper_data_iter_leaflist(nb_node, xpath, list_entry, list_keys, translator, flags, - cb, arg); + cb, arg, pdnode); break; case LYS_LIST: ret = nb_oper_data_iter_list(nb_node, xpath, list_entry, list_keys, translator, flags, cb, - arg); + arg, pdnode); break; case LYS_USES: ret = nb_oper_data_iter_children(snode, xpath, list_entry, list_keys, translator, false, - flags, cb, arg); + flags, cb, arg, pdnode); break; default: break; @@ -2089,7 +2173,8 @@ done: int nb_oper_data_iterate(const char *xpath, struct yang_translator *translator, - uint32_t flags, nb_oper_data_cb cb, void *arg) + uint32_t flags, nb_oper_data_cb cb, void *arg, + struct lyd_node **tree) { struct nb_node *nb_node; const void *list_entry = NULL; @@ -2198,16 +2283,24 @@ int nb_oper_data_iterate(const char *xpath, struct yang_translator *translator, */ if (dnode->schema->nodetype == LYS_LIST && lyd_child(dnode) && dnode->schema == nb_node->snode) - ret = nb_oper_data_iter_children( - nb_node->snode, xpath, list_entry, &list_keys, - translator, true, flags, cb, arg); + ret = nb_oper_data_iter_children(nb_node->snode, xpath, + list_entry, &list_keys, + translator, true, flags, cb, + arg, dnode); else ret = nb_oper_data_iter_node(nb_node->snode, xpath, list_entry, &list_keys, translator, true, - flags, cb, arg); + flags, cb, arg, dnode); list_delete(&list_dnodes); - yang_dnode_free(dnode); + if (dnode) { + while (lyd_parent(dnode)) + dnode = lyd_parent(dnode); + if (!tree) + lyd_free_all(dnode); + else + *tree = dnode; + } return ret; } diff --git a/lib/northbound.h b/lib/northbound.h index 9c0b4d16c3..2a338a082c 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -1268,12 +1268,16 @@ extern int nb_running_lock_check(enum nb_client client, const void *user); * arg * Arbitrary argument passed as the fourth parameter in each call to 'cb'. * + * tree + * If non-NULL will contain the data tree built from the walk. + * * Returns: * NB_OK on success, NB_ERR otherwise. */ extern int nb_oper_data_iterate(const char *xpath, struct yang_translator *translator, - uint32_t flags, nb_oper_data_cb cb, void *arg); + uint32_t flags, nb_oper_data_cb cb, void *arg, + struct lyd_node **tree); /* * Validate if the northbound operation is valid for the given node. diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index f2415d3383..cba40035a3 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -1492,7 +1492,7 @@ DEFPY (show_yang_operational_data, /* Obtain data. */ dnode = yang_dnode_new(ly_ctx, false); ret = nb_oper_data_iterate(xpath, translator, 0, nb_cli_oper_data_cb, - dnode); + dnode, NULL); if (ret != NB_OK) { if (format == LYD_JSON) vty_out(vty, "{}\n"); diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c index 7fd4af8356..ea5aa22b14 100644 --- a/lib/northbound_sysrepo.c +++ b/lib/northbound_sysrepo.c @@ -378,7 +378,7 @@ static int frr_sr_state_cb(sr_session_ctx_t *session, uint32_t sub_id, dnode = *parent; if (nb_oper_data_iterate(request_xpath, NULL, 0, - frr_sr_state_data_iter_cb, dnode) + frr_sr_state_data_iter_cb, dnode, NULL) != NB_OK) { flog_warn(EC_LIB_NB_OPERATIONAL_DATA, "%s: failed to obtain operational data [xpath %s]", -- 2.39.5