[RTG-DIR] RtgDir review: TRILL: Interface Addresses APPsub-TLV <draft-ietf-trill-ia-appsubtlv-07.txt>

["McPherson, Danny" <[e-mail]>]

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 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

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 IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.



Document: draft-ietf-trill-ia-appsubtlv-07.txt

Reviewer: Danny McPherson

Review Date: May 4, 2016

Intended Status: Proposed Standard



Summary:

 I have some minor concerns about this document that I think should be resolved before publication.



Comments:

I believe the draft is technically sound, however, the quality and readability needs a bit more work, particularly as it relates to introduction of new terms, and consistent application and use of all terms.  There are also some general error handling and encoding issues that need to be given consideration.



Major Issues:

I have no “Major” issues with this I-D.



Minor Issues:
ERROR HANDLING: There are a number of places in the document where it discusses the receipt of malformed, badly encoded, non-matching, or corrupt messages, and the advice is to either [silently] discard or ignore the messages.  Some general guidance should be given here to enable operational diagnosis of any issues that may result in temporal or persistent problems, where logging and other actions should occur.  Some aspects of this might leverage the OAM Framework efforts, although it appears much of the TRILL work leaves this to the implementer.
When using “Nickname” it would be useful to define the encoding as an unsigned 16-bit integer, or just reference "as specified in S 3.7 of RFC6325”.
The inclusion of the “TLV” acronym in the "APPsub-TLV” TLV name seems loose and redundant to me, as opposed to “APPsub TLV” or similar.
Inconsistent use of “Interface Address APPsub-TLV”, “IA APPSub-TLV”, “Interface Address APP-subTLV”, and “AppsubTLV” makes it seem like you’re talking about different things.
The use of “sub-sub-TLV” seems a bit loose and sloppy to me as well, and should be cleaned up.  E.g., S 5.2 “IA Appsub-TLV Sub-Sub-TLVs SubRegistry"
Only one of the “Figures” is labeled / captioned
The use of “Address Sets” and “Address Sets Ends” makes it a bit hard to read when used in sentences.  Perhaps an acronym for each, or hyphenating/underscoring them would make it more readable.
S 3.4 the 2-byte “Type” value in the diagram should be “TOPOLOGY”, not “DATALEN”.
I noticed that Radia was a co-author until the last revision, and now she doesn’t even exist in the Acknowledgements section.  While no explanation is required here, I did find this a bit odd.
IANA Considerations: Some guidance from the IANA folks on the formatting of this section might be in order.  It’s not as clear as it could be about what their instructions are here.
S 2: It’s unclear to me if the “Confidence” value of 255 “being treated as if it was 254” is inline with RFC6325 S 4.8.1 guidance?
In general, I agree there appear to be no new Security Considerations here.  I do not believe Asymmetry will be an issue with the forged packet discard issue although some consideration of this might be in order (or perhaps simply a reference to SAVI or other work here).  I wonder if some consideration should be given to broader disclosure of reachable layer 2 addresses here, but that seems a bit reaching as well.
Nits:

Abstract & Introduction: s/by-pass/bypass/
S.2: s/Data Label is reachable from /Data Label are reachable/
A reference for the first use of AFN would be useful, perhaps to the IANA registry.
Expressing TBD code points in [ ] brackets might help with readability as well
S 3.2 “if the Length is 0 or 1 or less” — not sure the “or less" is necessary?