Subject: Re: [netmod] LL review of draft-ietf-netmod-rfc6020bis-02

From: Jernej Tuljak <[email-masked]>
Date: Mon, 01 Dec 2014 09:09:05 +0100

Dne 28.11.2014 12:29, piše Martin Bjorklund:
> Lada,
>
> Thank you for this review!  Comments inline.
>
>
> Ladislav Lhotka <[email-masked]> wrote:
>> Hi,
>>
>> below are my comments to YANG 1.1 draft.
>>
>> Lada
>>
>> **** Sec. 4.2.7
>>       I think this is incorrect terminology: "When a node from one case
>>       is created, ..." - sec. 3 defines "data node" as a node in the
>>       schema tree. It should be "When a data node instance from one
>>       case is created, ...". Sec. 3 should probably also define what
>>       "instance" means.
> (This text hasn't changed from RFC 6020)
>
> How about: "When a node from one case is created in the data tree,
> ..."?
>
>
>> **** Sec. 4.2.2.2
>>       OLD
>>           A leaf-list is a sequence of leaf nodes with exactly one value of a
>> 	 particular type per leaf.
>>       NEW
>>           A leaf-list instance is a sequence of values of a particular type.
> (This text hasn't changed from RFC 6020)
>
> How about using similar language as in 4.2.2.4 for lists: "A leaf-list
> defines a sequence of values of a particular type."
>
>
>> **** Sec. 7.5.3
>>       The issue Y41 is marked as REVIEW, so I'd expect some text in
>>       this section explaining how "must" expressions on NP-containers
>>       are evaluated.
> Yes you are right.  Today there is actually some duplication of text
> between section 6.4.1 and 7.5.3.  Instead of adding to the
> duplication, I suggest we do:
>
> OLD:
>
>     When a datastore is validated, all "must" constraints are
>     conceptually evaluated once for each data node in the data tree, and
>     for all leafs with default values in use (see Section 7.6.1).  If a
>     data node does not exist in the data tree, and it does not have a
>     default value, its "must" statements are not evaluated.
>
> NEW:
>
>     When a datastore is validated, all "must" constraints are conceptually
>     evaluated once for each data node in the accessible tree (see
>     Section 6.4.1).
>
>
>> **** Sec. 7.19.5
>>       The second paragraph should say that a list key MUST NOT have a
>>       "when" statement. Such a statement has a different context node
>>       than a "when" statement on its ancestor, so it could break even
>>       if the two statements are identical.
> Ok, I agree.  Phil made the same comment at the mic in Honolulu, so
> unless anyone objects, I'll make this change.
>
>
>> **** Sec. 9.10.3
>>       In the last paragraph, what is "the prefix"? It depends on
>>       namespace declarations that are in effect, and there can even be
>>       multiple prefixes declared for the same URI.
>>
>>       Therefore, I think the string value has to be the exact value of
>>       the node (with no prefix if there is none).
> This would break modules like ietf-system, which use the
> <prefix>:<name> value.
>
> The idea is that the prefix is the prefix used in the import
> statement.
>
> NEW:
>
>     The string value of a node of type identityref in a "must" or
>     "when" XPath expression is the referred identity's qualified name
>     with the prefix present.  If the referred identity is defined in an
>     imported module, the prefix in the string value is the prefix
>     defined in the corresponding "import" statement.  Otherwise, the
>     prefix in the string value is the prefix for the current module.
>
>
>>       Consequently,
>>       comparison of identities in XPath expressions should always be
>>       performed via special XPath functions.
>> **** Sec. 9.10.5
>>       The "when" expression in the example should be
>>
>>       when 'derived-from-or-self(../crypto, "my-crypto", "aes")';
>>
>>       (see also my comment to sec. 10.4.1 below).
> See above.
>
>> **** Sec. 9.12
>>       The validation procedure is too XML-centric. The value of a union
>>       instance needn't be a string.
>>
>>       OLD
>>           When a string representing a union data type is validated,
>>           the string is validated against each member type, ...
>>       NEW
>>           A value representing a union data type is validated
>>           consecutively against each member type, ...
> Ok, fixed.
>
>> **** Sec. 10
>>       - The first paragraph should say that function signatures
>>         are specified in the same form as in XPath 1.0 spec.
> Ok.
>
>>       - Titles of subsections 10.1 and 10.2 could be "Node Set
>>         Functions" and "String Functions" (XPath spec uses these
>>         titles).
> But then it wouldn't be consistent with the other subsections...
>
>> ***** Sec. 10.1
>>        OLD
>>            The function current() takes no input parameters, and
>>            returns a node set with the initial context node.
>>        NEW
>>            The function current() takes no input parameters, and
>>            returns a node-set that has the context node as its only
>>            member.
> Ok, fixed.
>
>> ***** Sec. 10.3.1
>>        Instead of the deref() function I'd suggest to define a more
>>        general function evaluate(), that already exists in EXSLT, see
>>        http://www.exslt.org/dyn/functions/evaluate/:
>>
>>        object evaluate(string expr)
>>
>>        The evaluate() function takes a string, evaluates it as an XPath
>>        expression and returns the resulting value, which may be a
>>        node-set, string, number or boolean.
> I would like to hear other peoples opinion on this.

This would require implementations to implement XPath 1.0. If XPath is 
still just "used as a notation" in YANG 1.1, I don't think this would be 
acceptable. You don't need a full XPath 1.0 implementation to evaluate 
leafref paths or instance identifiers, but arbitrary expressions decided 
at runtime are a whole different kind of beast.

With 'evaluate()' the following text from 6.4 becomes impossible to achieve:

    An implementation may choose to implement them
    by hand, rather than using the XPath expression directly.

You can't implement a dynamic XPath expression by hand since it is not 
part of a model at implementation time.

@Ladislav, just to make sure - you are suggesting this, right?

augment "/path/to/some/container" {
when "evaluate(../../foo)"; // evaluates string-value of an instantiated 
data node
}

...
<foo>../foo2 = 'dark magic'</foo>
...

Jernej

>
>> ***** Sec. 10.4.1
>>        A similar function that's also needed is derived-from-or-self().
> Ok, added.
>
>>        It would be needed in example 10.4.1.1 for doing things like
>>
>>        augment "/interface" {
>>            when 'derived-from-or-self(type,
>> 	                     "example-interface",
>> 			     "fast-ethernet")';
>>            // fast-ethernet-specific definitions here
>>        }
>>        Alternatively, we could state in YANG 1.1 that derivation of
>>        identities is a reflexive relation, and then derived-from()
>>        would probably suffice.
>> ***** Sec. 10.6.1
>>        Maybe bit() is a better name than bit-is-set()?
> I prefer a more descriptive name than just "bit".
>
> is-bit-set()
> bit-set() -- not good, sounds like it might set the bit
> bit-set-p()
> bit-is-set()
>
>
>> ***** Sec. 13
>>        I don't know how how important it is to have the ABNF
>>        unambiguous. Previously, Jernej pointed out that refine-stmt
>>        makes the grammar ambiguous:
>>
>>        http://www.ietf.org/mail-archive/web/netmod/current/msg07740.html
>>        The if-feature-expr production is another source of
>>        ambiguity.
> Why is it ambiguous?
>
>> An unambiguous set of productions could look like
>>        this:
>>
>>        if-feature-expr = if-feature-disj
>>        if-feature-disj = if-feature-conj
>>        if-feature-conj = identifier-ref-arg
>>        if-feature-expr = if-feature-expr 'or' if-feature-disj
>>        if-feature-disj = if-feature-disj 'and' if-feature-conj
>>        if-feature-conj = 'not' if-feature-conj / '(' if-feature-expr ')'
> Hmm, this can't be right; you're defining each LHS twice.
>
>
> /martin
>
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

---

From: Ladislav Lhotka <[email-masked]>
Date: Tue, 02 Dec 2014 12:03:57 +0100

Jernej Tuljak <[email-masked]> writes:

...

> at runtime are a whole different kind of beast.

That's a fair point, but my motivation was to make it simpler for
implementations that *do* use standard XPath libraries that support the
evaluate function, such as libxslt or Saxon.

> part of a model at implementation time.

I don't see any difference here - the XPath expressions appearing in
"must" or "when" have to be evaluated against an instance document,
i.e. at runtime.

> <foo>../foo2 = 'dark magic'</foo>

Yes, but again I don't think it is that much different from

when "deref(../../foo)";

and

<foo>../dark/magic</foo>

Lada

-- 
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: E74E8C0C

---

From: Andy Bierman <[email-masked]>
Date: Tue, 2 Dec 2014 11:52:54 -0800

...

>> I would like to hear other peoples opinion on this.

I agree with Jernej.
Just to make sure I understand...
You want to use nodes in the datastore to hold XPath expressions
set by the client, that get evaluated inside of a must/when evaluation?
So the evaluate(../../foo) expression below would read the value of
the foo leaf and evaluate that string as an XPath expression?

I prefer the current deref function.
It is clear, constrained, and does not require a customized XPath parser,
or any XPath parser, to implement in a server.

Andy

> This would require implementations to implement XPath 1.0. If XPath is still
> just "used as a notation" in YANG 1.1, I don't think this would be
> leafref paths or instance identifiers, but arbitrary expressions decided at
> runtime are a whole different kind of beast.
>
> You can't implement a dynamic XPath expression by hand since it is not part
> of a model at implementation time.
>

---

From: Ladislav Lhotka <[email-masked]>
Date: Tue, 2 Dec 2014 21:30:26 +0100

> On 02 Dec 2014, at 20:52, Andy Bierman <[email-masked]> wrote:
> 

Yes, this is what these existing extensions do:

- http://www.exslt.org/dyn/functions/evaluate/
- http://www.saxonica.com/documentation9.4-demo/html/extensions/functions/evaluate.html

The same function is now also in XSLT 3.0 standard:

- http://www.w3.org/TR/xslt-30/#element-evaluate

> or any XPath parser, to implement in a server.

OK. Lada