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