Subject: Re: [yang-doctors] YANG doctor review of draft-ietf-rtgwg-yang-rip-02

From: Benoit Claise <[email-masked]>
Date: Wed, 3 Jan 2018 10:24:47 +0100

Hi Lada,

draft-ietf-rtgwg-yang-rip-07 is on the Jan 11th IESG telechat.
Would you mind confirming whether this version is fine?

Regards, Benoit
> Hi,
>
> I was assigned to be the YANG doctor for this document. Here is my
> review:
>
> **** General comments
>
> Overall, the module is in a good shape and integrates nicely with the
> core routing data model [RFC 8022].
>
> The I-D text is rather scarce, apart from the module it essentially
> contains only boilerplate stuff. Some information about the context in
> which this module is supposed to be used, design decisions etc. would
> be certainly useful.
>
> A non-normative appendix with a configuration example (instance data)
> would also help people that are not fluent in YANG to understand what
> data are included in this model.
>
> Some parts of the model – "distribute-list", "redistribute", "bfd",
> "authentication" on interfaces (or at least the "key" list) – are
> probably not specific to RIP and could be used by other protocols as
> well. If it is so, it would be better to model them separately as
> general mechanisms.
>
> Documents containing YANG modules that are imported by ietf-rip should
> appear among normative references. One way to achieve this (which is
> also useful by itself) is to include a table of namespace prefixes,
> see e.g.
>
> https://tools.ietf.org/html/rfc8022#section-2.3
>
> **** Specific comments
>
>       - Matching identities is done in the old YANG 1.0 way, for
>         example:
>
>         when "rt:type = 'rip:ripv2' or rt:type = 'rip:ripng'"
>
>         This is fragile because the identities are matched based on
>         string equality, so the result depends on the choice of the
>         namespace prefix. Unless there is a strong reason not to use
>         YANG 1.1, this is much simpler and more rubust
>
>         when "derived-from(rt:type, 'rip:rip')"
>
>       - In "redistribution", the must expressions are outright broken,
>         e.g. for IS-IS:
>
>         must "../../../../../rt:control-plane-protocol"
>         + "[rt:name = current()]/type = 'isis'" { ... }
>
>         First, the ietf-rip module also needs to import ietf-isis, and
>         then the must statement can be rewritten like so:
>
>         must "derived-from-or-self("
>         + "../../../../../rt:control-plane-protocol"
>         + "[rt:name = current()]/type, 'isis:isis')"
>
>       - Some descriptions could mention that the parameter (feature
>         etc.) only applies to RIP. For example, in
>
>         feature interface-statistics {
>           description
>             "This feature indicates that the system supports collecting
>              per-interface statistic data.";
>         }
>
>         I would add "... related to RIP." at the end.
>
>       - Maybe it's absolutely clear to experts but the name and
>         description of the "neighbor-configuration" feature look like
>         the feature can make neighbors remotely configurable. I had to
>         look up where it is used to find out what's the real
>         meaning. Perhaps something like "explicit-neighbors" might be
>         easier to understand?
>
>       - The name of "no-supply" leaf looks strange. I checked a couple
>         of CLIs and they use "passive", "passive-interface" or
>         "send-options" for preventing RIP updates from being sent.
>
>       - In "split-horizon" leaf, I would use "poison-reverse" enum
>         rather than "poison".
>
> Lada
>

---

From: Ladislav Lhotka <[email-masked]>
Date: Wed, 03 Jan 2018 16:22:40 +0100

Hi Benoit,

> Would you mind confirming whether this version is fine?

Yes, I think Xufeng addressed all my comments and suggestions.

Thanks, Lada

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

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 3 Jan 2018 10:22:25 -0800

I am looking at the -07 version of the draft, and I see that the YANG model has the WG chairs listed in the model. I thought we had decided not to include the name of the chairs in the model. See Appendix C of rfc6087bis for example of a YANG module template. Also the copyright is pretty old (it refers to 2016), but I assume the RFC editors will update that.

Cheers.

> On Jan 3, 2018, at 7:22 AM, Ladislav Lhotka <[email-masked]> wrote:
> 
> Hi Benoit,
> 
> Benoit Claise <[email-masked] <mailto:[email-masked]>> writes:
> 
>> Hi Lada,
>> 
> _______________________________________________
> yang-doctors mailing list
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/yang-doctors <https://www.ietf.org/mailman/listinfo/yang-doctors>
Mahesh Jethanandani
[email-masked]