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

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

Looking at -10, from the standpoint of one who is not a YANG doctor, I
see some issues outstanding.

"   This document defines a YANG [RFC6020][RFC6991] data model"
- RFC6020 is YANG 1.0 not 1.1 - I think there needs to be a reason not
to use 1.1

- And what is the status of this vis-a-vis NMDA?  Needs stating

- And RFC6991 is an odd reference for YANG. since it is data types; not
off the wall, but seems oddto me

1.2 Guidelines on tree diagrams can be found in Section 3 of
   [I-D.ietf-netmod-yang-tree-diagrams].
-This should be a Informative Reference, not a Normative one

2.2.  Routing Instance and Rib
- RIB?

3.  YANG Modules

 <CODE BEGINS> file "[e-mail]"
- The date will need updating and this calls for  a Note to the RFC
Editor

 "  namespace "urn:ietf:params:xml:ns:yang:ietf-i2rs-rib";
   // replace with iana namespace when assigned  "
I do not understand this

import ietf-interfaces {
     prefix if;
         reference "RFC 7223";
   }
- RFC7223 has been superseded unless you require the old version in
which case you should date the import.
Either way, the referenced I-D/RFC needs to appear in the Reference
section of this I-D

"    Copyright (c) <2018> IETF Trust and the persons
      identified as authors of the code.  All rights reserved.";  "
- This looks inadequate to me; see for example
draft-ietf-netmod-syslog-model
for a more comprehensive one

   revision "2018-02-12" {
- as before, a note to the RFC Editor is needed

         // RFC Ed.: replace XXXX with actual RFC number and remove
         // this note
- I see no XXXX to be replaced here

"leaf hop-limit {
       type uint8;
       description
         "The hop limit the header.";
"
Is something missing from the description?

description   "NvGRE can use eigher IPv4    or IPv6 header for
encapsulation.";
- gher?

- The  module has very few reference clauses and when I see something
capitalised, e.g.

leaf ip-rpf-check {
               "Each RIB can be optionally associated with a
            ENABLE_IP_RPF_CHECK
I suspect that a reference is required

leaf route-change-reason

- The values that this can take seem rather few

Finally, I like the Security Considerations

Tom Petch

----- Original Message -----
From: "Ebben Aries" <[e-mail]>
To: <[e-mail]>
Cc: <[e-mail]>; <[e-mail]>;
<[e-mail]>
Sent: Thursday, January 18, 2018 8:33 AM
Subject: [i2rs] 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
> - 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/
>
> _______________________________________________
> i2rs mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/i2rs