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 "ietf-i2rs-...@2017-12-05.yang"
- 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...@juniper.net>
To: <yang-doct...@ietf.org>
Cc: <i2rs@ietf.org>; <draft-ietf-i2rs-rib-data-model....@ietf.org>;
<i...@ietf.org>
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:
> - 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)
>
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
> i2rs@ietf.org
> https://www.ietf.org/mailman/listinfo/i2rs
_______________________________________________
i2rs mailing list
i2rs@ietf.org
https://www.ietf.org/mailman/listinfo/i2rs
