Subject: [netmod] choice/case in tree diagrams

From: Martin Bjorklund <[email-masked]>
Date: Mon, 05 Mar 2018 12:26:27 +0100 (CET)

Hi,

Lifting this issue to its own thread.

With this snippet:

    choice subnet {
      case prefix-length {
        leaf prefix-length {
          type uint8;
        }
      }
      case netmask {
        leaf netmask {
          type yang:dotted-quad;
        }
      }
    }

pyang prints choice/case nodes like this:

     +--rw (subnet)?
        +--:(prefix-length)
        |  +--rw prefix-length?   uint8
        +--:(netmask)
           +--rw netmask?         yang:dotted-quad

With the syntax defined in the yang-tree document:

  <status>--<flags> <name><opts> <type> <if-features>

it means that the choice node has <flags> just like any other node (in
this case "rw"), but the case node has "" as <flags>, and no space
after the "--".

This is clearly inconsistent, and something needs to be fixed.

The current yang-tree document doesn't say that choice/case should be
treated differently than other nodes.

Alternatives:

  1)  The document is correct, this is a bug in pyang, the output
      should be:

     +--rw (subnet)?
        +--rw :(prefix-length)
        |  +--rw prefix-length?   uint8
        +--rw :(netmask)
           +--rw netmask?         yang:dotted-quad

  2)  Since the choice/case nodes are not present in the data tree,
      they should not have any flags.  The document should be fixed to
      allow empty flags so we have:

     +-- (subnet)?
        +-- :(prefix-length)
        |  +--rw prefix-length?   uint8
        +-- :(netmask)
           +--rw netmask?         yang:dotted-quad

Note that the document is currently in AUTH48.

Something needs to be done in the document though, b/c it shows the
current pyang output in an example.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 5 Mar 2018 13:50:08 +0100

I prefer that the choice/case nodes do not have any flags since they
are not having a config true/false property on their own. And less
clutter is better.

/js

On Mon, Mar 05, 2018 at 12:26:27PM +0100, Martin Bjorklund wrote:
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <https://www.jacobs-university.de/>

---

From: Vladimir Vassilev <[email-masked]>
Date: Mon, 5 Mar 2018 14:14:26 +0100

> are not having a config true/false property on their own. And less
'choice' statements have 'config' substatement while 'case' do not. I 
myself figured that out while I was implementing tree diagrams support.

I would prefer the current pyang output and a change to the yang-tree 
document to specify that nodes without config substatement do not have 
<flags>.

Vladimir

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 5 Mar 2018 14:49:34 +0100

On Mon, Mar 05, 2018 at 02:14:26PM +0100, Vladimir Vassilev wrote:
> 'choice' statements have 'config' substatement while 'case' do not. I myself
> figured that out while I was implementing tree diagrams support.
> 

So it seems the running code got it right. ;-)

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 05 Mar 2018 14:54:18 +0100 (CET)

> So it seems the running code got it right. ;-)

As the author of that code, I think that was purely by accident...

But I'm not convinced it is the correct solution.  We have one example
in the other thread where someone was confused by the "rw" flag and
thought that it implied that the node would be present in the data
tree.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 5 Mar 2018 15:13:55 +0100

> thought that it implied that the node would be present in the data

So what does rw mean?

(i)  The schema node has a rw property.
(ii) The schema node can be instantiated and the instantiated data node
     has a rw property.

I think it is difficult to have both at the same time. If the tree is
a representation of schema nodes, then (i) seems to make more
sense. That said, the explanation in 2.6 is somewhat vague since it
says 'data' and not 'nodes' (like everywhere else):

OLD:

       <flags> is one of:
         rw  for configuration data
         ro  for non-configuration data, output parameters to rpcs
             and actions, and notification parameters

NEW:

       <flags> is one of:
         rw  for configuration data nodes
         ro  for non-configuration data nodes, output parameters to rpcs
             and actions, and notification parameters

The document (as far as I searched for it) does not clearly say that
'node' means 'schema node'. In hindsight, it might have been useful to
explicitely import terminology from RFC 7950 and to use it carefully
(RFC 7950 has 'schema node' and 'data node' but here we largely talk
about 'nodes' - and my assumption is that this means 'schema nodes'.)

/js

---

From: Vladimir Vassilev <[email-masked]>
Date: Mon, 5 Mar 2018 15:21:25 +0100

> thought that it implied that the node would be present in the data
+1. There are indeed very few 'config false;' statements in 'choice's in 
use and they do not justify the clutter and confusion of 'choice' 
representations in all tree diagrams. With that clarification I am not 
against the change specified in alternative 2.

Vladimir

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 05 Mar 2018 15:26:02 +0100 (CET)

>              and actions, and notification parameters

I think this is ok.  But that means that we also have to add:

           --  for a choice or case node

But in order to be consistent, we should probably have:

           --  for a choice, case, input or output node

This means that the correct tree syntax for choice and case will be:

     +-- (subnet)?
        +-- :(prefix-length)

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 05 Mar 2018 15:27:54 +0100 (CET)

>            --  for a choice, case, input or output node

Whoops, it shouldn't be "--".  Somehow we should say that no flags are
used for choice,case,input,output.

/martin

---

From: Ladislav Lhotka <[email-masked]>
Date: Mon, 05 Mar 2018 15:41:18 +0100

>            --  for a choice, case, input or output node

But unlike the three other statements, "choice" can have the config
substatement, so "rw/ro" makes sense there.

Lada

-- 
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67

---

From: Per Hedeland <[email-masked]>
Date: Mon, 5 Mar 2018 15:49:10 +0100

> substatement, so "rw/ro" makes sense there.

I don't think so - that config statement does not a define a property of
the choice node (it can obviously neither be read nor written), only a
default for descendant data nodes, as described in section 7.21.1 of RFC
7950.

--Per

---

From: Ladislav Lhotka <[email-masked]>
Date: Mon, 05 Mar 2018 16:06:54 +0100

> default for descendant data nodes, as described in section 7.21.1 of RFC

It is not a default - if a choice has "config false", then no descendant can be
"config true". One of the benefits of having rw/ro in the ascii tree is to see
where a state data subtree actually starts.

Lada

---

From: Per Hedeland <[email-masked]>
Date: Mon, 5 Mar 2018 18:40:08 +0100

> where a state data subtree actually starts.

It is a default, but yes, it is also a restriction in the specific case
of the argument being "false" at a point where the default would
otherwise be "true". And in that case it is equivalent to having "config
false" on all the descendant data nodes, and they will of course be
flagged as "ro" regardless of whether the "config false" comes from the
choice or the individual data nodes - and that is where the state *data*
suntree(s) actually start(s).

So I guess the question then is whether this specific case motivates
always having flags on specifically choice nodes, while the other
non-data nodes have no flags. Since the 'config' statement is ignored in
rpc/action input/output and notification, choice nodes there should then
presumably have "-w"/"ro"/"-n". Personally I think the diagram is
clearer with flags only on the data nodes.

--Per

---

From: Mahesh Jethanandani <[email-masked]>
Date: Mon, 5 Mar 2018 10:23:14 -0800

> On Mar 5, 2018, at 6:27 AM, Martin Bjorklund <[email-masked]> wrote:
> 
> Martin Bjorklund <[email-masked] <mailto:[email-masked]>> wrote:

I would agree, as having choice/case statements represented as schema nodes is not only confusing in the tree diagram, but also confusing when constructing an example. The tree diagram represents it as a node, where one would put it in the example, but validation complained about it (not being a node).

> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/netmod <https://www.ietf.org/mailman/listinfo/netmod>
Mahesh Jethanandani
[email-masked]

---

From: Vladimir Vassilev <[email-masked]>
Date: Tue, 6 Mar 2018 10:08:06 +0100

> clearer with flags only on the data nodes.
When I think about it <flags> do not have any information contents  
outside of the context of a data tree and its schema. So if we are 
removing clutter we should probably start there by specifying that 
<flags> should be ommited under rpc,notification and action.

Vladlimir

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 06 Mar 2018 10:44:11 +0100 (CET)

Hi,

After thinking some more about this, realizing that this document is
in AUTH48, and looking at the first sentence in the Abstract:

   This document captures the current syntax used in YANG module tree
   diagrams.

I have reached the conclusion that we probably shouldn't make any
drastic changes.

The current syntax, with flags for choice but not for case, may look a
bit odd, but it does follow RFC 7950 where a choice node can have a
config property, but case cannot.  Also, this syntax has now been used
for several years w/o causing much confusion.

I suggest the following changes to this document:

OLD:

       <flags> is one of:
         rw  for configuration data
         ro  for non-configuration data, output parameters to rpcs
             and actions, and notification parameters
         -w  for input parameters to rpcs and actions
         -u  for uses of a grouping
         -x  for rpcs and actions
         -n  for notifications
         mp  for nodes containing a "mount-point" extension statement

NEW:

       <flags> is one of:
         rw  for configuration data
         ro  for non-configuration data, output parameters to rpcs
             and actions, and notification parameters
         -w  for input parameters to rpcs and actions
         -u  for uses of a grouping
         -x  for rpcs and actions
         -n  for notifications
         mp  for nodes containing a "mount-point" extension statement

         case nodes do not have any <flags>.

Then, since the syntax requires whitespace before <name>:

we need to fix the examples:

OLD:

             +--rw (root-type)
                +--:(vrf-root)

NEW:

             +--rw (root-type)
                +-- :(vrf-root)

(two occurances)

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 06 Mar 2018 10:47:11 +0100 (CET)

Mahesh Jethanandani <[email-masked]> wrote:
> I would agree, as having choice/case statements represented as schema
> nodes is not only confusing in the tree diagram

Well, choice and case *are* schema nodes.  They are printed within
parentheses, which may give you a hint that they are not data nodes
and thus not visible in the payload.

The tree diagram is intended as a compact, easy-to-read representation
of the structure of the YANG module.  They will not contain all
details needed to produce good examples.

/martin

, but also confusing
> when constructing an example. The tree diagram represents it as a
> node, where one would put it in the example, but validation complained
> about it (not being a node).
> 
> > 
> > 
> > /martin
> > 
> > 
> >> 
> >> 
> > <https://www.ietf.org/mailman/listinfo/netmod>
> Mahesh Jethanandani
> [email-masked]
>

---

From: Robert Wilton <[email-masked]>
Date: Tue, 6 Mar 2018 09:57:58 +0000

+1 for this solution.

Thanks,
Rob

On 06/03/2018 09:44, Martin Bjorklund wrote:
> .

---

From: Lou Berger <[email-masked]>
Date: Tue, 06 Mar 2018 06:29:04 -0500

Martin,

> I have reached the conclusion that we probably shouldn't make any

I agree.

> Then, since the syntax requires whitespace before <name>:
I think we should match current tooling/practice here as well. Can you 
confirm how pyang works today?

My memory is no such space is added.  If my memory is correct, my 
preference is to change the text rather then the tooling.

Lou
(As contributor)

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 06 Mar 2018 13:10:26 +0100 (CET)

Lou Berger <[email-masked]> wrote:
> My memory is no such space is added.

This is correct.

> If my memory is correct, my

Maybe we can simply do:

OLD:

    <name> is the name of the node
      (<name>) means that the node is a choice node
     :(<name>) means that the node is a case node

      If the node is augmented into the tree from another module,
      its name is printed as <prefix>:<name>, where <prefix> is the
      prefix defined in the module where the node is defined.

NEW:

    <name> is the name of the node
      (<name>) means that the node is a choice node
     :(<name>) means that the node is a case node

      If the node is augmented into the tree from another module,
      its name is printed as <prefix>:<name>, where <prefix> is the
      prefix defined in the module where the node is defined.

      If the node is a case node, there is no space before the
      <name>.

/martin

---

From: Lou Berger <[email-masked]>
Date: Tue, 6 Mar 2018 07:17:46 -0500

>        If the node is a case node, there is no space before the
Looks good to me.

Thanks,
Lou

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 6 Mar 2018 13:48:25 +0100

>          case nodes do not have any <flags>.

I still think that it should be 'data node' instead of just
'data'. While not formally imported, the term 'data node' has a
definition in RFC 7950.

NEWER:

        <flags> is one of:

/js

---

From: Benoit Claise <[email-masked]>
Date: Tue, 6 Mar 2018 13:57:24 +0100

> I have reached the conclusion that we probably shouldn't make any
Obviously, otherwise I send the document back to the WG.

Regards, Benoit

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 06 Mar 2018 14:00:23 +0100 (CET)

> 'data'. While not formally imported, the term 'data node' has a

But choice is not a data node.  So if anything, it should be:

           rw  for configuration schema nodes
           ro  for non-configuration schema nodes, output parameters to rpcs
               and actions, and notification parameters

or possibly just

           rw  for configuration nodes
           ro  for non-configuration nodes, output parameters to rpcs
               and actions, and notification parameters

But then an "rpc" is also a "non-configuration schema node"... so now
it is not really clear which rule to follow for an rpc.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 6 Mar 2018 14:36:04 +0100

On Tue, Mar 06, 2018 at 02:00:23PM +0100, Martin Bjorklund wrote:
> But choice is not a data node.

EVEN NEVER

       <flags> is one of:
         rw  for configuration data nodes and choice nodes

My point is that 'data node' is a defined term in RFC 7950 and using
defined terms generally adds clarity.

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 06 Mar 2018 15:45:49 +0100 (CET)

> defined terms generally adds clarity.

I agree that using well defined terms add clarity.  However, the
proposed text is not quite right.  Possibly:

          rw  for configuration data- and choice nodes
          ro  for non-configuration data- and choice nodes,
              output parameters to rpcs and actions, and
              notification parameters

But maybe it is actually better to leave the original text, and go
through the entire document wrt terminology in a future update (if
necessary).  After all, the document is in AUTH48.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 6 Mar 2018 16:24:02 +0100

>               output parameters to rpcs and actions, and

Good.

> necessary).  After all, the document is in AUTH48.

I do not agree. 'data' is entirely undefined - it means nothing. How
do I know that 'data' includes 'choice nodes'? I think we better fix
this here.

I agree that a general careful check of terminology should be done in
an update but I do not see why AUTH48 says we should not fix the
problem that was discovered. And for me, a fix is not a fix if things
are left ambiguous.

/js

---

From: Ladislav Lhotka <[email-masked]>
Date: Tue, 06 Mar 2018 16:26:50 +0100

>           rw  for configuration data nodes

If we keep it also for choices, then it has to be "schema nodes".

>               and actions, and notification parameters

Same here.

Lada

---

From: joel jaeggli <[email-masked]>
Date: Tue, 6 Mar 2018 09:44:22 -0800

>> I have reached the conclusion that we probably shouldn't make any
I would tend to err on that side  as well it's a little late for that.
>>> >>>>>> (ii) The schema node can be instantiated and the instantiated
>>> data
>>> >>>>>> node
>>> >>>>>>       has a rw property.
>>> >>>>>>
>>> >>>>>>           ro for non-configuration data nodes, output
>>> parameters to
>>> >>>>>>           rpcs
>>> >>> I don't think so - that config statement does not a define a
>>> property
>>> >>> of
>>> >>> default for descendant data nodes, as described in section
>>> 7.21.1 of
>>> >>> RFC
>>> >>> 7950.

---

From: Benoit Claise <[email-masked]>
Date: Tue, 6 Mar 2018 22:49:37 +0100

Dear all,

The document draft-ietf-netmod-yang-tree-diagrams-06 is in AUTH48.
It's time to close this issue, let me copy [email-masked].
If you object to the change below, let me know before the end of the week.

The changes are to the text, so that the text is adapted to what the tools do.  No need to change the examples, or tooling.

OLD:
            <flags> is one of:

and

OLD:

NEW:

Regards, Benoit

---

From: Benoit Claise <[email-masked]>
Date: Sat, 10 Mar 2018 10:06:50 +0100

Thanks all.
This issue is closed and we will apply this change right now, part of 
the AUTH48 process.

Regards, Benoit
> The changes are to the text, so that the text is adapted to what the 
> tools do.  No need to change the examples, or tooling.
>
> OLD:
>            <flags> is one of: