Early Review of draft-ietf-trill-yang-oam-03
review-ietf-trill-yang-oam-03-rtgdir-early-morin-2016-05-20-00

Hello,



I have been selected as the Routing Directorate reviewer for this draft. 


The Routing Directorate seeks to review all routing or routing-related 


drafts as they pass through IETF last call and IESG review, and 


sometimes on special request (the latter case here). The purpose of the 


review is to provide assistance to the Routing ADs. For more information 


about the Routing Directorate, please see ​ 


<

http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

>

http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir











Although these comments are primarily for the use of the Routing ADs, it 


would be helpful if you could consider them along with any other 


comments that you receive, and strive to resolve them through discussion 


or by updating the draft.




Document: draft-ietf-trill-yang-oam-03
Reviewer: Thomas Morin
Review Date: 2016-04-26
Intended Status: Std track



Summary: I have some concerns about this document that I think should be 


resolved before publication.




Comments:



The review of this draft was made a bit of a pain by the fact the 


document is plagued with reference and formatting issues. Having a look 


at idnits output would have been a useful prerequisite before concluding 


the draft ready for reviews.






That said, the draft is I believe of satisfying quality. I'm raising 


below only one technical issue, related to reusing typedefs instead of 


introducing new ones.






Note well that this review is not a Yang doctor review, not a review 


made with a full understanding of TRILL and TRILL OAM. The review I made 


cannot be considered exhaustive in these respects.




Major Issues:



The RFCs for the generic OAM Yang datamodel, the TRILL OAM Framework and 


the TRILL FM Framework should I think all be Normative references.






There are a few Yang typedefs that I expect should be defined in other, 


standalones, specifications rather than in this draft which is specific 


to TRILL and OAM, namely the "vlan" and "rb-nickname" which I would 


expect to be defined in a generic IETF/IEEE datamodel (for "vlan") and 


in the base TRILL Yang model (for "rb-nickname"). It seems for instance 


the dot1q-types.yang model in draft-wilton-netmod-intf-vlan-yang has a 


dot1q-vlan-id typedef.  The problem may be deeper for RB nicknames: 


draft-ietf-trill-yang which I would expect to be the place for an 


rb-nickname typedef, not only does not define one and only defines 


nickname leaves each time specifying their type, but these type 


definitions seem to be inconsistent, sometimes uint16 type with a 


constrained range), sometimes uint32, sometime uint16 without a range 


constraint, etc.  The nickname issue deserves to be addressed across 


both documents in a better way.






Last, although it is unusual to consider editorial issues as "Major", I 


will mention them here because the draft in its current state really 


deserves a lot of polish:


there are multiple formatting issues, wrong/incomplete or not- 


up-to-date references. I would kindly suggest that maybe the authors 


could consider using a real tool to edit their document (automating 


layout in a reliable way and automatically keeping references up to 


date).   Details on the issues are listed below since they are, each 


individually, minor issues or smaller nits.




Minor Issues:

* references are messy, in particular:
   - RFC7174 is correctly listed, but the reference is incomplete


   - TRLOAMFRM is also listed although it refers to the draft that 


became RFC7174


   - RFC 7455 is not listed although its refers to in the text of the 


document


* Section 4.5 talks about MTV, but does not introduce the ping and 


traceroute extension that are defined in the Yang model


* contact info for the Yang datamodel only list the draft authors, but 


the WG should be listed I guess


* On page 9, under "revision", the "reference" item says RFC7455, which 


I guess is a copy-paste error; it should say "draft-ietf-trill-yang-oam"


* the description fields under "grouping command-ext-trill" / 


"out-of-band" for ipv4-address, ipv6-adress, trill-nickname, could be 


improved by indicating to which device the address is


* I wonder if the ecmp-choice leaf description should really explain the 


meaning of each value, since the type is defined in the GOAM Yang model 


that may be updated in the future (maybe with new values ?), maybe it 


would be better to just point there


* The IANA section says that the URI is TBD, while an URI is actually 


specified in the Yang code.


* Section 7 is pointing to RFC7455 for the OAM Base Mode, it could be 


helpful to point at the precise location (Appendix B.)




Nits:



- meta: check the many things that idnits raises 


(

https://tools.ietf.org/idnits?url=https://tools.ietf.org/id/draft-ietf-trill-yang-oam-03.txt

)



- draft title is not centered on head page


- missing line breaks in many many places (eg. after Section 2 title, in 


Yang excerpts in section 4, in the overview in Section 5, etc.)


- for leaf-list next-hop-rbridges on page 16, there is a typo: 


"perticular" instead of "particular"


- Section 4.5 says that "defined in TRILL YANG model" which really is 


redundant


- Section 5 says "The complete data hierarchy related to the OAM YANG 


model is presented below."  but the hierarchy presented if, of course, 


the one of the _TRILL_ OAM datamodel





Best,

-Thomas