From 00138ffb47acc58a49e93a9b291a4b9e0c92096e Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 7 Jan 2024 09:41:32 +0000 Subject: [PATCH 1/4] lib: fix clang SA warnings Signed-off-by: Christian Hopps --- lib/darr.h | 23 ++++++++++++++++------- lib/northbound_oper.c | 13 ++++++++++++- mgmtd/mgmt_fe_adapter.c | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/lib/darr.h b/lib/darr.h index 2b6f0db0b9ce..df8ace62dd15 100644 --- a/lib/darr.h +++ b/lib/darr.h @@ -77,6 +77,7 @@ */ #include +#include #include "memory.h" DECLARE_MTYPE(DARR); @@ -249,6 +250,10 @@ void *__darr_resize(void *a, uint count, size_t esize, struct memtype *mt); * pointers into the previous memory block are no longer valid. The `A` value * is guaranteed not to change if there is sufficient capacity in the array. * + * The exception to the no-change rule is if @C is passed as 0, it will be + * considered 1 so that an array is always allocated if currently NULL, + * i.e., @A will never be NULL after a call to darr_ensure_cap_mt() + * * Args: * A: (IN/OUT) the dynamic array, can be NULL. * C: Total capacity to guarantee. @@ -259,8 +264,9 @@ void *__darr_resize(void *a, uint count, size_t esize, struct memtype *mt); #define darr_ensure_cap_mt(A, C, MT) \ ({ \ /* Cast to avoid warning when C == 0 */ \ - if ((ssize_t)darr_cap(A) < (ssize_t)(C)) \ - _darr_resize_mt((A), (C), MT); \ + uint _c = (C) > 0 ? (C) : 1; \ + if ((size_t)darr_cap(A) < _c) \ + _darr_resize_mt((A), _c, MT); \ (A); \ }) #define darr_ensure_cap(A, C) darr_ensure_cap_mt(A, C, MTYPE_DARR) @@ -285,11 +291,14 @@ void *__darr_resize(void *a, uint count, size_t esize, struct memtype *mt); */ #define darr_ensure_i_mt(A, I, MT) \ ({ \ - if ((int)(I) > darr_maxi(A)) \ - _darr_resize_mt((A), (I) + 1, MT); \ - if ((I) + 1 > _darr_len(A)) \ - _darr_len(A) = (I) + 1; \ - &(A)[I]; \ + assert((int)(I) >= 0 && (int)(I) <= INT_MAX); \ + int _i = (int)(I); \ + if (_i > darr_maxi(A)) \ + _darr_resize_mt((A), _i + 1, MT); \ + assert((A) != NULL); \ + if ((uint)_i + 1 > _darr_len(A)) \ + _darr_len(A) = _i + 1; \ + &(A)[_i]; \ }) #define darr_ensure_i(A, I) darr_ensure_i_mt(A, I, MTYPE_DARR) diff --git a/lib/northbound_oper.c b/lib/northbound_oper.c index 334370d0aba0..bd6d870ebcaf 100644 --- a/lib/northbound_oper.c +++ b/lib/northbound_oper.c @@ -806,6 +806,13 @@ static const struct lysc_node *nb_op_sib_first(struct nb_op_yield_state *ys, const struct lysc_node *sib = lysc_node_child(parent); const struct lysc_node *first_sib; + /* + * NOTE: when we want to handle root level walks we will need to use + * lys_getnext() to walk root level of each module and + * ly_ctx_get_module_iter() to walk the modules. + */ + assert(darr_len(ys->node_infos) > 0); + /* * The top of the node stack points at @parent. * @@ -814,7 +821,7 @@ static const struct lysc_node *nb_op_sib_first(struct nb_op_yield_state *ys, * base of the user query, return the next schema node from the query * string (schema_path). */ - assert(darr_last(ys->node_infos)->schema == parent); + assert(darr_last(ys->node_infos) != NULL && darr_last(ys->node_infos)->schema == parent); if (darr_lasti(ys->node_infos) < ys->query_base_level) return ys->schema_path[darr_lasti(ys->node_infos) + 1]; @@ -1010,10 +1017,14 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume) * should be kept. */ ret = nb_op_iter_leaf(ys, nn, xpath_child); + if (ret != NB_OK) + goto done; sib = nb_op_sib_next(ys, sib); continue; case LYS_LEAFLIST: ret = nb_op_iter_leaflist(ys, nn, xpath_child); + if (ret != NB_OK) + goto done; sib = nb_op_sib_next(ys, sib); continue; case LYS_CONTAINER: diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c index d98444703f21..5f17b89c5c61 100644 --- a/mgmtd/mgmt_fe_adapter.c +++ b/mgmtd/mgmt_fe_adapter.c @@ -1105,7 +1105,7 @@ static int fe_adapter_send_tree_data(struct mgmt_fe_session_ctx *session, LYD_PRINT_WITHSIBLINGS)); /* buf may have been reallocated and moved */ msg = (typeof(msg))buf; - + (void)msg; /* suppress clang-SA unused warning on safety code */ if (ret != LY_SUCCESS) { MGMTD_FE_ADAPTER_ERR("Error building get-tree result for client %s session-id %" PRIu64 From cf67a7e26577b0dda276324b40a602ae084e504e Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sat, 6 Jan 2024 09:45:29 +0000 Subject: [PATCH 2/4] lib: mgmtd: implement full XPath 1.0 predicate functionality Allow user to specify full YANG compatible XPath 1.0 predicates. This allows for trimming results of generic queries using functions and other non-key predicates from XPath 1.0 Signed-off-by: Christian Hopps --- lib/mgmt_msg.c | 2 +- lib/northbound.c | 21 ++++++++ lib/northbound.h | 8 +++ lib/northbound_oper.c | 24 +++++++-- lib/yang.c | 105 ++++++++++++++++++++++++++++++++++++++++ lib/yang.h | 29 +++++++++++ mgmtd/mgmt_fe_adapter.c | 43 +++++++++++++--- mgmtd/mgmt_txn.c | 31 ++++++++---- mgmtd/mgmt_txn.h | 4 +- 9 files changed, 245 insertions(+), 22 deletions(-) diff --git a/lib/mgmt_msg.c b/lib/mgmt_msg.c index 99d000537ce9..782707b46314 100644 --- a/lib/mgmt_msg.c +++ b/lib/mgmt_msg.c @@ -207,7 +207,7 @@ bool mgmt_msg_procbufs(struct mgmt_msg_state *ms, } /** - * Write data from a onto the socket, using streams that have been queued for + * Write data onto the socket, using streams that have been queued for * sending by mgmt_msg_send_msg. This function should be reschedulable. * * Args: diff --git a/lib/northbound.c b/lib/northbound.c index 18d65e47f13e..03d252ee5210 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -6,6 +6,7 @@ #include +#include "darr.h" #include "libfrr.h" #include "log.h" #include "lib_errors.h" @@ -168,6 +169,26 @@ struct nb_node *nb_node_find(const char *path) return snode->priv; } +struct nb_node **nb_nodes_find(const char *xpath) +{ + struct lysc_node **snodes = NULL; + struct nb_node **nb_nodes = NULL; + bool simple; + LY_ERR err; + uint i; + + err = yang_resolve_snode_xpath(ly_native_ctx, xpath, &snodes, &simple); + if (err) + return NULL; + + darr_ensure_i(nb_nodes, darr_lasti(snodes)); + darr_foreach_i (snodes, i) + nb_nodes[i] = snodes[i]->priv; + darr_free(snodes); + return nb_nodes; +} + + void nb_node_set_dependency_cbs(const char *dependency_xpath, const char *dependant_xpath, struct nb_dependency_callbacks *cbs) diff --git a/lib/northbound.h b/lib/northbound.h index 018d09fac716..37b7055c10df 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -813,6 +813,14 @@ void nb_nodes_delete(void); */ extern struct nb_node *nb_node_find(const char *xpath); +/** + * nb_nodes_find() - find the NB nodes corresponding to complex xpath. + * @xpath: XPath to search for (with or without predicates). + * + * Return: a dynamic array (darr) of `struct nb_node *`s. + */ +extern struct nb_node **nb_nodes_find(const char *xpath); + extern void nb_node_set_dependency_cbs(const char *dependency_xpath, const char *dependant_xpath, struct nb_dependency_callbacks *cbs); diff --git a/lib/northbound_oper.c b/lib/northbound_oper.c index bd6d870ebcaf..4e131154e779 100644 --- a/lib/northbound_oper.c +++ b/lib/northbound_oper.c @@ -72,6 +72,7 @@ struct nb_op_node_info { * @schema_path: the schema nodes for each node in the query string. # @query_tokstr: the query string tokenized with NUL bytes. * @query_tokens: the string pointers to each query token (node). + * @non_specific_predicate: tracks if a query_token is non-specific predicate. * @walk_root_level: The topmost specific node, +1 is where we start walking. * @walk_start_level: @walk_root_level + 1. * @query_base_level: the level the query string stops at and full walks @@ -85,6 +86,7 @@ struct nb_op_yield_state { const struct lysc_node **schema_path; char *query_tokstr; char **query_tokens; + uint8_t *non_specific_predicate; int walk_root_level; int walk_start_level; int query_base_level; @@ -158,6 +160,7 @@ static inline void nb_op_free_yield_state(struct nb_op_yield_state *ys, if (!nofree_tree && ys_root_node(ys)) lyd_free_all(ys_root_node(ys)); darr_free(ys->query_tokens); + darr_free(ys->non_specific_predicate); darr_free(ys->query_tokstr); darr_free(ys->schema_path); darr_free(ys->node_infos); @@ -1142,18 +1145,23 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume) is_specific_node = false; if (list_start && at_clevel <= darr_lasti(ys->query_tokens) && + !ys->non_specific_predicate[at_clevel] && nb_op_schema_path_has_predicate(ys, at_clevel)) { err = lyd_new_path(&pni->inner->node, NULL, ys->query_tokens[at_clevel], NULL, 0, &node); if (!err) - /* predicate resolved to specific node */ is_specific_node = true; + else if (err == LY_EVALID) + ys->non_specific_predicate[at_clevel] = true; else { - flog_warn(EC_LIB_NB_OPERATIONAL_DATA, - "%s: unable to create node for specific query string: %s", + flog_err(EC_LIB_NB_OPERATIONAL_DATA, + "%s: unable to create node for specific query string: %s: %s", __func__, - ys->query_tokens[at_clevel]); + ys->query_tokens[at_clevel], + yang_ly_strerrcode(err)); + ret = NB_ERR; + goto done; } } @@ -1570,6 +1578,7 @@ static enum nb_error nb_op_yield(struct nb_op_yield_state *ys) static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys, struct nb_node **last) { + struct nb_node **nb_nodes = NULL; const struct lysc_node *sn; struct nb_node *nblast; char *s, *s2; @@ -1587,6 +1596,11 @@ static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys, * string over each schema trunk in the set. */ nblast = nb_node_find(ys->xpath); + if (!nblast) { + nb_nodes = nb_nodes_find(ys->xpath); + nblast = darr_len(nb_nodes) ? nb_nodes[0] : NULL; + darr_free(nb_nodes); + } if (!nblast) { flog_warn(EC_LIB_YANG_UNKNOWN_DATA_PATH, "%s: unknown data path: %s", __func__, ys->xpath); @@ -1614,6 +1628,7 @@ static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys, /* create our arrays */ darr_append_n(ys->schema_path, count); darr_append_n(ys->query_tokens, count); + darr_append_nz(ys->non_specific_predicate, count); for (sn = nblast->snode; sn; sn = sn->parent) ys->schema_path[--count] = sn; @@ -1675,6 +1690,7 @@ static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys, darr_free(ys->query_tokstr); darr_free(ys->schema_path); darr_free(ys->query_tokens); + darr_free(ys->non_specific_predicate); return NB_ERR; } diff --git a/lib/yang.c b/lib/yang.c index 18d2ac58d347..b2cc71b30918 100644 --- a/lib/yang.c +++ b/lib/yang.c @@ -251,6 +251,38 @@ void yang_snode_get_path(const struct lysc_node *snode, } } +LY_ERR yang_resolve_snode_xpath(struct ly_ctx *ly_ctx, const char *xpath, + struct lysc_node ***snodes, bool *simple) +{ + struct lysc_node *snode; + struct ly_set *set; + LY_ERR err; + + /* lys_find_path will not resolve complex xpaths */ + snode = (struct lysc_node *)lys_find_path(ly_ctx, NULL, xpath, 0); + if (snode) { + *darr_append(*snodes) = snode; + *simple = true; + return LY_SUCCESS; + } + + /* Try again to catch complex query cases */ + err = lys_find_xpath(ly_native_ctx, NULL, xpath, 0, &set); + if (err) + return err; + if (!set->count) { + ly_set_free(set, NULL); + return LY_ENOTFOUND; + } + + *simple = false; + darr_ensure_i(*snodes, set->count - 1); + memcpy(*snodes, set->snodes, set->count * sizeof(set->snodes[0])); + ly_set_free(set, NULL); + return LY_SUCCESS; +} + + struct lysc_node *yang_find_snode(struct ly_ctx *ly_ctx, const char *xpath, uint32_t options) { @@ -1019,3 +1051,76 @@ LY_ERR yang_lyd_new_list(struct lyd_node_inner *parent, /*NOTREACHED*/ return LY_EINVAL; } + + +int yang_trim_tree(struct lyd_node *root, const char *xpath) +{ + enum nb_error ret = NB_OK; + LY_ERR err; +#if 0 + err = lyd_trim_xpath(&root, xpath, NULL); + if (err) { + flog_err_sys(EC_LIB_LIBYANG, + "cannot obtain specific result for xpath \"%s\"", + xpath); + return NB_ERR; + } + return NB_OK; +#else + struct lyd_node *node; + struct lyd_node **remove = NULL; + struct ly_set *set = NULL; + uint32_t i; + + err = lyd_find_xpath3(NULL, root, xpath, NULL, &set); + if (err) { + flog_err_sys(EC_LIB_LIBYANG, + "cannot obtain specific result for xpath \"%s\"", + xpath); + ret = NB_ERR; + goto done; + } + /* + * Mark keepers and sweep deleting non-keepers. + * + * NOTE: We assume the data-nodes have NULL priv pointers and use that + * for our mark. + */ + + /* Mark */ + for (i = 0; i < set->count; i++) { + for (node = set->dnodes[i]; node; node = &node->parent->node) { + if (node->priv) + break; + if (node == set->dnodes[i]) + node->priv = (void *)2; + else + node->priv = (void *)1; + } + } + + darr_ensure_cap(remove, 128); + LYD_TREE_DFS_BEGIN (root, node) { + /* + * If this is a direct matching node then include it's subtree + * which won't be marked and would otherwise be removed. + */ + if (node->priv == (void *)2) + LYD_TREE_DFS_continue = 1; + else if (!node->priv) { + LYD_TREE_DFS_continue = 1; + *darr_append(remove) = node; + } + LYD_TREE_DFS_END(root, node); + } + darr_foreach_i (remove, i) + lyd_free_tree(remove[i]); + darr_free(remove); + +done: + if (set) + ly_set_free(set, NULL); + + return ret; +#endif +} diff --git a/lib/yang.h b/lib/yang.h index 3ce584b347c3..75dcab2d2af8 100644 --- a/lib/yang.h +++ b/lib/yang.h @@ -724,6 +724,35 @@ extern LY_ERR yang_lyd_new_list(struct lyd_node_inner *parent, const struct lysc_node *snode, const struct yang_list_keys *keys, struct lyd_node_inner **node); +/** + * yang_resolve_snodes() - Resolve an XPath to matching schema nodes. + * @ly_ctx: libyang context to operate on. + * @xpath: the path or XPath to resolve. + * @snodes: [OUT] pointer for resulting dynamic array (darr) of schema node + * pointers. + * @simple: [OUT] indicates if @xpath was resolvable simply or not. Non-simple + * means that the @xpath is not a simple path and utilizes XPath 1.0 + * functionality beyond simple key predicates. + * + * This function can be used to find the schema node (or nodes) that correspond + * to a given @xpath. If the @xpath includes non-key predicates (e.g., using + * functions) then @simple will be set to false, and @snodes may contain more + * than a single schema node. + * + * Return: a libyang error or LY_SUCCESS. + */ +extern LY_ERR yang_resolve_snode_xpath(struct ly_ctx *ly_ctx, const char *xpath, + struct lysc_node ***snodes, bool *simple); + +/** + * yang_trim_tree() - trim the data tree to the given xpath + * @root: the data tree + * @xpath: the xpath to trim @root to. + * + * Return: enum nb_error.. + */ +extern int yang_trim_tree(struct lyd_node *root, const char *xpath); + #ifdef __cplusplus } #endif diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c index 5f17b89c5c61..b7c7a0fff147 100644 --- a/mgmtd/mgmt_fe_adapter.c +++ b/mgmtd/mgmt_fe_adapter.c @@ -1132,14 +1132,21 @@ static int fe_adapter_send_tree_data(struct mgmt_fe_session_ctx *session, } /** - * Handle a get-tree message from the client. + * fe_adapter_handle_get_tree() - Handle a get-tree message from a FE client. + * @session: the client session. + * @msg_raw: the message data. + * @msg_len: the length of the message data. */ static void fe_adapter_handle_get_tree(struct mgmt_fe_session_ctx *session, - void *data, size_t len) + void *__msg, size_t msg_len) { - struct mgmt_msg_get_tree *msg = data; + struct mgmt_msg_get_tree *msg = __msg; + struct lysc_node **snodes = NULL; + char *xpath_resolved = NULL; uint64_t req_id = msg->req_id; uint64_t clients; + bool simple_xpath; + LY_ERR err; int ret; MGMTD_FE_ADAPTER_DBG("Received get-tree request from client %s for session-id %" PRIu64 @@ -1147,14 +1154,32 @@ static void fe_adapter_handle_get_tree(struct mgmt_fe_session_ctx *session, session->adapter->name, session->session_id, msg->req_id); + if (!MGMT_MSG_VALIDATE_NUL_TERM(msg, msg_len)) { + fe_adapter_send_error(session, req_id, false, -EINVAL, + "Invalid message rcvd from session-id: %" PRIu64, + session->session_id); + goto done; + } + if (session->txn_id != MGMTD_TXN_ID_NONE) { fe_adapter_send_error(session, req_id, false, -EINPROGRESS, "Transaction in progress txn-id: %" PRIu64 " for session-id: %" PRIu64, session->txn_id, session->session_id); - return; + goto done; } + + err = yang_resolve_snode_xpath(ly_native_ctx, msg->xpath, &snodes, + &simple_xpath); + if (err) { + fe_adapter_send_error(session, req_id, false, -EINPROGRESS, + "XPath doesn't resolve for session-id: %" PRIu64, + session->session_id); + goto done; + } + darr_free(snodes); + clients = mgmt_be_interested_clients(msg->xpath, false); if (!clients) { MGMTD_FE_ADAPTER_DBG("No backends provide xpath: %s for txn-id: %" PRIu64 @@ -1164,7 +1189,7 @@ static void fe_adapter_handle_get_tree(struct mgmt_fe_session_ctx *session, fe_adapter_send_tree_data(session, req_id, false, msg->result_type, NULL, 0); - return; + goto done; } /* Start a SHOW Transaction */ @@ -1173,7 +1198,7 @@ static void fe_adapter_handle_get_tree(struct mgmt_fe_session_ctx *session, if (session->txn_id == MGMTD_SESSION_ID_NONE) { fe_adapter_send_error(session, req_id, false, -EINPROGRESS, "failed to create a 'show' txn"); - return; + goto done; } MGMTD_FE_ADAPTER_DBG("Created new show txn-id: %" PRIu64 @@ -1182,13 +1207,17 @@ static void fe_adapter_handle_get_tree(struct mgmt_fe_session_ctx *session, /* Create a GET-TREE request under the transaction */ ret = mgmt_txn_send_get_tree_oper(session->txn_id, req_id, clients, - msg->result_type, msg->xpath); + msg->result_type, simple_xpath, + msg->xpath); if (ret) { /* destroy the just created txn */ mgmt_destroy_txn(&session->txn_id); fe_adapter_send_error(session, req_id, false, -EINPROGRESS, "failed to create a 'show' txn"); } +done: + darr_free(snodes); + darr_free(xpath_resolved); } /** diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index ea3236e80b96..1c6444563144 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -173,10 +173,11 @@ struct mgmt_get_data_req { struct txn_req_get_tree { char *xpath; /* xpath of tree to get */ - uint8_t result_type; /* LYD_FORMAT for results */ uint64_t sent_clients; /* Bitmask of clients sent req to */ uint64_t recv_clients; /* Bitmask of clients recv reply from */ int32_t partial_error; /* an error while gather results */ + uint8_t result_type; /* LYD_FORMAT for results */ + uint8_t simple_xpath; /* if xpath is simple */ struct lyd_node *client_results; /* result tree from clients */ }; @@ -1268,22 +1269,33 @@ static int txn_get_tree_data_done(struct mgmt_txn_ctx *txn, { struct txn_req_get_tree *get_tree = txn_req->req.get_tree; uint64_t req_id = txn_req->req_id; - int ret = 0; + int ret = NB_OK; /* cancel timer and send reply onward */ EVENT_OFF(txn->get_tree_timeout); - ret = mgmt_fe_adapter_send_tree_data(txn->session_id, txn->txn_id, - txn_req->req_id, - get_tree->result_type, - get_tree->client_results, - get_tree->partial_error, false); + if (!get_tree->simple_xpath && get_tree->client_results) { + /* + * We have a complex query so Filter results by the xpath query. + */ + ret = yang_trim_tree(get_tree->client_results, + txn_req->req.get_tree->xpath); + } + + if (ret == NB_OK) + ret = mgmt_fe_adapter_send_tree_data(txn->session_id, + txn->txn_id, + txn_req->req_id, + get_tree->result_type, + get_tree->client_results, + get_tree->partial_error, + false); /* we're done with the request */ mgmt_txn_req_free(&txn_req); if (ret) { - MGMTD_TXN_ERR("Error saving the results of GETTREE for txn-id %" PRIu64 + MGMTD_TXN_ERR("Error sending the results of GETTREE for txn-id %" PRIu64 " req_id %" PRIu64 " to requested type %u", txn->txn_id, req_id, get_tree->result_type); @@ -2352,7 +2364,7 @@ int mgmt_txn_send_get_req(uint64_t txn_id, uint64_t req_id, */ int mgmt_txn_send_get_tree_oper(uint64_t txn_id, uint64_t req_id, uint64_t clients, LYD_FORMAT result_type, - const char *xpath) + bool simple_xpath, const char *xpath) { struct mgmt_msg_get_tree *msg; struct mgmt_txn_ctx *txn; @@ -2370,6 +2382,7 @@ int mgmt_txn_send_get_tree_oper(uint64_t txn_id, uint64_t req_id, txn_req = mgmt_txn_req_alloc(txn, req_id, MGMTD_TXN_PROC_GETTREE); get_tree = txn_req->req.get_tree; get_tree->result_type = result_type; + get_tree->simple_xpath = simple_xpath; get_tree->xpath = XSTRDUP(MTYPE_MGMTD_XPATH, xpath); msg = mgmt_msg_native_alloc_msg(struct mgmt_msg_get_tree, slen + 1, diff --git a/mgmtd/mgmt_txn.h b/mgmtd/mgmt_txn.h index 4aa067775558..39d8cde16953 100644 --- a/mgmtd/mgmt_txn.h +++ b/mgmtd/mgmt_txn.h @@ -203,13 +203,15 @@ extern int mgmt_txn_send_get_req(uint64_t txn_id, uint64_t req_id, * req_id: FE client request identifier. * clients: Bitmask of clients to send get-tree to. * result_type: LYD_FORMAT result format. + * simple_xpath: true if xpath is simple (only key predicates). * xpath: The xpath to get the tree from. + * * Return: * 0 on success. */ extern int mgmt_txn_send_get_tree_oper(uint64_t txn_id, uint64_t req_id, uint64_t clients, LYD_FORMAT result_type, - const char *xpath); + bool simple_xpath, const char *xpath); /* * Notifiy backend adapter on connection. From 1e4229fc1f843f88e9c9bc6ec4700489a455e6cf Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sat, 6 Jan 2024 11:06:38 +0000 Subject: [PATCH 3/4] lib: use libyang functions if they are present Add configure.ac tests for libyang functions, if not present supply the functionality ourselves in yang.[ch] Signed-off-by: Christian Hopps --- configure.ac | 7 ++ lib/northbound_oper.c | 5 +- lib/vty.c | 85 +------------------- lib/yang.c | 176 +++++++++++++++++++++++++++++++++--------- lib/yang.h | 21 +++-- mgmtd/mgmt_txn.c | 5 +- 6 files changed, 163 insertions(+), 136 deletions(-) diff --git a/configure.ac b/configure.ac index a3b0370ec056..00cf6451fa47 100644 --- a/configure.ac +++ b/configure.ac @@ -1968,6 +1968,13 @@ AC_CHECK_MEMBER([struct lyd_node.priv], [], [ Instructions for this are included in the build documentation for your platform at http://docs.frrouting.org/projects/dev-guide/en/latest/building.html]) ]) ], [[#include ]]) + +AC_CHECK_LIB([yang],[lyd_find_xpath3],[],[AC_MSG_ERROR([m4_normalize([ +libyang missing lyd_find_xpath3])])]) +dnl -- don't add lyd_new_list3 to this list unless bug is fixed upstream +dnl -- https://github.com/CESNET/libyang/issues/2149 +AC_CHECK_FUNCS([ly_strerrcode ly_strvecode lyd_trim_xpath]) + CFLAGS="$ac_cflags_save" dnl --------------- diff --git a/lib/northbound_oper.c b/lib/northbound_oper.c index 4e131154e779..78106e4e4511 100644 --- a/lib/northbound_oper.c +++ b/lib/northbound_oper.c @@ -1409,11 +1409,8 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume) */ if (!node) { - /* NOTE: can also use lyd_new_list2 here when available */ err = yang_lyd_new_list(ni[-1].inner, sib, - &ni->keys, - (struct lyd_node_inner * - *)&node); + &ni->keys, &node); if (err) { darr_pop(ys->node_infos); ret = NB_ERR_RESOURCE; diff --git a/lib/vty.c b/lib/vty.c index a08a3c873590..0ee610e3aa52 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -3688,88 +3688,9 @@ static void vty_out_yang_error(struct vty *vty, LYD_FORMAT format, else if (ei->level == LY_LLWRN) severity = "warning"; - switch (ei->no) { - case LY_SUCCESS: - ecode = "ok"; - break; - case LY_EMEM: - ecode = "out of memory"; - break; - case LY_ESYS: - ecode = "system error"; - break; - case LY_EINVAL: - ecode = "invalid value given"; - break; - case LY_EEXIST: - ecode = "item exists"; - break; - case LY_ENOTFOUND: - ecode = "item not found"; - break; - case LY_EINT: - ecode = "operation interrupted"; - break; - case LY_EVALID: - ecode = "validation failed"; - break; - case LY_EDENIED: - ecode = "access denied"; - break; - case LY_EINCOMPLETE: - ecode = "incomplete"; - break; - case LY_ERECOMPILE: - ecode = "compile error"; - break; - case LY_ENOT: - ecode = "not"; - break; - default: - case LY_EPLUGIN: - case LY_EOTHER: - ecode = "other"; - break; - } - - if (err == LY_EVALID) { - switch (ei->vecode) { - case LYVE_SUCCESS: - evalid = NULL; - break; - case LYVE_SYNTAX: - evalid = "syntax"; - break; - case LYVE_SYNTAX_YANG: - evalid = "yang-syntax"; - break; - case LYVE_SYNTAX_YIN: - evalid = "yin-syntax"; - break; - case LYVE_REFERENCE: - evalid = "reference"; - break; - case LYVE_XPATH: - evalid = "xpath"; - break; - case LYVE_SEMANTICS: - evalid = "semantics"; - break; - case LYVE_SYNTAX_XML: - evalid = "xml-syntax"; - break; - case LYVE_SYNTAX_JSON: - evalid = "json-syntax"; - break; - case LYVE_DATA: - evalid = "data"; - break; - default: - case LYVE_OTHER: - evalid = "other"; - break; - } - } + ecode = yang_ly_strerrcode(err); + if (err == LY_EVALID && ei->vecode != LYVE_SUCCESS) + evalid = yang_ly_strvecode(ei->vecode); switch (format) { case LYD_XML: diff --git a/lib/yang.c b/lib/yang.c index b2cc71b30918..2860108d6454 100644 --- a/lib/yang.c +++ b/lib/yang.c @@ -999,86 +999,109 @@ int yang_get_node_keys(struct lyd_node *node, struct yang_list_keys *keys) return NB_OK; } +/* + * ------------------------ + * Libyang Future Functions + * ------------------------ + * + * All these functions are implemented in libyang versions (perhaps unreleased) + * beyond what we require currently so we must supply the functionality. + */ + +/* + * Safe to remove after libyang v2.1.xxx is required (.144 has a bug so + * something > .144) https://github.com/CESNET/libyang/issues/2149 + */ LY_ERR yang_lyd_new_list(struct lyd_node_inner *parent, const struct lysc_node *snode, const struct yang_list_keys *list_keys, - struct lyd_node_inner **node) + struct lyd_node **node) { +#if defined(HAVE_LYD_NEW_LIST3) && 0 + LY_ERR err; + const char *keys[LIST_MAXKEYS]; + + assert(list_keys->num <= LIST_MAXKEYS); + for (int i = 0; i < list_keys->num; i++) + keys[i] = list_keys->key[i]; + + err = lyd_new_list3(&parent->node, snode->module, snode->name, keys, + NULL, 0, node); + return err; +#else struct lyd_node *pnode = &parent->node; - struct lyd_node **nodepp = (struct lyd_node **)node; const char(*keys)[LIST_MAXKEYLEN] = list_keys->key; - /* - * When - * https://github.com/CESNET/libyang/commit/2c1e327c7c2dd3ba12d466a4ebcf62c1c44116c4 - * is released in libyang we should add a configure.ac check for the - * lyd_new_list3 function and use it here. - */ + assert(list_keys->num <= 8); switch (list_keys->num) { case 0: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp); + node); case 1: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0]); + node, keys[0]); case 2: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0], keys[1]); + node, keys[0], keys[1]); case 3: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0], keys[1], keys[2]); + node, keys[0], keys[1], keys[2]); case 4: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0], keys[1], keys[2], keys[3]); + node, keys[0], keys[1], keys[2], keys[3]); case 5: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0], keys[1], keys[2], keys[3], + node, keys[0], keys[1], keys[2], keys[3], keys[4]); case 6: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0], keys[1], keys[2], keys[3], + node, keys[0], keys[1], keys[2], keys[3], keys[4], keys[5]); case 7: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0], keys[1], keys[2], keys[3], + node, keys[0], keys[1], keys[2], keys[3], keys[4], keys[5], keys[6]); case 8: return lyd_new_list(pnode, snode->module, snode->name, false, - nodepp, keys[0], keys[1], keys[2], keys[3], + node, keys[0], keys[1], keys[2], keys[3], keys[4], keys[5], keys[6], keys[7]); } _Static_assert(LIST_MAXKEYS == 8, "max key mismatch in switch unroll"); /*NOTREACHED*/ return LY_EINVAL; +#endif } -int yang_trim_tree(struct lyd_node *root, const char *xpath) +/* + * Safe to remove after libyang v2.1.144 is required + */ +LY_ERR yang_lyd_trim_xpath(struct lyd_node **root, const char *xpath) { - enum nb_error ret = NB_OK; LY_ERR err; -#if 0 - err = lyd_trim_xpath(&root, xpath, NULL); +#ifdef HAVE_LYD_TRIM_XPATH + err = lyd_trim_xpath(root, xpath, NULL); if (err) { flog_err_sys(EC_LIB_LIBYANG, - "cannot obtain specific result for xpath \"%s\"", - xpath); - return NB_ERR; + "cannot obtain specific result for xpath \"%s\": %s", + xpath, yang_ly_strerrcode(err)); + return err; } - return NB_OK; + return LY_SUCCESS; #else struct lyd_node *node; struct lyd_node **remove = NULL; struct ly_set *set = NULL; uint32_t i; - err = lyd_find_xpath3(NULL, root, xpath, NULL, &set); + *root = lyd_first_sibling(*root); + + err = lyd_find_xpath3(NULL, *root, xpath, NULL, &set); if (err) { flog_err_sys(EC_LIB_LIBYANG, - "cannot obtain specific result for xpath \"%s\"", - xpath); - ret = NB_ERR; - goto done; + "cannot obtain specific result for xpath \"%s\": %s", + xpath, yang_ly_strerrcode(err)); + return err; } /* * Mark keepers and sweep deleting non-keepers. @@ -1100,7 +1123,7 @@ int yang_trim_tree(struct lyd_node *root, const char *xpath) } darr_ensure_cap(remove, 128); - LYD_TREE_DFS_BEGIN (root, node) { + LYD_TREE_DFS_BEGIN (*root, node) { /* * If this is a direct matching node then include it's subtree * which won't be marked and would otherwise be removed. @@ -1108,19 +1131,100 @@ int yang_trim_tree(struct lyd_node *root, const char *xpath) if (node->priv == (void *)2) LYD_TREE_DFS_continue = 1; else if (!node->priv) { - LYD_TREE_DFS_continue = 1; *darr_append(remove) = node; + LYD_TREE_DFS_continue = 1; } - LYD_TREE_DFS_END(root, node); + LYD_TREE_DFS_END(*root, node); } - darr_foreach_i (remove, i) + darr_foreach_i (remove, i) { + if (remove[i] == *root) + *root = (*root)->next; lyd_free_tree(remove[i]); + } darr_free(remove); -done: if (set) ly_set_free(set, NULL); - return ret; + return LY_SUCCESS; +#endif +} + +/* + * Safe to remove after libyang v2.1.128 is required + */ +const char *yang_ly_strerrcode(LY_ERR err) +{ +#ifdef HAVE_LY_STRERRCODE + return ly_strerrcode(err); +#else + switch (err) { + case LY_SUCCESS: + return "ok"; + case LY_EMEM: + return "out of memory"; + case LY_ESYS: + return "system error"; + case LY_EINVAL: + return "invalid value given"; + case LY_EEXIST: + return "item exists"; + case LY_ENOTFOUND: + return "item not found"; + case LY_EINT: + return "operation interrupted"; + case LY_EVALID: + return "validation failed"; + case LY_EDENIED: + return "access denied"; + case LY_EINCOMPLETE: + return "incomplete"; + case LY_ERECOMPILE: + return "compile error"; + case LY_ENOT: + return "not"; + case LY_EPLUGIN: + case LY_EOTHER: + return "other"; + default: + return "unknown"; + } +#endif +} + +/* + * Safe to remove after libyang v2.1.128 is required + */ +const char *yang_ly_strvecode(LY_VECODE vecode) +{ +#ifdef HAVE_LY_STRVECODE + return ly_strvecode(vecode); +#else + switch (vecode) { + case LYVE_SUCCESS: + return ""; + case LYVE_SYNTAX: + return "syntax"; + case LYVE_SYNTAX_YANG: + return "yang-syntax"; + case LYVE_SYNTAX_YIN: + return "yin-syntax"; + case LYVE_REFERENCE: + return "reference"; + case LYVE_XPATH: + return "xpath"; + case LYVE_SEMANTICS: + return "semantics"; + case LYVE_SYNTAX_XML: + return "xml-syntax"; + case LYVE_SYNTAX_JSON: + return "json-syntax"; + case LYVE_DATA: + return "data"; + case LYVE_OTHER: + return "other"; + default: + return "unknown"; + } #endif } diff --git a/lib/yang.h b/lib/yang.h index 75dcab2d2af8..dbb7f7163bd2 100644 --- a/lib/yang.h +++ b/lib/yang.h @@ -719,11 +719,6 @@ extern int yang_get_key_preds(char *s, const struct lysc_node *snode, /* Get YANG keys from an existing dnode */ extern int yang_get_node_keys(struct lyd_node *node, struct yang_list_keys *keys); -/* Create a new list lyd_node using `yang_list_keys` */ -extern LY_ERR yang_lyd_new_list(struct lyd_node_inner *parent, - const struct lysc_node *snode, - const struct yang_list_keys *keys, - struct lyd_node_inner **node); /** * yang_resolve_snodes() - Resolve an XPath to matching schema nodes. * @ly_ctx: libyang context to operate on. @@ -744,14 +739,16 @@ extern LY_ERR yang_lyd_new_list(struct lyd_node_inner *parent, extern LY_ERR yang_resolve_snode_xpath(struct ly_ctx *ly_ctx, const char *xpath, struct lysc_node ***snodes, bool *simple); -/** - * yang_trim_tree() - trim the data tree to the given xpath - * @root: the data tree - * @xpath: the xpath to trim @root to. - * - * Return: enum nb_error.. +/* + * Libyang future functions */ -extern int yang_trim_tree(struct lyd_node *root, const char *xpath); +extern const char *yang_ly_strerrcode(LY_ERR err); +extern const char *yang_ly_strvecode(LY_VECODE vecode); +extern LY_ERR yang_lyd_new_list(struct lyd_node_inner *parent, + const struct lysc_node *snode, + const struct yang_list_keys *keys, + struct lyd_node **nodes); +extern LY_ERR yang_lyd_trim_xpath(struct lyd_node **rootp, const char *xpath); #ifdef __cplusplus } diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index 1c6444563144..297482f01548 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -1278,8 +1278,9 @@ static int txn_get_tree_data_done(struct mgmt_txn_ctx *txn, /* * We have a complex query so Filter results by the xpath query. */ - ret = yang_trim_tree(get_tree->client_results, - txn_req->req.get_tree->xpath); + if (yang_lyd_trim_xpath(&get_tree->client_results, + txn_req->req.get_tree->xpath)) + ret = NB_ERR; } if (ret == NB_OK) From e85ff7a1f20d2429bffa2922272194da7326783f Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sat, 6 Jan 2024 09:47:04 +0000 Subject: [PATCH 4/4] tests: test new XPath 1.0 predicate functionality Signed-off-by: Christian Hopps --- tests/topotests/mgmt_oper/test_querying.py | 10 ++++++++++ tests/topotests/mgmt_oper/test_scale.py | 12 +++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/topotests/mgmt_oper/test_querying.py b/tests/topotests/mgmt_oper/test_querying.py index e53ea52c987a..220f2b4abd5e 100644 --- a/tests/topotests/mgmt_oper/test_querying.py +++ b/tests/topotests/mgmt_oper/test_querying.py @@ -51,6 +51,16 @@ def test_oper_simple(tgen): pytest.skip(tgen.errors) query_results = [ + # Non-key specific query with function filtering selector + '/frr-interface:lib/interface[contains(name,"eth")]/vrf', + # Non-key specific query with child value filtering selector + '/frr-interface:lib/interface[vrf="red"]/vrf', + '/frr-interface:lib/interface[./vrf="red"]/vrf', + # Container query with function filtering selector + '/frr-interface:lib/interface[contains(name,"eth")]/state', + # Multi list elemenet with function filtering selector + '/frr-interface:lib/interface[contains(name,"eth")]', + # # Specific list entry after non-specific lists '/frr-vrf:lib/vrf[name="default"]/frr-zebra:zebra/ribs/' 'rib[afi-safi-name="frr-routing:ipv4-unicast"][table-id="254"]/' diff --git a/tests/topotests/mgmt_oper/test_scale.py b/tests/topotests/mgmt_oper/test_scale.py index d7a0e25ad819..391400f9145c 100644 --- a/tests/topotests/mgmt_oper/test_scale.py +++ b/tests/topotests/mgmt_oper/test_scale.py @@ -11,7 +11,7 @@ """ Test static route functionality """ -import logging +import re import time import pytest @@ -63,5 +63,11 @@ def test_oper_simple(tgen): check_kernel_32(r1, "20.0.0.0", count, vrf, 1000) step(f"All {count} routes installed in kernel, continuing") - output = r1.cmd_raises("vtysh -c 'show mgmt get-data /frr-vrf:lib'") - step("Got output: output") + # output = r1.cmd_raises("vtysh -c 'show mgmt get-data /frr-vrf:lib'") + # step(f"Got output: {output[0:1024]}") + + query = '/frr-vrf:lib/vrf/frr-zebra:zebra/ribs/rib/route[contains(prefix,"20.0.0.12")]/prefix' + output = r1.cmd_raises(f"vtysh -c 'show mgmt get-data {query}'") + matches = re.findall(r'"prefix":', output) + # 20.0.0.12 + 20.0.0.12{0,1,2,3,4,5,6,7,8,9} + assert len(matches) == 11