Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-rfc7277bis-00

From: Qin Wu <[email-masked]>
Date: Mon, 4 Dec 2017 02:47:24 +0000

Support publication of this draft, my same question is applied to this draft as well.
Should this draft reference NMDA guideline or rfc6087bis?

-Qin
-----邮件原件-----
发件人: netmod [mailto:[email-masked]] 代表 Kent Watsen
发送时间: 2017年11月29日 3:43
收件人: [email-masked]
抄送: [email-masked]
主题: [netmod] WG Last Call: draft-ietf-netmod-rfc7277bis-00

[resending]

All,

This starts a two-week working group last call on draft-ietf-netmod-rfc7277bis-00.  

Please recall that this update's intention is to modify the YANG module to be in line with the NMDA guidelines [1].  Reviewing the diff between the two drafts [2] should reveal just this.

The working group last call ends on December 12.
Please send your comments to the netmod mailing list.

Positive comments, e.g., "I've reviewed this document and believe it is ready for publication", are welcome!
This is useful and important, even from authors.

[1] https://tools.ietf.org/html/draft-dsdt-nmda-guidelines-01
[2] https://tools.ietf.org/rfcdiff?url2=draft-ietf-netmod-rfc7277bis-00.txt

Thank you,
Netmod Chairs

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

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 04 Dec 2017 15:12:18 +0100 (CET)

Alex Campbell <[email-masked]> wrote:
> Hi,
> 
> I noticed the table in section 3 has been mangled - it has extra
> blank lines, entries in the wrong side of the table, and a missing
> pipe in the bottom right corner.

Thanks, now fixed.  The table now looks like this:

   +----------------------------------+--------------------------------+
   | YANG data node in                | IP-MIB object                  |
   | /if:interfaces/if:interface      |                                |
   +----------------------------------+--------------------------------+
   | ipv4                             | ipv4InterfaceEnableStatus      |
   | ipv4/enabled                     | ipv4InterfaceEnableStatus      |
   | ipv4/address                     | ipAddressEntry                 |
   | ipv4/address/ip                  | ipAddressAddrType              |
   |                                  | ipAddressAddr                  |
   | ipv4/neighbor                    | ipNetToPhysicalEntry           |
   | ipv4/neighbor/ip                 | ipNetToPhysicalNetAddressType  |
   |                                  | ipNetToPhysicalNetAddress      |
   | ipv4/neighbor/link-layer-address | ipNetToPhysicalPhysAddress     |
   | ipv4/neighbor/origin             | ipNetToPhysicalType            |
   | ipv6                             | ipv6InterfaceEnableStatus      |
   | ipv6/enabled                     | ipv6InterfaceEnableStatus      |
   | ipv6/forwarding                  | ipv6InterfaceForwarding        |
   | ipv6/address                     | ipAddressEntry                 |
   | ipv6/address/ip                  | ipAddressAddrType              |
   |                                  | ipAddressAddr                  |
   | ipv4/address/origin              | ipAddressOrigin                |
   | ipv6/address/status              | ipAddressStatus                |
   | ipv6/neighbor                    | ipNetToPhysicalEntry           |
   | ipv6/neighbor/ip                 | ipNetToPhysicalNetAddressType  |
   |                                  | ipNetToPhysicalNetAddress      |
   | ipv6/neighbor/link-layer-address | ipNetToPhysicalPhysAddress     |
   | ipv6/neighbor/origin             | ipNetToPhysicalType            |
   | ipv6/neighbor/state              | ipNetToPhysicalState           |
   +----------------------------------+--------------------------------+

           YANG Interface Data Nodes and Related IP-MIB Objects

/martin

> 
> Apart from that this draft looks good to me.
> ________________________________________
> From: netmod <[email-masked]> on behalf of Kent Watsen <[email-masked]>
> Sent: Wednesday, 29 November 2017 8:29 a.m.
> To: [email-masked]
> Cc: [email-masked]
> Subject: [netmod] WG Last Call: draft-ietf-netmod-rfc7277bis-00
> 
> All,
> 
> Please recall that this update's intention is to
> modify the YANG module to be in line with the NMDA
> guidelines [1].  Reviewing the diff between the two
> drafts [2] should reveal just this.
> 
> Positive comments, e.g., "I've reviewed this document
> and believe it is ready for publication", are welcome!

---

From: Vladimir Vassilev <[email-masked]>
Date: Mon, 11 Dec 2017 17:35:46 +0100

Hello,

I've reviewed this document and believe it is ready for publication.

The focus of my review this time was on validating the module and the example modules and example data through running code. I implemented NMDA for the open source tools we use and added a testcase that reproduces the result specified in "Appendix E. Example: NETCONF <get-data> Reply" after loading the configuration specified in "Appendix D.  Example: NETCONF <get-config> Reply" and providing the config false; data and system originated configuration as needed. I can confirm the implementation validated the example modules and the example data producing identical output.

IMO the examples are demonstrating well the concept of NMDA and its application for the ietf-interfaces module.

I had an issue with a bug in [email-masked] I had to fix in order to use the <get-data> RPC. The bug is already reported on the mailing-list.

diff ietf-patched/[email-masked] ietf-draft/[email-masked]
140c140
<         when 'derived-from-or-self(../datastore, "ds:operational")';
---
>         when 'derived-from-or-self(../datastore, "or:operational")';

Vladimir

---

From: Vladimir Vassilev <[email-masked]>
Date: Tue, 12 Dec 2017 21:04:16 +0100

Hello,

The previous post was intended for the rfc7223bis Last Call (wrong 
subject line).

I just completed similar validation through a testcase for the examples 
in rfc7277bis ("Appendix A.  Example: NETCONF <get-config> reply" and 
"Appendix B.  Example: NETCONF <get-data> Reply")

Here there are some inconsistencies between the <get-config> output and 
the expected <get-data> output. A missing origin bug is probably more 
significant then the rest. The following patch fixes the inconsistencies 
and the testcase passes:

--- before.txt    2017-12-12 20:39:09.037576425 +0100
+++ after.txt    2017-12-12 20:37:46.425656680 +0100
@@ -7,14 +7,12 @@
             <type>ianaift:ethernetCsmacd</type>
             <!-- other parameters from ietf-interfaces omitted -->
             <ipv4 xmlns="urn:ietf:params:xml:ns:yang:ietf-ip">
-             <forwarding>false</forwarding>
-             <mtu>1500</mtu>
               <address>
                 <ip>192.0.2.1</ip>
                 <prefix-length>24</prefix-length>
                 <origin>static</origin>
               </address>
-             <neighbor>
+             <neighbor or:origin="or:learned">
                 <ip>192.0.2.2</ip>
                 <link-layer-address>
                   00:01:02:03:04:05
@@ -22,7 +20,6 @@
               </neighbor>
             </ipv4>
             <ipv6 xmlns="urn:ietf:params:xml:ns:yang:ietf-ip">
-             <forwarding>false</forwarding>
               <mtu>1280</mtu>
               <address>
                 <ip>2001:db8::10</ip>

In contrast to rfc7223bis-00 I have not reviewed rfc7277bis-00 and this 
is only a report of a detected issue.

Vladimir

On 12/11/2017 05:35 PM, Vladimir Vassilev wrote:
> The focus of my review this time was on validating the module and the 
> example modules and example data through running code. I implemented 
> NMDA for the open source tools we use and added a testcase that 
> reproduces the result specified in "Appendix E. Example: NETCONF 
> <get-data> Reply" after loading the configuration specified in 
> "Appendix D.  Example: NETCONF <get-config> Reply" and providing the 
> config false; data and system originated configuration as needed. I 
> can confirm the implementation validated the example modules and the 
> example data producing identical output.
>
> IMO the examples are demonstrating well the concept of NMDA and its 
> application for the ietf-interfaces module.
>
> I had an issue with a bug in [email-masked] I 
> had to fix in order to use the <get-data> RPC. The bug is already 
> reported on the mailing-list.
>
> diff ietf-patched/[email-masked] 
> ietf-draft/[email-masked]
> 140c140
>> [2] 
>> https://tools.ietf.org/rfcdiff?url2=draft-ietf-netmod-rfc7277bis-00.txt
>>
>> Thank you,
>> Netmod Chairs
>>

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 13 Dec 2017 15:47:34 +0100 (CET)

Hi,

Thanks for reporting this.  I'll add the missing origin.  But why did
you think forwarding and mtu should be removed?  In fact, I think I
missed <enabled>, so here's my diff:

--- ex-get-data-reply.xml
+++ ex-get-data-reply.xml
@@ -13,6 +13,7 @@
         <!-- other parameters from ietf-interfaces omitted -->
 
         <ipv4 xmlns="urn:ietf:params:xml:ns:yang:ietf-ip">
+          <enabled>true</enabled>
           <forwarding>false</forwarding>
           <mtu>1500</mtu>
           <address>
@@ -20,12 +21,13 @@
             <prefix-length>24</prefix-length>
             <origin>static</origin>
           </address>
-          <neighbor>
+          <neighbor or:origin="or:learned">
             <ip>192.0.2.2</ip>
             <link-layer-address><!-- PPL -->00:01:02:03:04:05</link-layer-address>
           </neighbor>
         </ipv4>
         <ipv6 xmlns="urn:ietf:params:xml:ns:yang:ietf-ip">
+          <enabled>true</enabled>
           <forwarding>false</forwarding>
           <mtu>1280</mtu>
           <address>

/martin

Vladimir Vassilev <[email-masked]> wrote:
> I just completed similar validation through a testcase for the
> examples in rfc7277bis ("Appendix A.  Example: NETCONF <get-config>
> reply" and "Appendix B.  Example: NETCONF <get-data> Reply")
> 
> Here there are some inconsistencies between the <get-config> output
> and the expected <get-data> output. A missing origin bug is probably
> more significant then the rest. The following patch fixes the
> inconsistencies and the testcase passes:
> 
> In contrast to rfc7223bis-00 I have not reviewed rfc7277bis-00 and
> this is only a report of a detected issue.
> 
> Vladimir
>

---

From: Vladimir Vassilev <[email-masked]>
Date: Wed, 13 Dec 2017 16:26:48 +0100

Hi,

On 12/13/2017 03:47 PM, Martin Bjorklund wrote:

> you think forwarding and mtu should be removed?
1. IMO since <mtu> is not present in the <ipv4> container in the 
Appendix A (<get-config>) example and does not have default value in the 
model I still think it should be removed.
>    In fact, I think I
> missed <enabled>,
2. IMO both fixes adding <enabled> or removing <forwarding> should be OK 
depending on the RFC6243 defined with-defaults capability 'basic-mode' 
parameter advertised by the server. I was running the example with 
basic-mode=explicit

Vladimir

>   so here's my diff:

---

From: Vladimir Vassilev <[email-masked]>
Date: Wed, 13 Dec 2017 17:00:05 +0100

On 12/13/2017 04:26 PM, Vladimir Vassilev wrote:
> Appendix A (<get-config>) example and does not have default value in 
> the model I still think it should be removed.
Alternatively the ipv4/mtu node can be a good example of a 
origin="or:system" configuration.
>>    In fact, I think I
>> missed <enabled>,
> 2. IMO both fixes adding <enabled> or removing <forwarding> should be 
> OK depending on the RFC6243 defined with-defaults capability 
> 'basic-mode' parameter advertised by the server. I was running the 
> example with basic-mode=explicit
>
> Vladimir
>
>>   so here's my diff:
>>
>>               <link-layer-address><!-- PPL 
>> -->00:01:02:03:04:05</link-layer-address>
>>             </neighbor>
>>           </ipv4>

---

From: Kent Watsen <[email-masked]>
Date: Thu, 14 Dec 2017 07:04:03 +0000

All, 

  This last call is now closed.  Since the comments
  were minor, this last call is considered successful.

Authors,

  Please address any/all comments received during 
  the LC.  Once you've published the version that 
  you believe addresses all comments/pending changes,
  I'll perform the Shepherd review.

Thank you,
Kent

===== original message =====

All,

This starts a two-week working group last call on
draft-ietf-netmod-rfc7277bis-00.  

Thank you,
Netmod Chairs

---

From: Martin Bjorklund <[email-masked]>
Date: Sat, 16 Dec 2017 10:46:45 +0100 (CET)

Hi,

> origin="or:system" configuration.

Yes.

> > example with basic-mode=explicit

Right.  I now have this:

      <ipv4 xmlns="urn:ietf:params:xml:ns:yang:ietf-ip">
        <enabled or:origin="or:default">true</enabled>
        <forwarding or:origin="or:default">false</forwarding>
        <mtu or:origin="or:system">1500</mtu>
        <address>
          <ip>192.0.2.1</ip>
          <prefix-length>24</prefix-length>
          <origin>static</origin>
        </address>
        <neighbor or:origin="or:learned">
          <ip>192.0.2.2</ip>
          <link-layer-address>00:01:02:03:04:05</link-layer-address>
        </neighbor>
      </ipv4>
      <ipv6 xmlns="urn:ietf:params:xml:ns:yang:ietf-ip">
        <enabled or:origin="or:default">true</enabled>
        <forwarding or:origin="or:default">false</forwarding>
        <mtu>1280</mtu>
        ...

Do you think this is ok?

/martin

---

From: Vladimir Vassilev <[email-masked]>
Date: Mon, 18 Dec 2017 11:46:33 +0100

>          <forwarding or:origin="or:default">false</forwarding>
Yes. The or:default data makes the example even better.

1. However there is one more default value missing 
(/interfaces/interface[name='eth0']/enabled) for the example to be 
consistent
2. ... and in the last diff I unintentionally omitted the get-data 
output node <data> required namespace addition (this is also applicable 
to draft-ietf-netmod-7223bis-01):

diff -u before2.xml after2.xml
--- before2.xml    2017-12-18 11:41:54.029279321 +0100
+++ after2.xml    2017-12-18 11:36:25.973850340 +0100
@@ -1,4 +1,4 @@
-     <data>
+     <data xmlns="urn:ietf:params:xml:ns:yang:ietf-netconf-datastores">
         <interfaces
             xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces"
             xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type"
@@ -7,6 +7,7 @@
           <interface or:origin="or:intended">
             <name>eth0</name>
             <type>ianaift:ethernetCsmacd</type>
+           <enabled or:origin="or:default">true</enabled>
             <!-- other parameters from ietf-interfaces omitted -->
             <ipv4 xmlns="urn:ietf:params:xml:ns:yang:ietf-ip">
               <enabled or:origin="or:default">true</enabled>

Vladimir

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 18 Dec 2017 12:32:56 +0100 (CET)

Hi,

> (/interfaces/interface[name='eth0']/enabled) for the example to be

Note that the example has:

This covers the 'enabled' leaf (and more).  The idea is to let this
document focus on the ip parameters.

> output node <data> required namespace addition (this is also
> applicable to draft-ietf-netmod-7223bis-01):

Good catch!  I have applied this in both documents.

I also realized that the drafts don't use text from the latest
Security Considerations template; I have fixed that as well.

/martin

> +     <data
> xmlns="urn:ietf:params:xml:ns:yang:ietf-netconf-datastores">
>         <interfaces