Re: [i2rs] [yang-doctors] Yangdoctors early review of draft-ietf-i2rs-rib-data-model-09

["t.petch" <[e-mail]>]

---- Original Message -----
From: "Mahesh Jethanandani" <[e-mail]>
Sent: Thursday, February 22, 2018 12:43 AM
>
> > On Feb 20, 2018, at 7:24 AM, Amit Dass <[e-mail]>
wrote:
> >
> > Hi Ebben,
> >
> > I have updated the draft based on your comments. Could you please
have a look at the same and provide your feedback?
>
> The indentations are all over the place for the new references that
have been added.
>
> More importantly, and this discussion is still open in front of YANG
doctors, adding a reference statement to an import statement, seems to
imply a import by revision. As an example, the import of ietf-interfaces
has a reference to RFC 7223. But we know that ietf-interfaces is going
to updated soon by whatever RFC number gets assigned to rfc7223bis.
Expect further guidance on this.

Mahesh

As you doubtless know, this I-D is expected to be approved on March 8th,
so there is a certain urgency about this.

The latest guidelines, draft-ietf-netmod-rfc6087bis-18, include

   For every import or include statement that appears in a module
   contained in the specification, which identifies a module in a
   separate document, a corresponding normative reference to that
   document MUST appear in the Normative References section.

which seems clear and right to me.  This also says to me that a specific
version of the module is referenced via the Normative References
whether or not the import has a revision clause.

and it also says

 If an import statement is for a module from a stable source (e.g., an
   RFC for an IETF module), then a reference-stmt SHOULD be present
   within an import statement.

So for me there is (almost) always a reference statement to an RFC (or
I-D) even, or especially when, no particular revision is wanted.

This is what has coloured my comments on YANG modules.

Tom Petch

> Also, I do not see normative references to RFC 6991, and RFC 7223 in
the text of the document. If this is not clear, see rfc7223bis, where in
Section 3.1, there is normative reference to RFC7224 for the
iana-if-types module that is imported by the ietf-interfaces module.
>
> Cheers.
>
> >
> > https://datatracker.ietf.org/doc/draft-ietf-i2rs-rib-data-model/
> >
> > Best regards,
> > Amit
> >
> > -----Original Message-----
> > From: Ebben Aries [mailto:[e-mail]]
> > Sent: Thursday, January 18, 2018 9:33 AM
> > To: [e-mail]
> > Cc: [e-mail]; [e-mail];
[e-mail]
> > Subject: Yangdoctors early review of
draft-ietf-i2rs-rib-data-model-09
> >
> > Reviewer: Ebben Aries
> > Review result: On the Right Track
> >
> > 1 module in this draft:
> > - [e-mail]
> >
> > No YANG validation errors or warnings (from pyang 1.7.3 and yanglint
0.14.59)
> >
> > 0 examples are provided in this draft (section 3.12 of
> > draft-ietf-netmod-rfc6087bis-15)
> >
> > Module [e-mail]:
> > - yang-version statement missing - should be 1.1
> > - prefix 'iir' is recommended for this module, would 'rib' suffice
better?
> > - import "ietf-inet-types" should reference RFC 6991 per (not as a
comment)
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7
> > - import "ietf-interfaces" should reference RFC 7223 per
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7
>
> This should reference rfc7223bis.
>
> > - import "ietf-yang-types" should reference RFC 6991 per
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7
> > - Since this module imports "ietf-interfaces", a normative
references must be
> >  added per
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-3.9
> > - prefix "if" in the import "ietf-interfaces" can remove quotes to
remain
> >  consistent with other imports
> > - Remove WG Chairs from contact information per
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> > - Module description must contain most recent copyright notice per
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> > - Module description should contain note to RFC Ed. and placeholder
reference
> >  to RFC when assigned
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> > - Add placeholder reference and note to RFC Ed. for RFC when
assigned
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> > - Security Considerations should be updated to reflect new template
at
> >  https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines
> > - Section 1.2 should be replaced with reference to
> >  draft-ietf-netmod-yang-tree-diagrams-02 rather (as-is in other i2rs
YANG
> >  drafts in progress) per
> >
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-2.5.
1
> > - This module contains '12' features.  While it is understood the
purpose of
> >  these features in the module, take precaution as to complexity for
clients
> >  if they need to understand >= quantity of features per module in
use on a
> >  network-element.
> > - A few comments exist that are either unecessary or redundant.
Encode the
> >  comment intent rather in description fields if need be.
> > - Per NMDA, which datastores are targeted for the module?  Will all
RPC
> >  operations be acting upon the dynamic/ephemeral datastore?  It is
not clear
> >  to me if the intention is to be persistent or ephemeral
> >
> > General comments/Nits:
> > - references to 'def' could be expanded out to 'definition'
> > - references to 'decap' could be expanded out to 'decapsulation' for
> >  readability (across definitions and descriptions)
> > - Follow consistent capitalization of 'RIB' throughout document
text.  Mixed
> >  use of 'Rib' and 'rib' exists (Outside of YANG node lowercase
definitions).
> > - Is it necessary to prefix all nodes under the nexthop container
with
> >  "nexthop-"?
> > - Section 2.5 - route-add RPC - text mentions it is required that
the nh-add
> >  RPC be called as a pre-requisite however if the nh already exists
and the
> >  nexthop-id is known, this should not be necessary.  In addition,
the text
> >  reads 'or return' which should rather be a result of querying the
> >  appropriate node in the data tree.
> > - In 'IANA Considerations' - s/This document requests to
register/This
> >  document registers/
> >
> > _______________________________________________
> > yang-doctors mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/yang-doctors
>
> Mahesh Jethanandani
> [e-mail]
>
> _______________________________________________
> i2rs mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/i2rs