I took your latest pre -11 .xml revision that you attached so below is a
re-review against -10 and pre (-11).  Comments below

Compilation error that was introduced between -10 and -11
- ietf-i2rs-...@2017-12-05.yang:1224: error: unexpected keyword "range"

    leaf hop-limit {
      type uint8;
            range "1..255";
        "The path hop limit header.";
- Also, by introducing a range on a uint8, are you saying that a hop-limit=0
  would never be in use for any use-case here?

There are formatting and alignment issues all throughout the document that
need fixing (Mahesh pointed this out as well)

'ietf-interfaces' import carries the wrong RFC reference (RFC 7227)
- This should rather be RFC 7277

There are still ongoing discussions among yang-doctors on how references in
imports should be treated.  At a minimum for now, lets carry the expanded
syntax w/ the title of the RFC as such: (As Mahesh previous mentioned, this is
subject to change pending further discussion)
- reference "RFC 6991: Common YANG Data Types";
- reference "RFC 7277: A YANG Data Model for IP Management";
- reference "RFC 6991: Common YANG Data Types";

Module description still does not carry full copyright/distribution section as
specified in:

There still appears to be one remaining 'Rib' reference
- s/Routing Instance and Rib/Routing Instance and RIB/

Section 1.2 - Tree Diagrams
- The text here is an instruction template rather than what the actual text
  should be.  It should rather read:

   A simplified graphical representation of the data model is used in
   this document.  The meaning of the symbols in these diagrams is
   defined in [I-D.ietf-netmod-yang-tree-diagrams].

You did not address my previous comment/question re: NMDA (Tom mentioned this
as well)

You did not address my previous comment/question re: Section 2.5 (route-add

I still have concern to modules introducing large amounts of
features/if-features as previously noted but will defer to other's comments on
this as well

Formatting nits:
- L3605: s/route:A/route: A/
- L3610: s/from\/to RIB/from\/to a RIB/
- L3610: s/lead to suboptimal/lead to a suboptimal/
- L3611: s/possibly/possible/
- L3626: s/his/their/

Tom's previous comment re: ENABLE_IP_RPF_CHECK has not been addressed and
exists in both -10 and (pre) -11 versions



On Feb 20 15:24 PM, Amit Dass 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?
> Best regards,
> Amit
> -----Original Message-----
> From: Ebben Aries [] 
> Sent: Thursday, January 18, 2018 9:33 AM
> To:
> Cc:;;
> 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:
> - ietf-i2rs-...@2017-12-05.yang
> 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 ietf-i2rs-...@2017-12-05.yang:
> - 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)
> - import "ietf-interfaces" should reference RFC 7223 per
> - import "ietf-yang-types" should reference RFC 6991 per
> - Since this module imports "ietf-interfaces", a normative references must be
>   added per
> - prefix "if" in the import "ietf-interfaces" can remove quotes to remain
>   consistent with other imports
> - Remove WG Chairs from contact information per
> - Module description must contain most recent copyright notice per
> - Module description should contain note to RFC Ed. and placeholder reference
>   to RFC when assigned
> - Add placeholder reference and note to RFC Ed. for RFC when assigned
> - Security Considerations should be updated to reflect new template at
> - 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
> - 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/

i2rs mailing list

Reply via email to