diff --git a/lib/northbound.c b/lib/northbound.c index ad9e517d52..f860b83c45 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -127,6 +127,8 @@ static int nb_node_new_cb(const struct lysc_node *snode, void *arg) if (module && module->ignore_cfg_cbs) SET_FLAG(nb_node->flags, F_NB_NODE_IGNORE_CFG_CBS); + if (module && module->get_tree_locked) + SET_FLAG(nb_node->flags, F_NB_NODE_HAS_GET_TREE); return YANG_ITER_CONTINUE; } @@ -256,6 +258,7 @@ static unsigned int nb_node_validate_cbs(const struct nb_node *nb_node) { unsigned int error = 0; + bool state_optional = CHECK_FLAG(nb_node->flags, F_NB_NODE_HAS_GET_TREE); if (CHECK_FLAG(nb_node->flags, F_NB_NODE_IGNORE_CFG_CBS)) return error; @@ -273,15 +276,15 @@ static unsigned int nb_node_validate_cbs(const struct nb_node *nb_node) error += nb_node_validate_cb(nb_node, NB_CB_APPLY_FINISH, !!nb_node->cbs.apply_finish, true); error += nb_node_validate_cb(nb_node, NB_CB_GET_ELEM, - (nb_node->cbs.get_elem || nb_node->cbs.get), false); + (nb_node->cbs.get_elem || nb_node->cbs.get), state_optional); error += nb_node_validate_cb(nb_node, NB_CB_GET_NEXT, (nb_node->cbs.get_next || (nb_node->snode->nodetype == LYS_LEAFLIST && nb_node->cbs.get)), - false); - error += nb_node_validate_cb(nb_node, NB_CB_GET_KEYS, - !!nb_node->cbs.get_keys, false); - error += nb_node_validate_cb(nb_node, NB_CB_LOOKUP_ENTRY, - !!nb_node->cbs.lookup_entry, false); + state_optional); + error += nb_node_validate_cb(nb_node, NB_CB_GET_KEYS, !!nb_node->cbs.get_keys, + state_optional); + error += nb_node_validate_cb(nb_node, NB_CB_LOOKUP_ENTRY, !!nb_node->cbs.lookup_entry, + state_optional); error += nb_node_validate_cb(nb_node, NB_CB_RPC, !!nb_node->cbs.rpc, false); error += nb_node_validate_cb(nb_node, NB_CB_NOTIFY, @@ -2730,7 +2733,7 @@ void nb_init(struct event_loop *tm, const struct frr_yang_module_info *const modules[], size_t nmodules, bool db_enabled, bool load_library) { - struct yang_module *loaded[nmodules], **loadedp = loaded; + struct yang_module *loaded[nmodules]; /* * Currently using this explicit compile feature in libyang2 leads to @@ -2750,8 +2753,8 @@ void nb_init(struct event_loop *tm, for (size_t i = 0; i < nmodules; i++) { DEBUGD(&nb_dbg_events, "northbound: loading %s.yang", modules[i]->name); - *loadedp++ = yang_module_load(modules[i]->name, - modules[i]->features); + loaded[i] = yang_module_load(modules[i]->name, modules[i]->features); + loaded[i]->frr_info = modules[i]; } if (explicit_compile) diff --git a/lib/northbound.h b/lib/northbound.h index 7d38c63a86..0468c58de3 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -674,6 +674,8 @@ struct nb_node { #define F_NB_NODE_KEYLESS_LIST 0x02 /* Ignore config callbacks for this node */ #define F_NB_NODE_IGNORE_CFG_CBS 0x04 +/* Ignore state callbacks for this node */ +#define F_NB_NODE_HAS_GET_TREE 0x08 /* * HACK: old gcc versions (< 5.x) have a bug that prevents C99 flexible arrays @@ -701,6 +703,22 @@ struct frr_yang_module_info { */ const char **features; + /* + * If the module keeps its oper-state in a libyang tree + * this function should return that tree (locked if multi-threading). + * If this function is provided then the state callback functions + * (get_elem, get_keys, get_next, lookup_entry) need not be set for a + * module. The unlock_tree function if non-NULL will be called with + * the returned tree and the *user_lock value. + */ + const struct lyd_node *(*get_tree_locked)(const char *xpath, void **user_lock); + + /* + * This function will be called following a call to get_tree_locked() in + * order to unlock the tree if locking was required. + */ + void (*unlock_tree)(const struct lyd_node *tree, void *user_lock); + /* Northbound callbacks. */ const struct { /* Data path of this YANG node. */ @@ -1836,6 +1854,18 @@ extern struct lyd_node *nb_op_updatef(struct lyd_node *tree, const char *path, c extern struct lyd_node *nb_op_vupdatef(struct lyd_node *tree, const char *path, const char *val_fmt, va_list ap); +/** + * nb_notif_add() - Notice that the value at `path` has changed. + * @path - Absolute path in the state tree that has changed (either added or + * updated). + */ +void nb_notif_add(const char *path); + +/** + * nb_notif_delete() - Notice that the value at `path` has been deleted. + * @path - Absolute path in the state tree that has been deleted. + */ +void nb_notif_delete(const char *path); /** * nb_notif_set_filters() - add or replace notification filters @@ -1846,6 +1876,15 @@ extern struct lyd_node *nb_op_vupdatef(struct lyd_node *tree, const char *path, */ extern void nb_notif_set_filters(const char **selectors, bool replace); +/** + * nb_notif_enable_multi_thread() - enable use of multiple threads with nb_notif + * + * If the nb_notif_XXX calls will be made from multiple threads then locking is + * required. Call this function to enable that functionality, prior to using the + * nb_notif_XXX API. + */ +extern void nb_notif_enable_multi_thread(void); + extern void nb_notif_init(struct event_loop *loop); extern void nb_notif_terminate(void); diff --git a/lib/northbound_notif.c b/lib/northbound_notif.c index 9caca9f6d7..746b33beb2 100644 --- a/lib/northbound_notif.c +++ b/lib/northbound_notif.c @@ -58,6 +58,9 @@ RB_HEAD(op_changes, op_change); RB_PROTOTYPE(op_changes, op_change, link, op_change_cmp) RB_GENERATE(op_changes, op_change, link, op_change_cmp) +pthread_mutex_t _nb_notif_lock = PTHREAD_MUTEX_INITIALIZER; +pthread_mutex_t *nb_notif_lock; + struct op_changes nb_notif_adds = RB_INITIALIZER(&nb_notif_adds); struct op_changes nb_notif_dels = RB_INITIALIZER(&nb_notif_dels); struct event_loop *nb_notif_master; @@ -158,12 +161,14 @@ static void op_change_free(struct op_change *note) } /** - * op_changes_group_push() - Save the current set of changes on the queue. + * __op_changes_group_push() - Save the current set of changes on the queue. * * This function will save the current set of changes on the queue and * initialize a new set of changes. + * + * The lock must be held during this call. */ -static void op_changes_group_push(void) +static void __op_changes_group_push(void) { struct op_changes_group *changes; @@ -312,17 +317,34 @@ static void __op_change_add_del(const char *path, struct op_changes *this_head, nb_notif_set_walk_timer(); } -static void nb_notif_add(const char *path) +void nb_notif_add(const char *path) { + if (nb_notif_lock) + pthread_mutex_lock(nb_notif_lock); + __op_change_add_del(path, &nb_notif_adds, &nb_notif_dels); + + if (nb_notif_lock) + pthread_mutex_unlock(nb_notif_lock); } -static void nb_notif_delete(const char *path) +void nb_notif_delete(const char *path) { + if (nb_notif_lock) + pthread_mutex_lock(nb_notif_lock); + __op_change_add_del(path, &nb_notif_dels, &nb_notif_adds); + + if (nb_notif_lock) + pthread_mutex_unlock(nb_notif_lock); } + +/* ---------------------------------------------- */ +/* User functions to update and delete oper state */ +/* ---------------------------------------------- */ + struct lyd_node *nb_op_update(struct lyd_node *tree, const char *path, const char *value) { struct lyd_node *dnode; @@ -459,13 +481,21 @@ static struct op_changes_group *op_changes_group_next(void) { struct op_changes_group *group; + if (nb_notif_lock) + pthread_mutex_lock(nb_notif_lock); + group = op_changes_queue_pop(&op_changes_queue); if (!group) { - op_changes_group_push(); + __op_changes_group_push(); group = op_changes_queue_pop(&op_changes_queue); } + + if (nb_notif_lock) + pthread_mutex_unlock(nb_notif_lock); + if (!group) return NULL; + group->cur_changes = &group->dels; group->cur_change = RB_MIN(op_changes, group->cur_changes); if (!group->cur_change) { @@ -674,6 +704,11 @@ void nb_notif_set_filters(const char **selectors, bool replace) darr_free(selectors); } +void nb_notif_enable_multi_thread(void) +{ + nb_notif_lock = &_nb_notif_lock; +} + void nb_notif_init(struct event_loop *tm) { nb_notif_master = tm; diff --git a/lib/northbound_oper.c b/lib/northbound_oper.c index b7815b001a..ad495b6f9c 100644 --- a/lib/northbound_oper.c +++ b/lib/northbound_oper.c @@ -48,6 +48,9 @@ DEFINE_MTYPE_STATIC(LIB, NB_NODE_INFOS, "NB Node Infos"); /* ---------- */ PREDECL_LIST(nb_op_walks); +typedef const struct lyd_node *(*get_tree_locked_cb)(const char *xpath, void **user_tree_lock); +typedef void (*unlock_tree_cb)(const struct lyd_node *tree, void *user_tree_lock); + /* * This is our information about a node on the branch we are looking at */ @@ -81,6 +84,7 @@ struct nb_op_node_info { * @walk_start_level: @walk_root_level + 1. * @query_base_level: the level the query string stops at and full walks * commence below that. + * @user_tree: the user's existing state tree to copy state from or NULL. */ struct nb_op_yield_state { /* Walking state */ @@ -96,6 +100,11 @@ struct nb_op_yield_state { int query_base_level; bool query_list_entry; /* XXX query was for a specific list entry */ + /* For now we support a single use of this. */ + const struct lyd_node *user_tree; + void *user_tree_lock; + unlock_tree_cb user_tree_unlock; + /* Yielding state */ bool query_did_entry; /* currently processing the entry */ bool should_batch; @@ -125,6 +134,11 @@ static struct nb_op_walks_head nb_op_walks; static enum nb_error nb_op_yield(struct nb_op_yield_state *ys); static struct lyd_node *ys_root_node(struct nb_op_yield_state *ys); +static const void *nb_op_list_get_next(struct nb_op_yield_state *ys, struct nb_node *nb_node, + const struct nb_op_node_info *pni, const void *list_entry); +static const void *nb_op_list_lookup_entry(struct nb_op_yield_state *ys, struct nb_node *nb_node, + const struct nb_op_node_info *pni, struct lyd_node *node, + const struct yang_list_keys *keys); /* -------------------- */ /* Function Definitions */ @@ -163,6 +177,8 @@ static inline void nb_op_free_yield_state(struct nb_op_yield_state *ys, bool nofree_tree) { if (ys) { + if (ys->user_tree && ys->user_tree_unlock) + ys->user_tree_unlock(ys->user_tree, ys->user_tree_lock); EVENT_OFF(ys->walk_ev); nb_op_walks_del(&nb_op_walks, ys); /* if we have a branch then free up it's libyang tree */ @@ -300,9 +316,8 @@ static bool __move_back_to_next(struct nb_op_yield_state *ys, int i) static void nb_op_resume_data_tree(struct nb_op_yield_state *ys) { - struct nb_op_node_info *ni; + struct nb_op_node_info *pni, *ni; struct nb_node *nn; - const void *parent_entry; const void *list_entry; uint i; @@ -325,6 +340,7 @@ static void nb_op_resume_data_tree(struct nb_op_yield_state *ys) * restored. */ darr_foreach_i (ys->node_infos, i) { + pni = i > 0 ? &ys->node_infos[i - 1] : NULL; ni = &ys->node_infos[i]; nn = ni->schema->priv; @@ -335,9 +351,7 @@ static void nb_op_resume_data_tree(struct nb_op_yield_state *ys) ni == darr_last(ys->node_infos)); /* Verify the entry is still present */ - parent_entry = (i == 0 ? NULL : ni[-1].list_entry); - list_entry = nb_callback_lookup_entry(nn, parent_entry, - &ni->keys); + list_entry = nb_op_list_lookup_entry(ys, nn, pni, NULL, &ni->keys); if (!list_entry || list_entry != ni->list_entry) { /* May be NULL or a different pointer * move back to first of @@ -409,6 +423,7 @@ static enum nb_error nb_op_xpath_to_trunk(const char *xpath_in, char **xpath_out static enum nb_error nb_op_ys_finalize_node_info(struct nb_op_yield_state *ys, uint index) { + struct nb_op_node_info *pni = index == 0 ? NULL : &ys->node_infos[index - 1]; struct nb_op_node_info *ni = &ys->node_infos[index]; struct lyd_node *inner = ni->inner; struct nb_node *nn = ni->schema->priv; @@ -452,17 +467,12 @@ static enum nb_error nb_op_ys_finalize_node_info(struct nb_op_yield_state *ys, */ /* ni->list_entry starts as the parent entry of this node */ - ni->list_entry = nb_callback_get_next(nn, ni->list_entry, NULL); + ni->list_entry = nb_op_list_get_next(ys, nn, pni, NULL); for (i = 1; i < ni->position && ni->list_entry; i++) - ni->list_entry = nb_callback_get_next(nn, ni->list_entry, ni->list_entry); + ni->list_entry = nb_op_list_get_next(ys, nn, pni, ni->list_entry); - if (i != ni->position || !ni->list_entry) { - flog_warn(EC_LIB_NB_OPERATIONAL_DATA, - "%s: entry at position %d doesn't exist in: %s", __func__, - ni->position, ys->xpath); + if (i != ni->position || !ni->list_entry) return NB_ERR_NOT_FOUND; - } - } else { nb_op_get_keys((struct lyd_node_inner *)inner, &ni->keys); /* A list entry cannot be present in a tree w/o it's keys */ @@ -472,8 +482,10 @@ static enum nb_error nb_op_ys_finalize_node_info(struct nb_op_yield_state *ys, * Get this nodes opaque list_entry object */ + /* We need a lookup entry unless this is a keyless list */ - if (!nn->cbs.lookup_entry && ni->keys.num) { + if (!nn->cbs.lookup_entry && ni->keys.num && + !CHECK_FLAG(nn->flags, F_NB_NODE_HAS_GET_TREE)) { flog_warn(EC_LIB_NB_OPERATIONAL_DATA, "%s: data path doesn't support iteration over operational data: %s", __func__, ys->xpath); @@ -481,7 +493,7 @@ static enum nb_error nb_op_ys_finalize_node_info(struct nb_op_yield_state *ys, } /* ni->list_entry starts as the parent entry of this node */ - ni->list_entry = nb_callback_lookup_entry(nn, ni->list_entry, &ni->keys); + ni->list_entry = nb_op_list_lookup_entry(ys, nn, pni, NULL, &ni->keys); if (ni->list_entry == NULL) { flog_warn(EC_LIB_NB_OPERATIONAL_DATA, "%s: list entry lookup failed", __func__); @@ -635,6 +647,222 @@ static enum nb_error nb_op_ys_init_node_infos(struct nb_op_yield_state *ys) /* End of init code */ /* ================ */ +static const char *__module_name(const struct nb_node *nb_node) +{ + return nb_node->snode->module->name; +} + +static get_tree_locked_cb __get_get_tree_funcs(const char *module_name, + unlock_tree_cb *unlock_func_pp) +{ + struct yang_module *module = yang_module_find(module_name); + + if (!module || !module->frr_info->get_tree_locked) + return NULL; + + *unlock_func_pp = module->frr_info->unlock_tree; + return module->frr_info->get_tree_locked; +} + +static const struct lyd_node *__get_tree(struct nb_op_yield_state *ys, + const struct nb_node *nb_node, const char *xpath) +{ + get_tree_locked_cb get_tree_cb; + + if (ys->user_tree) + return ys->user_tree; + + get_tree_cb = __get_get_tree_funcs(__module_name(nb_node), &ys->user_tree_unlock); + assert(get_tree_cb); + + ys->user_tree = get_tree_cb(xpath, &ys->user_tree_lock); + return ys->user_tree; +} + +/** + * nb_op_libyang_cb_get() - get a leaf value from user supplied libyang tree. + */ +static enum nb_error nb_op_libyang_cb_get(struct nb_op_yield_state *ys, + const struct nb_node *nb_node, struct lyd_node *parent, + const char *xpath) +{ + const struct lysc_node *snode = nb_node->snode; + const struct lyd_node *tree = __get_tree(ys, nb_node, xpath); + struct lyd_node *node; + LY_ERR err; + + err = lyd_find_path(tree, xpath, false, &node); + /* We are getting LY_EINCOMPLETE for missing `type empty` nodes */ + if (err == LY_ENOTFOUND || err == LY_EINCOMPLETE) + return NB_OK; + else if (err != LY_SUCCESS) + return NB_ERR; + if (lyd_dup_single_to_ctx(node, snode->module->ctx, (struct lyd_node_inner *)parent, 0, + &node)) + return NB_ERR; + return NB_OK; +} + +static enum nb_error nb_op_libyang_cb_get_leaflist(struct nb_op_yield_state *ys, + const struct nb_node *nb_node, + struct lyd_node *parent, const char *xpath) +{ + const struct lysc_node *snode = nb_node->snode; + const struct lyd_node *tree = __get_tree(ys, nb_node, xpath); + struct ly_set *set = NULL; + LY_ERR err; + int ret = NB_OK; + uint i; + + err = lyd_find_xpath(tree, xpath, &set); + /* We are getting LY_EINCOMPLETE for missing `type empty` nodes */ + if (err == LY_ENOTFOUND || err == LY_EINCOMPLETE) + return NB_OK; + else if (err != LY_SUCCESS) + return NB_ERR; + + for (i = 0; i < set->count; i++) { + if (lyd_dup_single_to_ctx(set->dnodes[i], snode->module->ctx, + (struct lyd_node_inner *)parent, 0, NULL)) { + ret = NB_ERR; + break; + } + } + ly_set_free(set, NULL); + return ret; +} + +static const struct lyd_node *__get_node_other_tree(const struct lyd_node *tree, + const struct lyd_node *parent_node, + const struct lysc_node *schema, + const struct yang_list_keys *keys) +{ + char xpath[XPATH_MAXLEN]; + struct lyd_node *node; + int schema_len = strlen(schema->name); + struct ly_set *set = NULL; + int len; + + if (!parent_node) { + /* we need a full path to the schema node */ + if (!lysc_path(schema, LYSC_PATH_DATA, xpath, sizeof(xpath))) + return NULL; + len = strlen(xpath); + } else { + if (!lyd_path(parent_node, LYD_PATH_STD, xpath, sizeof(xpath))) + return NULL; + len = strlen(xpath); + /* do we have room for slash and the node basename? */ + if (len + 1 + schema_len + 1 > XPATH_MAXLEN) + return NULL; + xpath[len++] = '/'; + strlcpy(&xpath[len], schema->name, sizeof(xpath) - len); + len += schema_len; + } + if (keys) + yang_get_key_preds(&xpath[len], schema, keys, sizeof(xpath) - len); + + if (lyd_find_xpath(tree, xpath, &set)) + return NULL; + if (set->count < 1) + return NULL; + node = set->dnodes[0]; + ly_set_free(set, NULL); + return node; +} + +static const void *nb_op_list_lookup_entry(struct nb_op_yield_state *ys, struct nb_node *nb_node, + const struct nb_op_node_info *pni, struct lyd_node *node, + const struct yang_list_keys *keys) +{ + struct yang_list_keys _keys; + const struct lyd_node *tree; + const struct lyd_node *parent_node; + + /* Use user callback */ + if (!CHECK_FLAG(nb_node->flags, F_NB_NODE_HAS_GET_TREE)) { + if (node) + return nb_callback_lookup_node_entry(node, pni ? pni->list_entry : NULL); + + assert(keys); + return nb_callback_lookup_entry(nb_node, pni ? pni->list_entry : NULL, keys); + } + + if (!keys) { + assert(node); + if (yang_get_node_keys(node, &_keys)) { + flog_warn(EC_LIB_LIBYANG, + "%s: can't get keys for lookup from existing data node %s", + __func__, node->schema->name); + return NULL; + } + keys = &_keys; + } + tree = __get_tree(ys, nb_node, NULL); + parent_node = pni ? pni->inner : NULL; + return __get_node_other_tree(tree, parent_node, nb_node->snode, keys); +} + +static const void *__get_next(struct nb_op_yield_state *ys, struct nb_node *nb_node, + const struct nb_op_node_info *pni, const void *list_entry) +{ + const struct lysc_node *snode = nb_node->snode; + const struct lyd_node *tree = __get_tree(ys, nb_node, NULL); + const struct lyd_node *parent_node = pni ? pni->inner : NULL; + const struct lyd_node *node = list_entry; + + if (!node) + return __get_node_other_tree(tree, parent_node, snode, NULL); + + node = node->next; + LY_LIST_FOR (node, node) { + if (node->schema == snode) + break; + } + return node; +} + +static const void *nb_op_list_get_next(struct nb_op_yield_state *ys, struct nb_node *nb_node, + const struct nb_op_node_info *pni, const void *list_entry) +{ + if (!CHECK_FLAG(nb_node->flags, F_NB_NODE_HAS_GET_TREE)) + return nb_callback_get_next(nb_node, pni ? pni->list_entry : NULL, list_entry); + return __get_next(ys, nb_node, pni, list_entry); +} + +static enum nb_error nb_op_list_get_keys(struct nb_op_yield_state *ys, struct nb_node *nb_node, + const void *list_entry, struct yang_list_keys *keys) +{ + const struct lyd_node_inner *list_node = list_entry; + const struct lyd_node *child; + uint count = 0; + + /* Use user callback */ + if (!CHECK_FLAG(nb_node->flags, F_NB_NODE_HAS_GET_TREE)) + return nb_callback_get_keys(nb_node, list_entry, keys); + + assert(list_node->schema->nodetype == LYS_LIST); + + /* + * NOTE: libyang current stores the keys as the first children of a list + * node we count on that here. + */ + + LY_LIST_FOR (lyd_child(&list_node->node), child) { + if (!lysc_is_key(child->schema)) + break; + if (count == LIST_MAXKEYS) { + zlog_err("Too many keys for list_node: %s", list_node->schema->name); + break; + } + strlcpy(keys->key[count++], lyd_get_value(child), sizeof(keys->key[0])); + } + keys->num = count; + + return 0; +} + + /** * nb_op_add_leaf() - Add leaf data to the get tree results * @ys - the yield state for this tree walk. @@ -660,8 +888,13 @@ static enum nb_error nb_op_iter_leaf(struct nb_op_yield_state *ys, if (lysc_is_key(snode)) return NB_OK; + /* See if we use data tree directly */ + if (CHECK_FLAG(nb_node->flags, F_NB_NODE_HAS_GET_TREE)) + return nb_op_libyang_cb_get(ys, nb_node, ni->inner, xpath); + /* Check for new simple get */ if (nb_node->cbs.get) + /* XXX: need to run through translator */ return nb_node->cbs.get(nb_node, ni->list_entry, ni->inner); data = nb_callback_get_elem(nb_node, xpath, ni->list_entry); @@ -699,8 +932,13 @@ static enum nb_error nb_op_iter_leaflist(struct nb_op_yield_state *ys, /* Check for new simple get */ if (nb_node->cbs.get) + /* XXX: need to run through translator */ return nb_node->cbs.get(nb_node, ni->list_entry, ni->inner); + if (CHECK_FLAG(nb_node->flags, F_NB_NODE_HAS_GET_TREE)) + /* XXX: need to run through translator */ + return nb_op_libyang_cb_get_leaflist(ys, nb_node, ni->inner, xpath); + do { struct yang_data *data; @@ -1313,9 +1551,8 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume) * -------------------- */ if (list_start) { - list_entry = - nb_callback_lookup_node_entry( - node, parent_list_entry); + list_entry = nb_op_list_lookup_entry(ys, nn, pni, node, + NULL); /* * If the node we created from a * specific predicate entry is not @@ -1348,10 +1585,7 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume) * (list_entry != NULL) the list iteration. */ /* Obtain [next] list entry. */ - list_entry = - nb_callback_get_next(nn, - parent_list_entry, - list_entry); + list_entry = nb_op_list_get_next(ys, nn, pni, list_entry); } /* @@ -1477,8 +1711,7 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume) /* Need to get keys. */ if (!CHECK_FLAG(nn->flags, F_NB_NODE_KEYLESS_LIST)) { - ret = nb_callback_get_keys(nn, list_entry, - &ni->keys); + ret = nb_op_list_get_keys(ys, nn, list_entry, &ni->keys); if (ret) { darr_pop(ys->node_infos); ret = NB_ERR_RESOURCE; @@ -1490,8 +1723,9 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume) */ len = darr_strlen(ys->xpath); if (ni->keys.num) { - yang_get_key_preds(ys->xpath + len, sib, - &ni->keys, + darr_ensure_avail(ys->xpath, + yang_get_key_pred_strlen(sib, &ni->keys) + 1); + yang_get_key_preds(ys->xpath + len, sib, &ni->keys, darr_cap(ys->xpath) - len); } else { /* add a position predicate (1s based?) */ diff --git a/lib/yang.c b/lib/yang.c index dd48d8861b..a57b247634 100644 --- a/lib/yang.c +++ b/lib/yang.c @@ -1357,9 +1357,21 @@ uint32_t yang_get_list_elements_count(const struct lyd_node *node) } while (node); return count; } +int yang_get_key_pred_strlen(const struct lysc_node *snode, const struct yang_list_keys *keys) +{ + const struct lysc_node_leaf *skey; + size_t len = 0; + size_t i = 0; -int yang_get_key_preds(char *s, const struct lysc_node *snode, - struct yang_list_keys *keys, ssize_t space) + LY_FOR_KEYS (snode, skey) { + /* [%s='%s'] */ + len += 5 + strlen(skey->name) + strlen(keys->key[i]); + i++; + } + return len; +} + +int yang_get_key_preds(char *s, const struct lysc_node *snode, const struct yang_list_keys *keys, ssize_t space) { const struct lysc_node_leaf *skey; ssize_t len2, len = 0; diff --git a/lib/yang.h b/lib/yang.h index 748f089037..3877a421c5 100644 --- a/lib/yang.h +++ b/lib/yang.h @@ -20,6 +20,8 @@ extern "C" { #endif +struct frr_yang_module_info; + /* Maximum XPath length. */ #define XPATH_MAXLEN 1024 @@ -45,6 +47,7 @@ struct yang_module { RB_ENTRY(yang_module) entry; const char *name; const struct lys_module *info; + const struct frr_yang_module_info *frr_info; #ifdef HAVE_SYSREPO sr_subscription_ctx_t *sr_subscription; struct event *sr_thread; @@ -879,7 +882,11 @@ bool yang_is_last_level_dnode(const struct lyd_node *dnode); /* Create a YANG predicate string based on the keys */ extern int yang_get_key_preds(char *s, const struct lysc_node *snode, - struct yang_list_keys *keys, ssize_t space); + const struct yang_list_keys *keys, ssize_t space); + +/* Get the length of the predicate string based on the keys */ +extern int yang_get_key_pred_strlen(const struct lysc_node *snode, + const struct yang_list_keys *keys); /* Get YANG keys from an existing dnode */ extern int yang_get_node_keys(struct lyd_node *node, struct yang_list_keys *keys); diff --git a/tests/lib/northbound/test_oper_data.c b/tests/lib/northbound/test_oper_data.c index 0b334c6522..a38325173a 100644 --- a/tests/lib/northbound/test_oper_data.c +++ b/tests/lib/northbound/test_oper_data.c @@ -236,13 +236,9 @@ static int frr_test_module_vrfs_vrf_ping(struct nb_cb_rpc_args *args) return NB_OK; } -/* - * XPath: /frr-test-module:frr-test-module/c1value - */ -static struct yang_data * -frr_test_module_c1value_get_elem(struct nb_cb_get_elem_args *args) +static struct yang_data *__return_null(struct nb_cb_get_elem_args *args) { - return yang_data_new_uint8(args->xpath, 21); + return NULL; } /* @@ -263,6 +259,14 @@ static enum nb_error frr_test_module_c2cont_c2value_get(const struct nb_node *nb return NB_OK; } +/* + * XPath: /frr-test-module:frr-test-module/c3value + */ +static struct yang_data *frr_test_module_c3value_get_elem(struct nb_cb_get_elem_args *args) +{ + return yang_data_new_uint8(args->xpath, 21); +} + /* clang-format off */ const struct frr_yang_module_info frr_test_module_info = { .name = "frr-test-module", @@ -316,12 +320,20 @@ const struct frr_yang_module_info frr_test_module_info = { }, { .xpath = "/frr-test-module:frr-test-module/c1value", - .cbs.get_elem = frr_test_module_c1value_get_elem, + .cbs.get_elem = __return_null, }, { .xpath = "/frr-test-module:frr-test-module/c2cont/c2value", .cbs.get = frr_test_module_c2cont_c2value_get, }, + { + .xpath = "/frr-test-module:frr-test-module/c3value", + .cbs.get_elem = frr_test_module_c3value_get_elem, + }, + { + .xpath = "/frr-test-module:frr-test-module/c4cont/c4value", + .cbs.get_elem = __return_null, + }, { .xpath = NULL, }, diff --git a/tests/lib/northbound/test_oper_data.in b/tests/lib/northbound/test_oper_data.in index 94fcdc1e1c..bed83b8d74 100644 --- a/tests/lib/northbound/test_oper_data.in +++ b/tests/lib/northbound/test_oper_data.in @@ -2,7 +2,7 @@ show yang operational-data /frr-test-module:frr-test-module show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[2] show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[3]/interface show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[10] -show yang operational-data /frr-test-module:frr-test-module/c1value +show yang operational-data /frr-test-module:frr-test-module/c3value show yang operational-data /frr-test-module:frr-test-module/c2cont show yang operational-data /frr-test-module:frr-test-module/c2cont/ show yang operational-data /frr-test-module:frr-test-module/c2cont/c2value diff --git a/tests/lib/northbound/test_oper_data.refout b/tests/lib/northbound/test_oper_data.refout index 57061d0371..2feadf4b77 100644 --- a/tests/lib/northbound/test_oper_data.refout +++ b/tests/lib/northbound/test_oper_data.refout @@ -125,10 +125,10 @@ test# show yang operational-data /frr-test-module:frr-test-module } ] }, - "c1value": 21, "c2cont": { "c2value": 2868969987 - } + }, + "c3value": 21 } } test# show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[2] @@ -174,10 +174,10 @@ test# show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name= } test# show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[10] {} -test# show yang operational-data /frr-test-module:frr-test-module/c1value +test# show yang operational-data /frr-test-module:frr-test-module/c3value { "frr-test-module:frr-test-module": { - "c1value": 21 + "c3value": 21 } } test# show yang operational-data /frr-test-module:frr-test-module/c2cont diff --git a/tests/lib/northbound/test_oper_exists.c b/tests/lib/northbound/test_oper_exists.c new file mode 100644 index 0000000000..17afcc7fd4 --- /dev/null +++ b/tests/lib/northbound/test_oper_exists.c @@ -0,0 +1,266 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2018 NetDEF, Inc. + * Renato Westphal + * Copyright (C) 2025 LabN Consulting, L.L.C. + */ + +#include +#include + +#include "debug.h" +#include "frrevent.h" +#include "vty.h" +#include "command.h" +#include "memory.h" +#include "lib_vty.h" +#include "log.h" +#include "northbound.h" +#include "northbound_cli.h" + +static struct event_loop *master; +static struct lyd_node *data_tree; +static uint data_tree_lock; + +const char *data_json = "\n" + "{\n" + " \"frr-test-module:frr-test-module\": {\n" + " \"vrfs\": {\n" + " \"vrf\": [\n" + " {\n" + " \"name\": \"vrf0\",\n" + " \"interfaces\": {\n" + " \"interface\": [\n" + " \"eth0\",\n" + " \"eth1\",\n" + " \"eth2\",\n" + " \"eth3\"\n" + " ],\n" + " \"interface-new\": [\n" + " \"eth0\",\n" + " \"eth1\",\n" + " \"eth2\",\n" + " \"eth3\"\n" + " ]\n" + " },\n" + " \"routes\": {\n" + " \"route\": [\n" + " {\n" + " \"prefix\": \"10.0.0.0/32\",\n" + " \"next-hop\": \"172.16.0.0\",\n" + " \"interface\": \"eth0\",\n" + " \"metric\": 0,\n" + " \"active\": [null]\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.1/32\",\n" + " \"next-hop\": \"172.16.0.1\",\n" + " \"interface\": \"eth1\",\n" + " \"metric\": 1\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.2/32\",\n" + " \"next-hop\": \"172.16.0.2\",\n" + " \"interface\": \"eth2\",\n" + " \"metric\": 2,\n" + " \"active\": [null]\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.3/32\",\n" + " \"next-hop\": \"172.16.0.3\",\n" + " \"interface\": \"eth3\",\n" + " \"metric\": 3\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.4/32\",\n" + " \"next-hop\": \"172.16.0.4\",\n" + " \"interface\": \"eth4\",\n" + " \"metric\": 4,\n" + " \"active\": [null]\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.5/32\",\n" + " \"next-hop\": \"172.16.0.5\",\n" + " \"interface\": \"eth5\",\n" + " \"metric\": 5\n" + " }\n" + " ]\n" + " }\n" + " },\n" + " {\n" + " \"name\": \"vrf1\",\n" + " \"interfaces\": {\n" + " \"interface\": [\n" + " \"eth0\",\n" + " \"eth1\",\n" + " \"eth2\",\n" + " \"eth3\"\n" + " ],\n" + " \"interface-new\": [\n" + " \"eth0\",\n" + " \"eth1\",\n" + " \"eth2\",\n" + " \"eth3\"\n" + " ]\n" + " },\n" + " \"routes\": {\n" + " \"route\": [\n" + " {\n" + " \"prefix\": \"10.0.0.0/32\",\n" + " \"next-hop\": \"172.16.0.0\",\n" + " \"interface\": \"eth0\",\n" + " \"metric\": 0,\n" + " \"active\": [null]\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.1/32\",\n" + " \"next-hop\": \"172.16.0.1\",\n" + " \"interface\": \"eth1\",\n" + " \"metric\": 1\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.2/32\",\n" + " \"next-hop\": \"172.16.0.2\",\n" + " \"interface\": \"eth2\",\n" + " \"metric\": 2,\n" + " \"active\": [null]\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.3/32\",\n" + " \"next-hop\": \"172.16.0.3\",\n" + " \"interface\": \"eth3\",\n" + " \"metric\": 3\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.4/32\",\n" + " \"next-hop\": \"172.16.0.4\",\n" + " \"interface\": \"eth4\",\n" + " \"metric\": 4,\n" + " \"active\": [null]\n" + " },\n" + " {\n" + " \"prefix\": \"10.0.0.5/32\",\n" + " \"next-hop\": \"172.16.0.5\",\n" + " \"interface\": \"eth5\",\n" + " \"metric\": 5\n" + " }\n" + " ]\n" + " }\n" + " }\n" + " ]\n" + " },\n" + " \"c2cont\": {\n" + " \"c2value\": 2868969987\n" + " },\n" + " \"c3value\": 21\n" + " }\n" + "}\n"; + + +static const struct lyd_node *test_oper_get_tree_locked(const char *xpath) +{ + ++data_tree_lock; + return data_tree; +} + +static void test_oper_unlock_tree(const struct lyd_node *tree __attribute__((unused))) +{ + data_tree_lock--; +} + +static int __rpc_return_ok(struct nb_cb_rpc_args *args) +{ + return NB_OK; +} + +/* clang-format off */ +const struct frr_yang_module_info frr_test_module_info = { + .name = "frr-test-module", + .get_tree_locked = test_oper_get_tree_locked, + .unlock_tree = test_oper_unlock_tree, + .nodes = { + { + .xpath = "/frr-test-module:frr-test-module/vrfs/vrf/ping", + .cbs.rpc = __rpc_return_ok, + }, + { + .xpath = NULL, + }, + } +}; +/* clang-format on */ + +static const struct frr_yang_module_info *const modules[] = { + &frr_test_module_info, +}; + +static void vty_do_exit(int isexit) +{ + printf("\nend.\n"); + + lyd_free_all(data_tree); + + cmd_terminate(); + vty_terminate(); + nb_terminate(); + yang_terminate(); + event_master_free(master); + + log_memstats(NULL, true); + if (!isexit) + exit(0); +} + + +static struct lyd_node *load_data(void) +{ + struct ly_in *in = NULL; + struct lyd_node *tree = NULL; + LY_ERR err; + + err = ly_in_new_memory(data_json, &in); + if (!err) + err = lyd_parse_data(ly_native_ctx, NULL, in, LYD_JSON, LYD_PARSE_STRICT, LYD_VALIDATE_OPERATIONAL, &tree); + ly_in_free(in, 0); + if (err) { + fprintf(stderr, "LYERR: %s\n", getcwd(NULL, 0)); + fprintf(stderr, "LYERR: %s\n", ly_last_errmsg()); + exit(1); + } + return tree; +} + +/* main routine. */ +int main(int argc, char **argv) +{ + struct event thread; + + /* Set umask before anything for security */ + umask(0027); + + /* master init. */ + master = event_master_create(NULL); + + // zlog_aux_init("NONE: ", ZLOG_DISABLED); + + /* Library inits. */ + cmd_init(1); + cmd_hostname_set("test"); + vty_init(master, false); + lib_cmd_init(); + debug_init(); + nb_init(master, modules, array_size(modules), false, false); + + /* Create artificial data. */ + data_tree = load_data(); + + /* Read input from .in file. */ + vty_stdio(vty_do_exit); + + /* Fetch next active thread. */ + while (event_fetch(master, &thread)) + event_call(&thread); + + /* Not reached. */ + exit(0); +} diff --git a/tests/lib/northbound/test_oper_exists.in b/tests/lib/northbound/test_oper_exists.in new file mode 100644 index 0000000000..7b83c27a0b --- /dev/null +++ b/tests/lib/northbound/test_oper_exists.in @@ -0,0 +1,8 @@ +show yang operational-data /frr-test-module:frr-test-module +show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[2] +show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[3]/interface +show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[10] +show yang operational-data /frr-test-module:frr-test-module/c3value +show yang operational-data /frr-test-module:frr-test-module/c2cont +show yang operational-data /frr-test-module:frr-test-module/c2cont/ +show yang operational-data /frr-test-module:frr-test-module/c2cont/c2value diff --git a/tests/lib/northbound/test_oper_exists.py b/tests/lib/northbound/test_oper_exists.py new file mode 100644 index 0000000000..423414eb85 --- /dev/null +++ b/tests/lib/northbound/test_oper_exists.py @@ -0,0 +1,5 @@ +import frrtest + + +class TestNbOperData(frrtest.TestRefOut): + program = "./test_oper_exists" diff --git a/tests/lib/northbound/test_oper_exists.refout b/tests/lib/northbound/test_oper_exists.refout new file mode 100644 index 0000000000..4060a096fd --- /dev/null +++ b/tests/lib/northbound/test_oper_exists.refout @@ -0,0 +1,208 @@ +test# show yang operational-data /frr-test-module:frr-test-module +{ + "frr-test-module:frr-test-module": { + "vrfs": { + "vrf": [ + { + "name": "vrf0", + "interfaces": { + "interface": [ + "eth0", + "eth1", + "eth2", + "eth3" + ], + "interface-new": [ + "eth0", + "eth1", + "eth2", + "eth3" + ] + }, + "routes": { + "route": [ + { + "prefix": "10.0.0.0/32", + "next-hop": "172.16.0.0", + "interface": "eth0", + "metric": 0, + "active": [null] + }, + { + "prefix": "10.0.0.1/32", + "next-hop": "172.16.0.1", + "interface": "eth1", + "metric": 1 + }, + { + "prefix": "10.0.0.2/32", + "next-hop": "172.16.0.2", + "interface": "eth2", + "metric": 2, + "active": [null] + }, + { + "prefix": "10.0.0.3/32", + "next-hop": "172.16.0.3", + "interface": "eth3", + "metric": 3 + }, + { + "prefix": "10.0.0.4/32", + "next-hop": "172.16.0.4", + "interface": "eth4", + "metric": 4, + "active": [null] + }, + { + "prefix": "10.0.0.5/32", + "next-hop": "172.16.0.5", + "interface": "eth5", + "metric": 5 + } + ] + } + }, + { + "name": "vrf1", + "interfaces": { + "interface": [ + "eth0", + "eth1", + "eth2", + "eth3" + ], + "interface-new": [ + "eth0", + "eth1", + "eth2", + "eth3" + ] + }, + "routes": { + "route": [ + { + "prefix": "10.0.0.0/32", + "next-hop": "172.16.0.0", + "interface": "eth0", + "metric": 0, + "active": [null] + }, + { + "prefix": "10.0.0.1/32", + "next-hop": "172.16.0.1", + "interface": "eth1", + "metric": 1 + }, + { + "prefix": "10.0.0.2/32", + "next-hop": "172.16.0.2", + "interface": "eth2", + "metric": 2, + "active": [null] + }, + { + "prefix": "10.0.0.3/32", + "next-hop": "172.16.0.3", + "interface": "eth3", + "metric": 3 + }, + { + "prefix": "10.0.0.4/32", + "next-hop": "172.16.0.4", + "interface": "eth4", + "metric": 4, + "active": [null] + }, + { + "prefix": "10.0.0.5/32", + "next-hop": "172.16.0.5", + "interface": "eth5", + "metric": 5 + } + ] + } + } + ] + }, + "c2cont": { + "c2value": 2868969987 + }, + "c3value": 21 + } +} +test# show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[2] +{ + "frr-test-module:frr-test-module": { + "vrfs": { + "vrf": [ + { + "name": "vrf0", + "routes": { + "route": [ + { + "prefix": "10.0.0.1/32", + "next-hop": "172.16.0.1", + "interface": "eth1", + "metric": 1 + } + ] + } + } + ] + } + } +} +test# show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[3]/interface +{ + "frr-test-module:frr-test-module": { + "vrfs": { + "vrf": [ + { + "name": "vrf0", + "routes": { + "route": [ + { + "interface": "eth2" + } + ] + } + } + ] + } + } +} +test# show yang operational-data /frr-test-module:frr-test-module/vrfs/vrf[name='vrf0']/routes/route[10] +{} +test# show yang operational-data /frr-test-module:frr-test-module/c3value +{ + "frr-test-module:frr-test-module": { + "c3value": 21 + } +} +test# show yang operational-data /frr-test-module:frr-test-module/c2cont +{ + "frr-test-module:frr-test-module": { + "c2cont": { + "c2value": 2868969987 + } + } +} +test# show yang operational-data /frr-test-module:frr-test-module/c2cont/ +{ + "frr-test-module:frr-test-module": { + "c2cont": { + "c2value": 2868969987 + } + } +} +test# show yang operational-data /frr-test-module:frr-test-module/c2cont/c2value +{ + "frr-test-module:frr-test-module": { + "c2cont": { + "c2value": 2868969987 + } + } +} +test# +end. diff --git a/tests/lib/subdir.am b/tests/lib/subdir.am index 1a21684f16..ca74306543 100644 --- a/tests/lib/subdir.am +++ b/tests/lib/subdir.am @@ -131,6 +131,19 @@ EXTRA_DIST += \ # end +check_PROGRAMS += tests/lib/northbound/test_oper_exists +tests_lib_northbound_test_oper_exists_CFLAGS = $(TESTS_CFLAGS) +tests_lib_northbound_test_oper_exists_CPPFLAGS = $(TESTS_CPPFLAGS) +tests_lib_northbound_test_oper_exists_LDADD = $(ALL_TESTS_LDADD) +tests_lib_northbound_test_oper_exists_SOURCES = tests/lib/northbound/test_oper_exists.c +nodist_tests_lib_northbound_test_oper_exists_SOURCES = yang/frr-test-module.yang.c +EXTRA_DIST += \ + tests/lib/northbound/test_oper_exists.in \ + tests/lib/northbound/test_oper_exists.py \ + tests/lib/northbound/test_oper_exists.refout \ + # end + + check_PROGRAMS += tests/lib/test_assert tests_lib_test_assert_CFLAGS = $(TESTS_CFLAGS) tests_lib_test_assert_CPPFLAGS = $(TESTS_CPPFLAGS) diff --git a/yang/frr-test-module.yang b/yang/frr-test-module.yang index 773a959553..909c199b2f 100644 --- a/yang/frr-test-module.yang +++ b/yang/frr-test-module.yang @@ -139,5 +139,23 @@ module frr-test-module { } } } + choice bchoice { + description "a choice statement"; + case case3 { + leaf c3value { + type uint8; + description "A uint8 value for case 3"; + } + } + case case4 { + container c4cont { + description "case 2 container"; + leaf c4value { + type uint32; + description "A uint32 value for case 4"; + } + } + } + } } }