Re: I-D Action: draft-ietf-rtgwg-yang-vrrp-05.txt

[Robert Wilton <[e-mail]>]

Hi Xufeng,

Thanks, some comments inline ...


On 17/10/2017 14:57, Xufeng Liu wrote:
>
> Hi Rob,
>
> Thanks for examining this and providing the comments.
>
> *From:*Robert Wilton [mailto:[e-mail]]
> *Sent:* Monday, October 16, 2017 8:49 AM
> *To:* [e-mail]; [e-mail]
> *Cc:* Benoit Claise (bclaise) <[e-mail]>; Alia Atlas 
> <[e-mail]>
> *Subject:* Re: I-D Action: draft-ietf-rtgwg-yang-vrrp-05.txt
>
> Hi draft-ietf-rtgwg-yang-vrrp draft authors,
>
> I have a couple of comments on the latest revision of this draft:
>
> 1) The top level "vrrp container" has the right name, but can be 
> "config false" since it doesn't hold any configuration today.  This is 
> noting that RFC 7950 allows the "vrrp container" to become "config 
> true" as a backwards compatible change in future if required.
>
> */[Xufeng] However, making the container “config false” would prevent 
> vendor augmentations from adding nodes that are “config true”./*
>
This equivalent logic could apply to any "config false" node in any YANG 
data tree ;-)

The generic solution here is that the standard model should be "config 
false", and the vendor model should have a deviation on the top level 
VRRP container to make it "config true" and then augment it with the 
vendor configuration.


> 2) I've also noticed that this draft defined two separate versions of 
> the VRRP YANG module.  The second version in the appendix is for 
> pre-NMDA implementations:
>
>    <CODE BEGINS> file "[e-mail]" 
> <mailto:[e-mail]>
>    module ietf-vrrp {
>      yang-version 1.1;
>      namespace "urn:ietf:params:xml:ns:yang:ietf-vrrp";
>      prefix "vrrp";
>
> And
>    <CODE BEGINS> file "[e-mail]" 
> <mailto:[e-mail]>
>    module ietf-vrrp {
>      yang-version 1.1;
>      namespace "urn:ietf:params:xml:ns:yang:ietf-vrrp";
>      prefix "vrrp";
>
> Generically, I don't think that it is a good idea to define two 
> versions of the same YANG module in one draft.
>
> */[Xufeng] /**/This is to address Alia’s comment about “/*that there is an implementation. That implies that the existing model should be in the appendix.”
> */I think that such a case has not been thoroughly discussed. In the 
> NMDA Guidelines, this case falls into category  (b).(“/*either an existing model or ...
> */”). The goal here is to provide a model that does not break the 
> existing implementation, and can work with previous version of 
> ietf-interfaces in RFC7223. For such a purpose, none of the following 
> suggestions works too well./*
>
>
I think that existing implementations should just implement whichever ID 
version of the module that they have currently implement.  I don't think 
that means that IETF should have to publish that ID version as part of 
an RFC.  It isn't like the old revision goes away once the RFC has been 
published.

Your new NMDA compatible module doesn't have any direct dependency on 
RFC7223bis, i.e. when I tried it compile your NMDA version of the VRRP 
module it works just fine with the existing RFC 7223 version of 
ietf-interfaces.yang (i.e. rev 2014-05-08).  The VRRP state nodes would 
appear under the interfaces/interface part of the tree, but given that 
VRRP would always expect to be configured to have any state this should 
be fine, and is valid YANG.

>
> If a backwards compatible NMDA version of a module is required, then 
> an extra "-state" module should be put in the appendix instead (e.g. 
> [e-mail] 
> <mailto:[e-mail]>). This "-state" module could 
> be used in conjunction with [e-mail] 
> <mailto:[e-mail]> until NMDA implementations are 
> available.
>
> */[Xufeng] There is no such a parent root “-state” module in either 
> RFC7223 or RFC7223bis, so we cannot create such a module cleanly, 
> following the conventions in the guideline. If we did, such a module 
> would break the current implementations./*
>
When generating a state tree, special consideration needs to be applied 
for modules that have already been published with existing state trees.  
In that case, the generated "-state" module would augment 
interfaces-state/interface/ipv6.

These steps here might be slightly more accurate for generating a state 
module:
https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ#Q4

> *//*
>
>
> Alternatively, if you must define an existing pre NMDA version of the 
> VRRP module then it should definitely be given a different module 
> name, e.g. [e-mail] 
> <mailto:[e-mail]>. But I believe that this 
> would be an inferior solution since, compared to a separate "-state" 
> module, it will make it harder for clients to migrate to the NMDA 
> modules in future.
>
> */[Xufeng] Why is the different name necessary. The name 
> “ietf-vrrp-split-tree” has some issues: 1) breaks the currently 
> implementation; 2) the naming is arbitrary, without any conventional 
> rules; 3) brings more confusions./*
>
I just think that it is very confusing for a single draft to publish two 
module revisions with the same name.  Particularly given that two 
modules don't follow the YANG upgrade rules.  Tooling like RFC strip 
will automatically extract both revisions etc.  The name doesn't need to 
be "ietf-vrrp-split-tree", just any unique name. But as mentioned below, 
I don't think that you should need to publish it at all.

> *//*
>
> */Why can’t we use the same name? with one older version and one newer 
> version, just like ietf-interfaces with two versions (one in RFC7223 
> and RFC7223bis). The older version reflects the older (current) 
> implementation working with RFC7223 without NMDA. When the vendor 
> moves to NMDA and RFC7223bis, the newer version will be used./*
>
The version in RFC7223bis is a backwards compatible upgrade to the 
existing RFC7223 version.  I.e. it hasn't removed any existing nodes, it 
had just marked part of the tree as deprecated.

I don't see the benefit in putting the old revision into the RFC, it was 
just a working draft version of the model, a foot note in history on the 
path of standardization.

>
> Finally, having actually read at the main VRRP module, then on the 
> assumption that VRRP is always configured before it is used, then the 
> "NMDA version" may well be sufficient to use on both existing NETCONF 
> implementations and NMDA compatible NETCONF implementations.  The only 
> thing that you can't see when using the NMDA version of the module on 
> a "pre NMDA" implementation is the applied VRRP configuration.  
> Whether this is important enough to not define the extra "-state" 
> module is unclear, but my instinct would be that it is better to just 
> leave it out.
>
> */[Xufeng] The “applied configuration” is a feature that could be 
> useful, but not something that the VRRP model has to have. If it were 
> not for being close to the existing implementations, we’d use only the 
> NMDA model. The “pre NMDA” implementation uses ietf-interfaces in 
> RFC7223, which has the -state branch. A matching VRRP model would be 
> more consistent and convenient for such implementations. If we had 
> only the NMDA model, we’d force the implementation to update to the 
> NMDA VRRP model (and better with RFC7223bis). The older version of the 
> non-NMDA model would not require the implementation to change 
> immediately./*
>
> */It is all about the existing implementations. We can have several 
> levels of convenience for the implementations. If it is ok to require 
> them to upgrade now, we can drop the non-NMDA version./*
>
I don't think that we have to require them to upgrade now, I think that 
they should use whichever pre-standard version they are currently using, 
but I wouldn't put this into the RFC (not even in the appendix), it will 
just cause confusion in the long term.

E.g. for a protocol draft you wouldn't expect a RFC to describe other 
pre-standard specifications before it was standardized.  I'm not sure 
why YANG modules should be treated any differently.

Thanks,
Rob


> *//*
>
> */Thanks,
> - Xufeng/*
>
> Thanks,
> Rob
>
> *//*
>
> On 30/09/2017 15:12, [e-mail] 
> <mailto:[e-mail]> wrote:
>
>     A New Internet-Draft is available from the on-line Internet-Drafts directories.
>
>     This draft is a work item of the Routing Area Working Group WG of the IETF.
>
>              Title           : A YANG Data Model for Virtual Router Redundancy Protocol (VRRP)
>
>              Authors         : Xufeng Liu
>
>                                Athanasios Kyparlis
>
>                                Ravi Parikh
>
>                                Acee Lindem
>
>                                Mingui Zhang
>
>       Filename        : draft-ietf-rtgwg-yang-vrrp-05.txt
>
>       Pages           : 68
>
>       Date            : 2017-09-30
>
>     Abstract:
>
>         This document describes a data model for Virtual Router Redundancy
>
>         Protocol (VRRP).  Both version 2 and version 3 of VRRP are covered.
>
>     The IETF datatracker status page for this draft is:
>
>     https://datatracker.ietf.org/doc/draft-ietf-rtgwg-yang-vrrp/
>
>     There are also htmlized versions available at:
>
>     https://tools.ietf.org/html/draft-ietf-rtgwg-yang-vrrp-05
>
>     https://datatracker.ietf.org/doc/html/draft-ietf-rtgwg-yang-vrrp-05
>
>     A diff from the previous version is available at:
>
>     https://www.ietf.org/rfcdiff?url2=draft-ietf-rtgwg-yang-vrrp-05
>
>     Please note that it may take a couple of minutes from the time of submission
>
>     until the htmlized version and diff are available at tools.ietf.org.
>
>     Internet-Drafts are also available by anonymous FTP at:
>
>     ftp://ftp.ietf.org/internet-drafts/
>
>     _______________________________________________
>
>     rtgwg mailing list
>
>     [e-mail] <mailto:[e-mail]>
>
>     https://www.ietf.org/mailman/listinfo/rtgwg
>