Subject: [netmod] instance-identifier bugs

From: Andy Bierman <[email-masked]>
Date: Mon, 02 Feb 2009 07:38:49 -0800

Hi,

 From 9.13, para 2:

    The syntax for an instance-identifier is a subset of the XPath
    syntax, which is used to uniquely identify a node in the data tree.
    It is an absolute XPath location path in abbreviated syntax, where
    axes are not permitted, and predicates are used only for specifying
    the values for the key nodes for list entries, or a value of a leaf-
    list.

This text is correct.

 From 9.13, para 4:

    The syntax is formally defined by the rule "absolute-instid" in
    Section 12.

This text is not correct.
The rule 'instance-identifier-str' in the ABNF defines the syntax.

The ABNF in sec. 12 is not correct:

instance-identifier-str
                        = < a string which matches the rule
                            instance-identifier >

instance-identifier    = absolute-instid / relative-instid

absolute-instid        = 1*("/" (node-identifier *predicate))

relative-instid        = descendant-instid /
                          (("." / "..") "/"
                           *relative-instid)

descendant-instid      = node-identifier *predicate
                          absolute-instid

predicate              = "[" *WSP predicate-expr *WSP "]"

predicate-expr         = (node-identifier / ".") *WSP "=" *WSP
                          ((DQUOTE string DQUOTE) /
                           (SQUOTE string SQUOTE))

Relative instance-identifiers do not make any sense.
The whole point of this exercise is to have a context-free
NETCONF database node identifier.  The text in 9.13, para 2
says this is an absolute instance-identifier.

Corrected ABNF:

instance-identifier-str
                        = < a string which matches the rule
                            instance-identifier >

instance-identifier    = absolute-instid

absolute-instid        = 1*("/" (node-identifier *predicate))

predicate              = "[" *WSP predicate-expr *WSP "]"

predicate-expr         = node-identifier *WSP "=" *WSP
                          ((DQUOTE string DQUOTE) /
                           (SQUOTE string SQUOTE))

(BTW, please add some indentation to the ABNF,
instead of starting on column 1).

Andy

_______________________________________________
netmod mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netmod

---

From: Andy Bierman <[email-masked]>
Date: Mon, 02 Feb 2009 07:46:32 -0800

....
>                          ((DQUOTE string DQUOTE) /

I took out '.' by mistake.
It is needed for a leaf-list instance-identifier.

The old 'predicate-expr' was correct.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Tue, 03 Feb 2009 07:59:44 -0800

...
>                          ((DQUOTE string DQUOTE) /

Why is only one form of a PrimaryExpr allowed here (literal)?
What about 'number' results?

XPath:

    /foo/bar[index=7]

YANG:

   /foo/bar[index='7']

There are significant differences between the 2 expressions.
(Read XPath, 3.4 for fun ;-)

Also, somebody familiar with XPath might think numbers
should be OK, instead of 'invalid-value'.

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 03 Feb 2009 23:08:22 +0100 (CET)

> The ABNF in sec. 12 is not correct:

[...]

> says this is an absolute instance-identifier.

Ok, but strictly speaking this falls under the category of "correct
but confusing" - the text *does* reference the absolut form, and the
relative form is not used.

I will make the changes you suggest, but I will reduce it some more
to:

  instance-identifier    = 1*("/" (node-identifier *predicate))

and then let the text refer to 'instance-identifier'.

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 05 Feb 2009 20:57:40 +0100 (CET)

> (Read XPath, 3.4 for fun ;-)

Yes, there is a difference, and the difference works in favor for
using literals - when literals are used, exact string comparison is
done, but with numbers, 64 bit floating point comparison is done.  So
with this data:

  <foo>
    <bar>
      <index>922337203685477581</index>
    </bar>
  </foo>

The expression:  /foo/bar[index=922337203685477580] will match the
instance, but the expression /foo/bar[index='922337203685477580']
will not match.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Thu, 05 Feb 2009 13:16:27 -0800

> instance, but the expression /foo/bar[index='922337203685477580']

Well, there are are plenty or cornser cases I guess.
Using a string literal, then '1' and '1.0' do not match either.
I find the limited subset of XPath used by YANG to be
somewhat arbitrary, and that makes it confusing.

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 05 Feb 2009 22:42:25 +0100 (CET)

> somewhat arbitrary, and that makes it confusing.

Do you mean "used by YANG in the instance-identifier expressions"?

Would it be less arbitrary and confusing if we allow numbers?  I don't
have a problem with that.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Thu, 05 Feb 2009 17:06:40 -0800

> Do you mean "used by YANG in the instance-identifier expressions"?

I prefer to minimize all the little differences.
It is better to allow both literal and number in leafref
and instance-identifier, and include some text that says
where literal is better than number and vice versa.
Let the data modeller decide what to do.  Number is
only unsafe for RelationalExpr, for int64 and uint64.
Why forbid its use for data types that will barely get used?
A warning that only safe expressions SHOULD be used is good enough.

One more big leafref/instance-identifier open issue I just
thought of.  The text seems to imply that the require-instance
check is applied when the value is created.  This is of course
not the case for <candidate/>, only for <running/>.
We must not introduce PDU order dependencies,
that we have been careful to avoid so far.

There should be some text in the Constraints section
that says leafref/instance-identifier 'require-instance'
check (and leafref validation if it is false) is performed
prior to 'commit' for <candidate/>, or right away for <running/>.

There could be <edit-config> PDUs where one subtree creates a
leafref or instance-identifier that points off to another part
of the config that is getting deleted or modified
in the same PDU. The config could be changed by later PDUs,
such that the <commit> will eventually fail because the
leafref/instance-identifier set in the first PDU is no longer valid.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Thu, 05 Feb 2009 17:21:21 -0800

>> Do you mean "used by YANG in the instance-identifier expressions"?

I changed my mind about numbers in leafref/instance-identifier
predicates.  These path expressions are not really XPath,
they are YANG.  The numbers are YANG numbers, not IEEE754 numbers.
It would be too confusing and prone to errors.
The single-quoted string is the same in both YANG and XPath,
so it is safer to stick with that.

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 09 Feb 2009 21:48:20 +0100 (CET)

Hi,

Andy Bierman <[email-masked]> wrote:
> check is applied when the value is created.

That's not the intention.

> This is of course

The current text says:

  All leafref nodes MUST reference existing leaf instances for the
  data to be valid.  This constraint is enforced according to the
  rules in ^constraints^.

And @constraints@ says:

- If the constraint is defined on configuration data, it MUST be true in
  a valid configuration data tree.

And *** Validation says:

  When datastore processing is complete, the final contents MUST obey
  all validation constraints.  This validation processing is performed
  at differing time according to the datastore.  If the datastore is
  <running/> or <startup/>, these constraints MUST be enforced at the
  end of the <edit-config> or <copy-config> operation.  If the
  datastore is <candidate>, the constraint enforcement is delayed
  until a <commit> or <validate> operation.

What needs to be done to make it more clear?

/martin

---

From: Andy Bierman <[email-masked]>
Date: Mon, 09 Feb 2009 13:38:50 -0800

> What needs to be done to make it more clear?

This is good enough.
The constraints could say that only the <running/> configuration
is required to be valid at all times. (If not, the
agent has a bug, or maybe the YANG file is wrong.)

IMO, startup is an open-issue.
There is a use-case for allowing it to be invalid
within the current running agent:

   1) copy-config(next-config, startup)
   2) upload-next-image(image-file)
   3) reboot

NETCONF access to startup is not mandatory anyway.

For <candidate/>, this config is never required
to be valid.  The <commit> operation applies
the requested changes to <running/>, and that
is the config database that must be valid.

Andy