Skip to content

Commit

Permalink
lib: bring in good fixes from broken branch
Browse files Browse the repository at this point in the history 
Signed-off-by: Christian Hopps <[email protected]>
  • Loading branch information
@choppsv1
choppsv1 committed Oct 30, 2023
1 parent 9e5e0e6 commit 6911d17
Showing 1 changed file with 86 additions and 58 deletions.
144 changes: 86 additions & 58 deletions lib/northbound_oper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 10
#define NB_OP_WALK_INTERVAL_US (NB_OP_WALK_INTERVAL_MS * 1000)

/* ---------- */
/* Data Types */
Expand All @@ -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 */
};
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand All @@ -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 */
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

/*
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -825,19 +834,27 @@ 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)) {
// XXX: ni->has_lookup_next &&
// XXX restore this after testings: ni->lookup_next_ok &&
// XXX: should have something like && > 1 to
// 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)
// (monotime_since(&ys->start_time, NULL) > NB_OP_WALK_INTERVAL_US ||
(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;
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,
Expand All @@ -850,6 +867,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) {
Expand All @@ -861,9 +879,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);
Expand All @@ -888,7 +908,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) {
Expand All @@ -903,7 +923,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, "/");
Expand Down Expand Up @@ -941,11 +962,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);
Expand All @@ -954,6 +975,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) {
Expand All @@ -965,8 +991,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:
Expand Down

0 comments on commit 6911d17

Please sign in to comment.