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

Instance-id predicates compilation fails due to difference in xpath predicates and keys count #2306

Open
optimden opened this issue Oct 3, 2024 · 1 comment
Labels
is:question Issue is actually a question.

Comments

@optimden
Copy link
Contributor

optimden commented Oct 3, 2024

Hello!
We face one issue in the following situation.
First of all some information about versions of libnetconf2 related libraries which we use:
libnetconf2 - 2.1.38 (b933667bc55f46a36f2d721c8a5ce2966bccbbdb)
netopeer - 2.1.49 (b1f7bfda930aa2cf5e4759aefed80dedbd56b94b)
sysrepo - 2.2.33 (a1075ab5a5563f4acc21218ac861b4e8e20457b9)
libyang - 2.1.144 (eecad55)

We form alarm-notification (based on [email protected]) in our plugin for sysrepo which must look like (when is received in MgSoft NETCONF Browser, for example):

<alarm-notification xmlns="urn:ietf:params:xml:ns:yang:ietf-alarms">
    <resource xmlns:if="urn:ietf:params:xml:ns:yang:ietf-interfaces"
          xmlns:bbf-moca25access="urn:bbf:yang:bbf-moca25access"
          xmlns:bbf-moca="urn:bbf:yang:bbf-moca-base">/if:interfaces-state/if:interface[if:name='en1']/bbf-moca25access:line/bbf-moca:line/bbf-moca:status/bbf-moca:nc-status/bbf-moca:associated-device[bbf-moca:node-id='1']
    </resource>
    <alarm-type-id xmlns:bbf-moca-alt="urn:bbf:yang:bbf-moca-alarm-types">bbf-moca-alt:moca-cn-link-alarm</alarm-type-id>
    <time>some time</time>
    <perceived-severity>minor</perceived-severity>
    <alarm-text>some alarm text</alarm-text>
</alarm-notification>"

But actually it looks like:

<alarm-notification xmlns="urn:ietf:params:xml:ns:yang:ietf-alarms">
    <resource>/ietf-interfaces:interfaces-state/interface[name='en1']/bbf-moca25access:line/bbf-moca:line/status/nc-status/associated-device[node-id='1']</resource>
    <alarm-type-id xmlns:bbf-moca-alt="urn:bbf:yang:bbf-moca-alarm-types">bbf-moca-alt:moca-cn-link-alarm</alarm-type-id>
    <time>some time</time>
    <perceived-severity>minor</perceived-severity>
    <alarm-text>some text</alarm-text>
</alarm-notification>"

So, no prefixes have been substituted in resource node xpath. And also resource node hasn't xmlns attributes.
I would like to note that we haven't any deviations for resource node from the mentioned yang.

Our code looks like this from the bird eyes:

// For each required node of alarm notification:
lyd_new_path(notification, NULL, path, value, 0, &node);

// So, after we accumulated all `struct lyd_node *`s in notification, we send notification to the client through netopeer/libnetconf2
sr_notif_send_tree(sess, notification, 0, 0);

The next thing goes wrong during creating new path for resource node (as i see).
Resource node is union which can be one from instance-identifier, object-identifier, string, uuid.
Libyang stacktrace which leads to the problematic place (as i found it):

       lyd_new_path
		|
	lyd_new_path_
		|
	lyd_create_term
		|
	lyd_value_store
		|
	lyplg_type_store_union
	        |
        union_find_type // libyang iterates over all types under the hood of union
                |
        union_store_type // it tries to save this node data as finding type
                |
                 ---> lyplg_type_store_instanceid // in my case it's the first type which libyang tries from the list of union types
                                              |
                            lyplg_type_lypath_new
                                              |
                                   ly_path_compile
                                              |
                                _ly_path_compile
                                              |
                           ly_path_compile_predicate    <--- problem occurs here

Libyang tries determine the first applicable type from this list for the current xpath. Actually, it must be instance-id, but libyang fails to store this xpath as instance-id and stores it as string (the next applicable type from the list of types).

The reason to storing it as instance-id is that predicates compilation fails due to difference in xpath predicates and keys count.
Libyang splits original xpath:

/ietf-alarms:alarm-notification/resource, value = /ietf-interfaces:interfaces-state/interface[name='en1']/bbf-moca25access:line/bbf-moca:line/status/nc-status/associated-device[node-id='1']

To two path segments, and handles them separately (compiles predicates for them). But the problem occurs when it handles the second part of this xpath. It finds that there is only one predicate:

associated-device[node-id='1']

But when it counts quantity of xpath's keys it iterates over all keys in xpath, not over keys in current path segment.
So,

LY_ERR
ly_path_compile_predicate(const struct ly_ctx *ctx, const struct lysc_node *cur_node, const struct lys_module *cur_mod,
        const struct lysc_node *ctx_node, const struct lyxp_expr *expr, uint32_t *tok_idx, LY_VALUE_FORMAT format,
        void *prefix_data, struct ly_path_predicate **predicates)
{
       ...
       if (expr->tokens[*tok_idx] == LYXP_TOKEN_NAMETEST) {
       ...
            /* check that all keys were set */
            key_count = 0;
            for (key = lysc_node_child(ctx_node); key && (key->flags & LYS_KEY); key = key->prev) {
                ++key_count;
            }
            if (LY_ARRAY_COUNT(*predicates) != key_count) {    <--- we proceed to this condition with
           
           1 predicate and 2 keys:
               name - from the first xpath predicate - interface[name='en1']
               node-id - from currently handled path segment's predicate - associated-device[node-id='1']

                /* names (keys) are unique - it was checked when parsing */
                LOGVAL(ctx, LYVE_XPATH, "Predicate missing for a key of %s \"%s\" in path.",
                    lys_nodetype2str(ctx_node->nodetype), ctx_node->name);
                ly_path_predicates_free(ctx, *predicates);
                *predicates = NULL;
                ret = LY_EVALID;
                goto cleanup;
            }
    ...

I tried dirty solution with iterating over keys in a different direction in function above (key->prev instead of key->next). It worked for sending notification and this notification showed with all required resource node attributes and prefixes substituted into xpath. Meanwhile it worked when all sysrepo initial steps with installing and filling modules have been passed. If use this change when sysrepo and netopeer start first time and there aren't any sysrepo files already presented in /etc/sysrepo folder, sysrepo and netopeer won't start at all. I understand that it's dirty and wrong change, so i would like to consult with you of how can i fix this problem properly.

If some of my conclusions are wrong or i missed something important, please write me to correct.

@michalvasko
Copy link
Member

The spec is quite clear on this, you must uniquely identify the instance, it is not possible without all the list keys for every list in the value. So the proper solution is to include the value for all the keys.

@michalvasko michalvasko added the is:question Issue is actually a question. label Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:question Issue is actually a question.
Projects
None yet
Development

No branches or pull requests

2 participants