Skip to main content

Early Review of draft-ietf-babel-yang-model-03
review-ietf-babel-yang-model-03-yangdoctors-early-krejci-2019-10-14-00

Request Review of draft-ietf-babel-yang-model
Requested revision No specific revision (document currently at 13)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2019-10-15
Requested 2019-09-24
Requested by Donald E. Eastlake 3rd
Authors Mahesh Jethanandani , Barbara Stark
I-D last updated 2019-10-14
Completed reviews Yangdoctors Early review of -03 by Radek Krejčí (diff)
Secdir Last Call review of -09 by Loganaden Velvindron (diff)
Yangdoctors Last Call review of -09 by Radek Krejčí (diff)
Rtgdir Last Call review of -09 by Ron Bonica (diff)
Genart Last Call review of -09 by Stewart Bryant (diff)
Comments
Related draft draft-ietf-babel-information-model-09 is about to pass WG Last Call.
Assignment Reviewer Radek Krejčí
State Completed
Request Early review on draft-ietf-babel-yang-model by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/Z5AeTtyaxHkv-Zl5mK2vE7Iba5w
Reviewed revision 03 (document currently at 13)
Result Ready w/nits
Completed 2019-10-14
review-ietf-babel-yang-model-03-yangdoctors-early-krejci-2019-10-14-00
There is a single module in the draft: ietf-babel@2019-08-22.yang.

There are 2 warnings reported by pyang 2.0.2, but both warnings seem to be
false.

ietf-babel@2019-08-22.yang:53: warning: RFC 8407: 3.1: The IETF Trust Copyright
statement seems to be missing (see pyang --ietf-help for details).
ietf-babel@2019-08-22.yang:208: warning: the module seems to use RFC 2119
keywords, but the required text from RFC 8174 is not found (see pyang
--ietf-help for details).

The draft is well written with an informative overview of the YANG module
sections. Maybe some set of example configuration data (some Appendix) would be
useful.

Regarding the YANG module, I have just a few notes:

- In multiple description texts, nodes are referenced by a path in format with
dashes (e.g. babel-mac-default-apply). This format is confusing especially when
some element in the path contains dash(es). Why the common format with slashes
is not used?

- Description and reference statements in containers and lists are placed as
the last statement (after children nodes). To increase readability and allow
people to decide if the content of the container/list is interesting for
detailed view, please move the descriptions / references to the beginning.

- grouping routes
  - just think if the grouping is reusable - in the module itself, the grouping
  is used just once and needless creation of groupings decrease module
  readability

- routes/routes/received-metric
  -  ... indicate a the route ... -> indicate the route ...

- babel/interfaces/metric-algorithm
  - the text "The value MUST be one of those listed in
  'metric-comp-algorithm'." in description just duplicates the type's
  information (identityref with metric-comp-algorithms base).

- babel/interfaces/mac-key-sets
  - what does the babel-mac-key-sets in description refer to?

- babel/interfaces/stats/reset
  - I'm confused, is it per-interface reset (as placed in the module) or
  system-wide interfaces stats reset (as information model defines it?). If it
  is the system-wide reset, why tha action cannot be placed in babel container?

- babel/mac/test/output/resulting-hash
  - according to the content, it seems to me that the description is actually
  the missing description of the test action.

- babel/dtls/test/output
  - seems to me more as a description of the action itself (where the
  description is missing)