Hi Danny, https://tools.ietf.org/html/draft-ietf-trill-ia-appsubtlv-08 has been posted incorporating the resolutions to your comments.
Thanks, Donald =============================== Donald E. Eastlake 3rd +1-508-333-2270 (cell) 155 Beaver Street, Milford, MA 01757 USA [email protected] On Thu, May 5, 2016 at 2:41 PM, McPherson, Danny <[email protected]> wrote: > > I’m comfortable with your stated intentions here Donald. > > Thanks for the prompt response, > > > -danny > > > > > On 5/5/16, 12:02 PM, "Donald Eastlake" <[email protected]> wrote: > >>Hi Danny, >> >>Thanks for your comments. See below. >> >>On Wed, May 4, 2016 at 10:27 AM, McPherson, Danny >><[email protected]> wrote: >>> >>> 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. >> >>Thanks. >> >>> Minor Issues: >>> >>> 1. 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. >> >>In the IETF context "silently discard" means that there is no >>on-the-wire message sent. It says nothing about whether or not >>counters are kept of such condition or errors are logged. A suggestion >>to log such events and/or keep such counters can be added. >> >>> 2. 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”. >> >>OK. Will add the reference. >> >>> 3. 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. >> >>This comes from RFC 6823, Section 3.2, which says that sub-TLVs that >>go inside the GENAPP TLV "are refrred to as APPsub-TLVs". >> >>> 4. 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. >> >>OK - that should be made more consistent, probably standardizing on >>"IA APPsub-TLV". >> >>> 5. 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" >> >>You don't like "sub-sub-TLV"? >> >>Seems like, strictly speaking, you have IS-IS PDUs which contain >>TLVs. Then some TLVs can contain sub-TLVs. (The GENAPP TLV is the only >>one that occurs to me with a special name for its sub-TLVs, namely >>APPsub-TLVs.) and some sub-TLVs can contain a further nested level, >>which it seems to me to be precise and logical to call sub-sub-TLVs. >>(I am not aware of any requirement for any more deeper nesting in a >>use of IS-IS.) So, would you prefer that what are called sub-sub-TLVs >>in this document just be called "sub-TLVs" (which I agree they are) >>resulting in two different levels with the same name? While there >>might be some errors in their use in this draft, the mere use of >>APPsub-TLV and sub-sub-TLV for the two levels does not seem "loose and >>sloppy" to me... >> >>> 6. Only one of the “Figures” is labeled / captioned >> >>OK. All the principal figures should be labeled. (I don't think cases >>where there is a small, indented figure that just expands part of a >>principal figure and appears shortly after the principal figure need >>to be captioned.) So, the initial figures in Sections 3.1, 3.2, 3.3, >>and 3.4 would have Figures numbers and captions added. >> >>> 7. 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. >> >>OK - I'll see what I can do. >> >>> 8. S 3.4 the 2-byte “Type” value in the diagram should be >>> “TOPOLOGY”, not “DATALEN”. >> >>Thanks for noticing this error. >> >>> 9. 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. >> >>I think her listing as an author was in error. >> >>> 10. 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. >> >>There are some improvements that could be made. In inverse order, >>Section 5.3 looks fine. In Section 5.2, "Available" should be changed >>to "Unassigned" as that is the preferred IANA term. Section 5.1 is >>talking about assignments that have already happened and looks OK as >>far as the table of values goes; however, the material after the first >>sentence after that table seems inappropriate in an "IANA >>Considerations" section and should, perhpas, be in a new "Processing >>Address Sets" section. >> >>> 11. 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? >> >>The idea is that local configuration or learning should be able to >>override address reachability received through network messages. Thus >>such information, when manually configured, defaults to have confidence >>255. >> >>RFC 6325 Section 4.8.1 just says that information learned via ESADI >>will have a confidence of from 0 to 254 but don't actually say what to >>do if it is recreived as 255. This is updated by Section 6.2 RFC 7357, >>1st paragraph, that makes it clear that a received value of 255 is >>just treated as if it was 254. Thus it is consistent with these prior >>RFCs to the IA APPsub-TLV draft to give this rule for handling the >>value 255 in the Confidence field of IP APPsub-TLVs. >> >>> 12. 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: >>> >>> 1. Abstract & Introduction: s/by-pass/bypass/ >> >>OK. >> >>> 2. S.2: s/Data Label is reachable from /Data Label are reachable/ >> >>"... inteface ... is reachable ...", so I think "is" is correct but >>I'll see if I can re-word this sentence. >> >>> 3. A reference for the first use of AFN would be useful, perhaps to >>> the IANA registry. >> >>OK. >> >>> 4. Expressing TBD code points in [ ] brackets might help with >>> readability as well >> >>OK. >> >>> 5. S 3.2 “if the Length is 0 or 1 or less” — not sure the “or less" >>> is necessary? >> >>OK, the "or less" should be removed. >> >>Thanks, >>Donald >>=============================== >> Donald E. Eastlake 3rd +1-508-333-2270 (cell) >> 155 Beaver Street, Milford, MA 01757 USA >> [email protected] _______________________________________________ trill mailing list [email protected] https://www.ietf.org/mailman/listinfo/trill
