diff --git a/lib/northbound_oper.c b/lib/northbound_oper.c index 1e216247a5a6..0e88907767c1 100644 --- a/lib/northbound_oper.c +++ b/lib/northbound_oper.c @@ -21,7 +21,8 @@ DEFINE_MTYPE_STATIC(LIB, NB_YIELD_STATE, "NB Yield State"); DEFINE_MTYPE_STATIC(LIB, NB_XPATH, "NB XPath String"); /* Amount of time allowed to spend constructing oper-state prior to yielding */ -#define NB_OP_WALK_INTERVAL_US 10000 +#define NB_OP_WALK_INTERVAL_MS 50 +#define NB_OP_WALK_INTERVAL_US (NB_OP_WALK_INTERVAL_MS * 1000) /* ---------- */ /* Data Types */ @@ -32,10 +33,12 @@ DEFINE_MTYPE_STATIC(LIB, NB_XPATH, "NB XPath String"); */ struct nb_op_node_info { struct lyd_node_inner *inner; + const struct lysc_node *schema; /* inner schema in case we rm inner */ struct yang_list_keys keys; /* if list, keys to locate element */ const void *list_entry; /* opaque entry from user or NULL */ uint xpath_len; /* length of the xpath string for this node */ - uint nlist_ents; /* number of list elements created so far */ + uint niters; /* # list elems create this iteration */ + uint nents; /* # list elems create so far */ bool has_lookup_next : 1; /* if this node support lookup next */ bool lookup_next_ok : 1; /* if this and all previous support */ }; @@ -104,11 +107,11 @@ static inline void nb_op_free_yield_state(struct nb_op_yield_state *ys) } } -static struct lyd_node_inner *ys_base_node(struct nb_op_yield_state *ys) +static const struct lysc_node *ys_base_schema_node(struct nb_op_yield_state *ys) { if (ys->walk_root_level == -1) return NULL; - return ys->node_infos[ys->walk_root_level].inner; + return ys->node_infos[ys->walk_root_level].schema; } static struct lyd_node *ys_root_node(struct nb_op_yield_state *ys) @@ -159,11 +162,6 @@ static void nb_op_get_keys(struct lyd_node_inner *list_node, static enum nb_error __move_back_to_next(struct nb_op_yield_state *ys, int i) { struct nb_op_node_info *ni; - struct lyd_node_inner *parent; - struct nb_node *nn; - const void *list_entry; - enum nb_error ret; - LY_ERR err; for (; i >= ys->walk_root_level; i--) { if (ys->node_infos[i].has_lookup_next) @@ -173,32 +171,19 @@ static enum nb_error __move_back_to_next(struct nb_op_yield_state *ys, int i) return NB_ERR_NOT_FOUND; ni = &ys->node_infos[i]; - nn = ni->inner->schema->priv; - /* trim the tree */ - parent = i == 0 ? NULL : ni[-1].inner; + /* + * The i'th node has been lost after a yield so trim it from the tree + * now. + */ lyd_free_tree(&ni->inner->node); ni->inner = NULL; ni->list_entry = NULL; - list_entry = (i == 0 ? NULL : ni[-1].list_entry); - list_entry = nb_callback_lookup_next(nn, list_entry, &ni->keys); - if (!list_entry) - /* recurse if not found */ - return __move_back_to_next(ys, i - 1); - - /* get the keys of the new entry */ - ret = nb_callback_get_keys(nn, list_entry, &ni->keys); - if (ret) { - flog_warn(EC_LIB_NB_CB_STATE, "%s: failed to get list keys", - __func__); - return ret; - } - if (ni->keys.num != yang_snode_num_keys(nn->snode)) - return NB_ERR_INCONSISTENCY; - err = yang_lyd_new_list(parent, nn->snode, &ni->keys, &ni->inner); - if (err) - return NB_ERR_RESOURCE; + /* + * Leave the empty-of-data node_info on top, nb_op_walk will deal with + * this, by doing a lookup-next with the keys which we still have. + */ return NB_OK; } @@ -212,15 +197,32 @@ static enum nb_error nb_op_resume_data_tree(struct nb_op_yield_state *ys) uint i; /* - * Walk the rightmost branch from base to tip verifying lookup_next list - * nodes are still present. If not then we prune the branch and resume - * with the lookup_next and the keys from the old node. + * IMPORTANT: On yielding: we always yield after the initial list + * element has been created and handled, so the top of the yield stack + * will always point at a list node. + * + * Additionally, that list node has been processed and was in the + * process of being "get_next"d when we yielded. We process the + * lookup-next list node last so all the rest of the data (to the left) + * has been gotten. NOTE: To keep this simple we will require only a + * single lookup-next sibling in any parents list of children. + * + * Walk the rightmost branch from base to tip verifying all list nodes + * are still present. If not we backup to the node which has a lookup + * next, and we prune the branch to this node. If the list node that + * went away is the topmost we will be using lookup_next, but if it's a + * parent then our list_item was restored. + */ + + /* TODO: batching support, if we sent a batch during the previous yield, + * then we need to prune all list entries prior to the topmost one when + * restoring the walk. */ darr_foreach_i (ys->node_infos, i) { ni = &ys->node_infos[i]; - nn = ni->inner->schema->priv; + nn = ni->schema->priv; - if (CHECK_FLAG(ni->inner->schema->nodetype, LYS_CONTAINER)) + if (CHECK_FLAG(ni->schema->nodetype, LYS_CONTAINER)) continue; /* Verify the entry is still present */ @@ -342,6 +344,7 @@ static enum nb_error nb_op_init_data_tree(const char *xpath, ni = &ninfo[i - 1]; memset(ni, 0, sizeof(*ni)); ni->inner = inner; + ni->schema = inner->schema; /* * NOTE: we could build this by hand with a litte more effort, * but this simple implementation works and won't be expensive @@ -357,7 +360,7 @@ static enum nb_error nb_op_init_data_tree(const char *xpath, ni = &ninfo[i]; inner = ni->inner; - nn = inner->schema->priv; + nn = ni->schema->priv; ni->has_lookup_next = nn->cbs.lookup_next != NULL; ni->list_entry = i == 0 ? NULL : ni[-1].list_entry; @@ -622,7 +625,7 @@ static const struct lysc_node *nb_op_sib_next(const struct lysc_node *sib) */ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) { - struct lyd_node_inner *walk_base_node = ys_base_node(ys); + const struct lysc_node *walk_base_snode = ys_base_schema_node(ys); const struct lysc_node *sib; const void *parent_list_entry = NULL; const void *list_entry = NULL; @@ -638,7 +641,7 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) monotime(&ys->start_time); /* Don't currently support walking all root nodes */ - if (!walk_base_node) + if (!walk_base_snode) return NB_ERR_NOT_FOUND; /* @@ -648,15 +651,15 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) * walking, starting with non-yielding children. */ if (is_resume) - sib = darr_last(ys->node_infos)->inner->schema; + sib = darr_last(ys->node_infos)->schema; else if (ys->query_list_entry) - sib = walk_base_node->schema; + sib = walk_base_snode; else { /* Start with non-yielding children first. */ - sib = lysc_node_child(walk_base_node->schema); + sib = lysc_node_child(walk_base_snode); sib = __sib_next(false, sib); if (!sib) - sib = lysc_node_child(walk_base_node->schema); + sib = lysc_node_child(walk_base_snode); } char *xpath_child = NULL; @@ -680,7 +683,7 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) */ /* Grab the containing node. */ - sib = ni->inner->schema; + sib = ni->schema; if (sib->nodetype == LYS_CONTAINER) { /* If we added an empty container node (no @@ -758,7 +761,9 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) /* push this container node on top of the stack */ ni = darr_append(ys->node_infos); ni->inner = (struct lyd_node_inner *)node; - ni->nlist_ents = 0; + ni->schema = node->schema; + ni->niters = 0; + ni->nents = 0; ni->has_lookup_next = false; ni->lookup_next_ok = ni[-1].lookup_next_ok; ni->list_entry = ni[-1].list_entry; @@ -771,7 +776,11 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) continue; case LYS_LIST: - list_start = ni->inner->schema != sib; + /* + * ni->inner may be NULL here if we resumed and it was + * gone. ni->schema and ni->keys will still be valid. + */ + list_start = ni->schema != sib; if (list_start) { /* * Our node info wasn't on top so this is a new @@ -825,19 +834,29 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) goto query_entry_skip_keys; } } else if (ni && ni->lookup_next_ok && - (monotime_since(&ys->start_time, NULL) > - NB_OP_WALK_INTERVAL_US || - ni->nlist_ents > 1)) { + // make sure we advance, if the interval is + // fast and we are very slow. + ((monotime_since(&ys->start_time, NULL) > + NB_OP_WALK_INTERVAL_US && + ni->niters) || + (ni->niters + 1) % 10000 == 0)) { /* This is a yield supporting list node and * we've been running at least our yield * interval, so yield. + * + * NOTE: we never yield on list_start, and we + * are always about to be doing a get_next. */ - ni->nlist_ents = 0; + DEBUGD(&nb_dbg_events, + "%s: yielding after %u iterations", + __func__, ni->niters); + + ni->niters = 0; return NB_YIELD; } else if (!list_start && !list_entry && ni->has_lookup_next) { /* We don't have the previous object, but we have - * the previous keys + * the previous keys, lookup next. */ list_entry = nb_callback_lookup_next(nn, @@ -850,6 +869,7 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) parent_list_entry, list_entry); } + if (!list_entry) { /* End of list iteration. */ if (ys->query_list_entry && at_base_level) { @@ -861,9 +881,11 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) sib = nb_op_sib_next(sib); /* Pop the list node up to our parent, but - * only if we've already pushed the current list - * node; for `list_start` this hasn't happened - * yet, as it happens below. + * only if we've already pushed a node for the + * current list schema. For `list_start` this + * hasn't happened yet, as would have happened + * below. So basically we processed an empty + * list. */ if (!list_start) ys_pop_inner(ys); @@ -888,7 +910,7 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) } /* Move on to the sibling of the list node */ - break; + continue; } if (list_start) { @@ -903,7 +925,8 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) ni->lookup_next_ok = ((!pni && ys->finish) || pni->lookup_next_ok) && ni->has_lookup_next; - ni->nlist_ents = 0; + ni->niters = 0; + ni->nents = 0; /* this will be our predicate-less xpath */ darr_in_strcat(ys->xpath, "/"); @@ -941,11 +964,11 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) &ni->keys, darr_cap(ys->xpath) - len); } else { - /* add a position predicate */ + /* add a position predicate (1s based?) */ darr_ensure_avail(ys->xpath, 10); snprintf(ys->xpath + len, darr_cap(ys->xpath) - len + 1, "[%u]", - ni->nlist_ents + 1); + ni->nents + 1); } darr_setlen(ys->xpath, strlen(ys->xpath + len) + len + 1); @@ -954,6 +977,11 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) /* * Create the new list entry node. */ +#if 0 + err = lyd_new_list2(&ni[-1].inner->node, sib->module, + sib->name, ys->xpath + len, false, + &node); +#endif err = yang_lyd_new_list(ni[-1].inner, sib, &ni->keys, (struct lyd_node_inner **)&node); if (err) { @@ -965,8 +993,10 @@ static enum nb_error nb_op_walk(struct nb_op_yield_state *ys, bool is_resume) * Save the new list entry with the list node info */ ni->inner = (struct lyd_node_inner *)node; + ni->schema = node->schema; ni->list_entry = list_entry; - ni->nlist_ents += 1; + ni->niters += 1; + ni->nents += 1; /* Skip over the key children, they've been created. */ query_entry_skip_keys: