[i2rs] Benoit Claise's Discuss on draft-ietf-i2rs-yang-l3-topology-13: (with DISCUSS and COMMENT)

Benoit Claise <bclaise@cisco.com> Wed, 13 December 2017 08:58 UTC

Return-Path: <bclaise@cisco.com>
X-Original-To: i2rs@ietf.org
Delivered-To: i2rs@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 4281E127871; Wed, 13 Dec 2017 00:58:28 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Benoit Claise <bclaise@cisco.com>
To: The IESG <iesg@ietf.org>
Cc: draft-ietf-i2rs-yang-l3-topology@ietf.org, shares@ndzh.com, i2rs-chairs@ietf.org, shares@ndzh.com, i2rs@ietf.org, mvasko@cesnet.cz, ladislav.lhotka@nic.cz
X-Test-IDTracker: no
X-IETF-IDTracker: 6.67.1
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <151315550826.30150.3938652389982359856.idtracker@ietfa.amsl.com>
Date: Wed, 13 Dec 2017 00:58:28 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2rs/EF9oYTBGOuI98-F0ZtVRnjI_CkA>
Subject: [i2rs] Benoit Claise's Discuss on draft-ietf-i2rs-yang-l3-topology-13: (with DISCUSS and COMMENT)
X-BeenThere: i2rs@ietf.org
X-Mailman-Version: 2.1.22
List-Id: "Interface to The Internet Routing System \(IRS\)" <i2rs.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/i2rs>, <mailto:i2rs-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/i2rs/>
List-Post: <mailto:i2rs@ietf.org>
List-Help: <mailto:i2rs-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/i2rs>, <mailto:i2rs-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 13 Dec 2017 08:58:28 -0000

Benoit Claise has entered the following ballot position for
draft-ietf-i2rs-yang-l3-topology-13: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-i2rs-yang-l3-topology/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Here is another validation issue, on the latest v13, that must be corrected.
Actually multiple mistakes, all with the same root cause. Note that those
warnings were reported in the datatracker:
   - Yang catalog entry for ietf-l3-unicast-topology-state@2017-11-15.yang
   - Yang catalog entry for ietf-l3-unicast-topology@2017-11-15.yang

Thanks to Michal, here is the detailed explanation.
Hi Benoit,
firstly, you see the output only with -V because it changes verbosity to
'warnings' instead of the default one 'errors'. So, the warnings are valid and
the when conditions are wrong. In short, in these cases the local module is not
ietf-l3-unicast-topology, but ietf-network (despite being defined in
ietf-l3-unicast-topology) - "ietf-network:network-types/l3-unicast-topology"
should be
"ietf-network:network-types/ietf-l3-unicast-topology:l3-unicast-topology" or
even "network-types/ietf-l3-unicast-topology:l3-unicast-topology".

To explain, here is the (long) justification. I will use the first warning as
an example, all the other warnings are caused by the same mistake. We are
dealing with a when condition "nw:network-types/l3-unicast-topology" in an
augment "/nw:networks/nw:network" in the module ietf-l3-unicast-topology. As
per [1], the context node of the XPath expression (when condition) is the
augment target node "/nw:networks/nw:network". If you notice, the warning warns
about not finding the node "l3-unicast-topology", not "nw:network-types". So,
what node is actually referenced by "l3-unicast-topology"? Looking at [2],
second bullet point, the namespace (prefix) should that of the "current node".
Sadly, there is no (I have found none) formal definition of "current node" but
I dared to assume it to be the node returned by the "current()" [3] function.
As was said at the beginning, the "initial context node" is
"/nw:networks/nw:network". Finally, we can now decide that
"l3-unicast-topology" actually takes the prefix of the module "ietf-network"
and there is no such node, this node is defined in the module
"ietf-l3-unicast-topology".

I hope it is clear now.

Kind regards,
Michal

[1] https://tools.ietf.org/html/rfc7950#page-137
[2] https://tools.ietf.org/html/rfc7950#section-6.4.1
[3] https://tools.ietf.org/html/rfc7950#section-10.1.1

On Tuesday, December 12, 2017 16:49 CET, Benoit Claise <bclaise@cisco.com>
wrote:

> Hi Radek,
>
> I'm looking at warnings for
> ietf-l3-unicast-topology-state@2017-11-15.yang and
> ietf-l3-unicast-topology-state@2017-11-15.yang
> See http://www.claise.be/IETFYANGPageCompilation.html
>
> Here is something interesting: the warnings only appear with the -V option.
>
> bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ yanglint
> ietf-l3-unicast-topology@2017-11-15.yang
> bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ yanglint -i
> ietf-l3-unicast-topology@2017-11-15.yang
> bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ yanglint -i -V
> ietf-l3-unicast-topology@2017-11-15.yang
> warn: Schema node "l3-unicast-topology" not found
> (ietf-network:network-types/l3-unicast-topology) with context node
> "/ietf-network:networks/network".
> warn: Schema node "l3-unicast-topology" not found
> (../ietf-network:network-types/l3-unicast-topology) with context node
> "/ietf-network:networks/network/node".
> warn: Schema node "l3-unicast-topology" not found
> (../ietf-network:network-types/l3-unicast-topology) with context node
> "/ietf-network:networks/ietf-network:network/link".
> warn: Schema node "l3-unicast-topology" not found
> (../../ietf-network:network-types/l3-unicast-topology) with context node
>
"/ietf-network:networks/ietf-network:network/ietf-network:node/termination-point".
> bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ yanglint -v > yanglint 0.13.79 >
bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ > >
bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ yanglint >
ietf-l3-unicast-topology-state@2017-11-15.yang >
bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ yanglint -i >
ietf-l3-unicast-topology-state@2017-11-15.yang >
bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ yanglint -i -V >
ietf-l3-unicast-topology-state@2017-11-15.yang > warn: Schema node
"l3-unicast-topology" not found >
(ietf-network:network-types/l3-unicast-topology) with context node >
"/ietf-network:networks/network". > warn: Schema node "l3-unicast-topology" not
found > (../ietf-network:network-types/l3-unicast-topology) with context node >
"/ietf-network:networks/network/node". > warn: Schema node
"l3-unicast-topology" not found >
(../ietf-network:network-types/l3-unicast-topology) with context node >
"/ietf-network:networks/ietf-network:network/link". > warn: Schema node
"l3-unicast-topology" not found >
(../../ietf-network:network-types/l3-unicast-topology) with context node >
"/ietf-network:networks/ietf-network:network/ietf-network:node/termination-point".
> warn: Schema node "l3-unicast-topology" not found >
(ietf-network-state:network-types/l3-unicast-topology) with context node >
"/ietf-network-state:networks/network". > warn: Schema node
"l3-unicast-topology" not found >
(../ietf-network-state:network-types/l3-unicast-topology) with context > node
"/ietf-network-state:networks/network/node". > warn: Schema node
"l3-unicast-topology" not found >
(../ietf-network-state:network-types/l3-unicast-topology) with context > node
"/ietf-network-state:networks/ietf-network-state:network/link".

> warn: Schema node "l3-unicast-topology" not found
> (../../ietf-network-state:network-types/l3-unicast-topology) with
> context node
>
"/ietf-network-state:networks/ietf-network-state:network/ietf-network-state:node/termination-point".
> bclaise@bclaise-VirtualBox:~/ietf/YANG-all$ > > Should we pay attention to
those? > > Regards, Benoit

====================================================
Preliminary note: I hope I'm doing the right thing by updating this DISCUSS
point as  I understand that the document is back to the WG. However, since I
reviewed the version 11, since some of my ballot points have been addressed
(thank you), and since I wanted to share my feedback publicly, here is my
feedback.

1. The examples.
In the AUTH48 for the RESTCONF RFC, the example YANG module discussion came up
(again).  And the examples in draft-ietf-i2rs-yang-l3-topology were also
discussed. Here is the feedback from one YANG doctor, from a couple of days ago.

Look at this:

   module example-ietf-ospf-topology {
     ...
     namespace
       "urn:ietf:params:xml:ns:yang:example-ietf-ospf-topology";
     ...
     description
       "This module defines a model for OSPF network topologies.
        Copyright (c) 2017 IETF Trust and the persons identified as
        authors of the code.

They are using the IANA-controlled namespace w/o registering it.

This module *really* looks like a proper normative module, rather than an
example.  They went to far in trying to mimic a real module.

It is clear that we need more guidelines in 6087 for how to write
example modules.

I was going to ask if this module passed YANG doctor review - then I
checked and saw that version -02 was reviewed, which didn't include
this example.  How should we (the YANG doctors) handle such a case?

In this case they should:

  1.  change the name to example-ospf-topology
  2.  change the namespace to urn:example:ospf-topology
  3.  remove the top-level statements:
          organization, contact, revision

  4.  change the top-level description to what the text in the draft
      says:

      description
        "This module is intended as an example for how the
         Layer 3 Unicast topology model can be extended to cover
         OSFP topologies.";

(same for the other example module)

As I mentioned to the authors, respective chairs and AD already, we should
follow the decision in this NETMOD email thread
https://www.ietf.org/mail-archive/web/netmod/current/msg17428.html This will
hopefully resolve fast. Once settled, the examples should be updated.

4.

       leaf-list router-id {
           type inet:ip-address;
           description
             "Router-id for the node";
         }

My initial DISCUSS was: We don't want to wait for
https://tools.ietf.org/html/draft-ietf-rtgwg-routing-types-00 (btw, we should
expedite this publication), but any good reason why this is aligned with its
definition?
    typedef router-id {
       type yang:dotted-quad;
       description
         "A 32-bit number in the dotted quad format assigned to each
          router. This number uniquely identifies the router within an
          Autonomous System.";
     }

My NEW DISCUSS: since is in IETF LC and on the telechat on Oct 12th, it makes
sense to import its router-id


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

- YANG definition "YANG: A data definition language for NETCONF"
I would use:
   YANG is a data modeling language used to model configuration data,
   state data, Remote Procedure Calls, and notifications for network
   management protocols [RFC7950]

- There are multiple slightly different definitions of the datastore in
the different RFCs.
Let's not add to the confusion.
Pick one (RFC6241 should be the latest one) and mention the reference.

I'm copying Lada, as the responsible YANG doctor, in case he wants to add anything.
- section 7
OLD:
The moodel defines
NEW:
The model defines