Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix oper-state queries that involve choice/case nodes (backport #18231) #18233

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 48 additions & 41 deletions lib/northbound_oper.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ nb_op_create_yield_state(const char *xpath, struct yang_translator *translator,

ys = XCALLOC(MTYPE_NB_YIELD_STATE, sizeof(*ys));
ys->xpath = darr_strdup_cap(xpath, (size_t)XPATH_MAXLEN);
/* remove trailing '/'s */
while (darr_len(ys->xpath) > 1 && ys->xpath[darr_len(ys->xpath) - 2] == '/') {
darr_setlen(ys->xpath, darr_len(ys->xpath) - 1);
*darr_last(ys->xpath) = 0;
}
ys->xpath_orig = darr_strdup(xpath);
ys->translator = translator;
ys->flags = flags;
Expand Down Expand Up @@ -395,8 +400,7 @@ static enum nb_error nb_op_ys_finalize_node_info(struct nb_op_yield_state *ys,
/* Assert that we are walking the rightmost branch */
assert(!inner->parent || inner == inner->parent->child->prev);

if (CHECK_FLAG(inner->schema->nodetype,
LYS_CASE | LYS_CHOICE | LYS_CONTAINER)) {
if (CHECK_FLAG(inner->schema->nodetype, LYS_CONTAINER)) {
/* containers have only zero or one child on a branch of a tree */
inner = ((struct lyd_node_inner *)inner)->child;
assert(!inner || inner->prev == inner);
Expand Down Expand Up @@ -511,6 +515,13 @@ static enum nb_error nb_op_ys_init_node_infos(struct nb_op_yield_state *ys)
* -- save the prefix length.
*/
inner = node;
<<<<<<< HEAD
=======
prevlen = 0;
xplen = strlen(xpath);
darr_free(ys->xpath);
ys->xpath = xpath;
>>>>>>> b9ff10b08 (lib: nb: fix oper-state queries that involve choice/case nodes)
for (i = len; i > 0; i--, inner = &inner->parent->node) {
ni = &ys->node_infos[i - 1];
ni->inner = inner;
Expand Down Expand Up @@ -818,8 +829,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).
*/
if (last != NULL &&
!CHECK_FLAG(last->schema->nodetype, LYS_CASE | LYS_CHOICE))
if (last != NULL)
assert(last->schema == parent);
if (darr_lasti(ys->node_infos) < ys->query_base_level)
return ys->schema_path[darr_lasti(ys->node_infos) + 1];
Expand Down Expand Up @@ -896,7 +906,8 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
if (!walk_stem_tip)
return NB_ERR_NOT_FOUND;

if (ys->schema_path[0]->nodetype == LYS_CHOICE) {
if (ys->schema_path[0]->parent &&
CHECK_FLAG(ys->schema_path[0]->parent->nodetype, LYS_CHOICE|LYS_CASE)) {
flog_err(EC_LIB_NB_OPERATIONAL_DATA,
"%s: unable to walk root level choice node from module: %s",
__func__, ys->schema_path[0]->module->name);
Expand Down Expand Up @@ -1003,8 +1014,12 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
LYS_LEAF | LYS_LEAFLIST | LYS_CONTAINER))
xpath_child = nb_op_get_child_path(ys->xpath, sib,
xpath_child);
else if (CHECK_FLAG(sib->nodetype, LYS_CASE | LYS_CHOICE))
else if (CHECK_FLAG(sib->nodetype, LYS_CASE | LYS_CHOICE)) {
darr_in_strdup(xpath_child, ys->xpath);
len = darr_last(ys->node_infos)->xpath_len;
darr_setlen(xpath_child, len + 1);
xpath_child[len] = 0;
}

nn = sib->priv;

Expand Down Expand Up @@ -1621,22 +1636,22 @@ static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys,
* NOTE: appears to be a bug in nb_node linkage where parent can be NULL,
* or I'm misunderstanding the code, in any case we use the libyang
* linkage to walk which works fine.
*
* XXX: we don't actually support choice/case yet, they are container
* types in the libyang schema, but won't be in data so our length
* checking gets messed up.
*/
for (sn = nblast->snode, count = 0; sn; count++, sn = sn->parent)
for (sn = nblast->snode, count = 0; sn; sn = sn->parent) {
if (sn != nblast->snode)
assert(CHECK_FLAG(sn->nodetype,
LYS_CONTAINER | LYS_LIST |
LYS_CHOICE | LYS_CASE));
LYS_CONTAINER | LYS_LIST | LYS_CHOICE | LYS_CASE));
if (!CHECK_FLAG(sn->nodetype, LYS_CHOICE | LYS_CASE))
count++;
}
/* 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;
for (sn = nblast->snode; sn; sn = sn->parent) {
if (!CHECK_FLAG(sn->nodetype, LYS_CHOICE | LYS_CASE))
ys->schema_path[--count] = sn;
}

/*
* Now tokenize the query string and get pointers to each token
Expand All @@ -1655,50 +1670,42 @@ static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys,
int nlen = strlen(name);
int mnlen = 0;

/*
* Technically the query_token for choice/case should probably be pointing at
* the child (leaf) rather than the parent (container), however,
* we only use these for processing list nodes so KISS.
*/
if (CHECK_FLAG(ys->schema_path[i]->nodetype,
LYS_CASE | LYS_CHOICE)) {
ys->query_tokens[i] = ys->query_tokens[i - 1];
continue;
}

s2 = s;
while (true) {
s2 = strstr(s, name);
/* skip past any module name prefix */
s2 = strstr(s2, name);
if (!s2)
goto error;

if (s2[-1] == ':') {
if (s2 > s && s2[-1] == ':') {
mnlen = strlen(modname) + 1;
if (ys->query_tokstr > s2 - mnlen ||
strncmp(s2 - mnlen, modname, mnlen - 1))
goto error;
if (s2 - s < mnlen || strncmp(s2 - mnlen, modname, mnlen - 1)) {
/* No match of module prefix, advance and try again */
s2 += strlen(name);
continue;
}
s2 -= mnlen;
nlen += mnlen;
}

s = s2;
if ((i == 0 || s[-1] == '/') &&
(s[nlen] == 0 || s[nlen] == '[' || s[nlen] == '/'))
if ((i == 0 || s2[-1] == '/') &&
(s2[nlen] == 0 || s2[nlen] == '[' || s2[nlen] == '/')) {
s = s2;
break;
/*
* Advance past the incorrect match, must have been
* part of previous predicate.
*/
s += nlen;
}
/* No exact match at end, advance and try again */
s2 += strlen(name);
}

/* NUL terminate previous token and save this one */
if (i > 0)
if (i > 0) {
assert(s[-1] == '/');
s[-1] = 0;
}
ys->query_tokens[i] = s;
s += nlen;
}

/* NOTE: need to subtract choice/case nodes when these are supported */
ys->query_base_level = darr_lasti(ys->schema_path);

return NB_OK;
Expand Down
10 changes: 10 additions & 0 deletions tests/lib/northbound/test_oper_data.in
Original file line number Diff line number Diff line change
@@ -1,2 +1,12 @@
show yang operational-data /frr-test-module:frr-test-module
<<<<<<< HEAD
=======
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/c2cont
show yang operational-data /frr-test-module:frr-test-module/c2cont/
show yang operational-data /frr-test-module:frr-test-module/c2cont/c2value
>>>>>>> 73df7da40 (tests: add unit-test for choice/case node queries)
test rpc
76 changes: 76 additions & 0 deletions tests/lib/northbound/test_oper_data.refout
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,82 @@ test# show yang operational-data /frr-test-module:frr-test-module
}
}
}
<<<<<<< HEAD
=======
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/c1value
{
"frr-test-module:frr-test-module": {
"c1value": 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
}
}
}
>>>>>>> 73df7da40 (tests: add unit-test for choice/case node queries)
test# test rpc
vrf testname data testdata
test#
Expand Down
Loading