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:[email protected]]
*Sent:* Monday, October 16, 2017 8:49 AM
*To:* [email protected]; [email protected]
*Cc:* Benoit Claise (bclaise) <[email protected]>; Alia Atlas
<[email protected]>
*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 "[email protected]"
<mailto:[email protected]>
module ietf-vrrp {
yang-version 1.1;
namespace "urn:ietf:params:xml:ns:yang:ietf-vrrp";
prefix "vrrp";
And
<CODE BEGINS> file "[email protected]"
<mailto:[email protected]>
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.
[email protected]
<mailto:[email protected]>). This "-state" module could
be used in conjunction with [email protected]
<mailto:[email protected]> 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. [email protected]
<mailto:[email protected]>. 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, [email protected]
<mailto:[email protected]> 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
[email protected] <mailto:[email protected]>
https://www.ietf.org/mailman/listinfo/rtgwg
_______________________________________________
rtgwg mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/rtgwg
