Hi Donald, -08 addresses all my comments, thanks. BTW 1 more editorial nit introduced in -08, in the new paragraph in the security considerations you have misspelt encapsulation.
Regards Ben > On 17 Jan 2018, at 23:23, Donald Eastlake <[email protected]> wrote: > > Hi Ben, > > Thanks for the further review and comments. See below. > > Version -08 has been uploaded with the goal of resolving these comments. > > On Wed, Jan 17, 2018 at 8:48 AM, Ben Niven-Jenkins > <[email protected] <mailto:[email protected]>> wrote: >> >> Hi Donald, >> >> Apologies for not responding sooner, I have reviewed the latest version >> (-07) and still have a couple of comments, see inline below. I have also >> included at the bottom of this email some additional editorial nits I found >> when reading -07. I have trimmed previous comment and responses from you >> that are now covered in the document. >> >> On 10 Jan 2018, at 20:05, Donald Eastlake <[email protected] >> <mailto:[email protected]>> wrote: >> >> ... >> >> On Thu, Oct 26, 2017 at 10:29 PM, Donald Eastlake <[email protected] >> <mailto:[email protected]>> wrote: >>> >>> Hi Ben, >>> >>> Thanks for your review. It appears that is was not responded to in a >>> timely fashion. Apologies on behalf of the authors. >>> >>> (Your review was of the -02 version. The current version is -05.) >>> >>> On Sun, Apr 24, 2016 at 4:28 AM, Ben Niven-Jenkins >>> <[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. 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-directory-assisted-encap-02.txt >>>> Reviewer: Ben Niven-Jenkins >>>> Review Date: 21 April 2016 >>>> Intended Status: Proposed Standard >>>> >>>> Summary: >>>> I have significant concerns about this document and recommend that >>>> the Routing ADs discuss these issues further with the authors. >>> >>> Hopefully the changes made between the version -02 you reviewed and >>> the current -05 have made some improvements and, based on your >>> comments and WG LC comments, further improvements can be made. >> >> >> I think the -07 is almost good to go, the only outstanding concern I have is >> with regards to the security considerations section, see below. > > Thanks. > >>>> Comments: >>>> Overall this is not the easiest document to read although some of >>>> that might be due to my lack of background in TRILL and its >>>> terminology. >>>> >>>> Major Issues: >>>> >>>> 1) The document has an Intended Status of Proposed Standard, however >>>> it does not contain any RFC2119 keywords and does not seem to make >>>> any normative statements about required behaviour which I would have >>>> expected in a Proposed Standard. >>> >>> Well, in version -05 there is at least one keyword instance. >>> Furthermore, I don't know that such keywords need to always be used >>> when an implementation requirement level is being specified. That >>> said, we could see if additional RFC 2119 keywords are warranted. >> >> I noted this as a flag to the ADs because the lack of RFC2119 key words >> seemed unusual to me. If the ADs are happy for this to be proposed standard >> then I am happy with it being a proposed standard. > > OK. > >>>> 2) Section 4: If I understand correctly the TRILL-EN spoofs the >>>> Ingress RBridge edge node's nickname in the source address field of >>>> the TRILL header. Is this likely to introduce problems? E.g. if >>>> RBridges will accept & forward frames that have their own source >>>> address in, does it perpetuate routing loops or present security >>>> considerations that the document should discuss? >>> >>> TRILL goes to great lengths to avoid loops and has a hop count in the >>> TRILL header so that, should there be a transient loop, a TRILL packet >>> in that loop (i.e., an encapsulated frame) will be discarded. In the >>> potentially more dangerous case of multi-destination packets, as >>> compared with known unicast, where copies could multiply due to forks >>> in the distribution tree, a Reverse Path Forwarding Check is used to >>> discard packets that appear to be on the wrong link or when there is >>> disagreement about the distribution tree. >>> >>> Security Considerations should probably say more on this. >>> >>>> Section 8 on Security Considerations also looks very light on >>>> text. If you are allowing TRILL-ENs to spoof RBridge source >>>> addresses (which I think you are, see comment above) I think you >>>> should have a discussion about that somewhere in the document. >>> >>> I agree that some further discussion is needed in the Security >>> Considerations section. >> >> I don’t see any discussion on TRILL-ENs spoofing ingress bridge nicknames in >> section 7 on security considerations. I see the security consideration >> section of the referenced RFC6325 states "RBridges do not prevent nodes from >> impersonating other nodes” although RFC6325 doesn’t appear to discuss the >> security considerations related to allowing such impersonation. >> >> I think it would be valuable for the security considerations section of >> draft-ietf-trill-directory-assisted-encap to explicitly call out that >> TRILL-ENs spoof ingress bridge nicknames and explain why that is not an >> issue (the text you use above is sufficient for that purpose IMO). >> >> I leave it up to you & the chairs/ADs to decide if my suggestion is overkill >> and that the existing reference to RFC6325 is sufficient for a reader >> skilled in the art of TRILL. > > A paragraph has been added to the Security Considerations to cover this. > >>>> Minor Issues: >>>> >>>> 1) Section 3. I am not sure what Figure 2 is trying to convey and it >>>> is not referred to by the main text. Is it required? >>> >>> Figure 2 is intended to show the header of a pre-encapsulated frame >>> going from a TRILL-EN to an edge TRILL switch. If it is retained in >>> the draft, there should be clarifying text that references it. >> >> I still don’t see any reference to figure 2 in the text (or to figure 1 for >> that matter). > > References have been added. > >>>> However, Section 4 says >>>> >>>> The TRILL-EN learns this nickname by listening >>>> to the TRILL IS-IS Hellos from the Ingress RBridge. >>>> >>>> which makes me think if the TRILL-EN is running IS-IS for hellos, is >>>> pushing the directory such an obstacle? >>> >>> That text refers to snooping on IS-IS messages, not running IS-IS. >> >> Ah, I see. IMO explicitly using the term “snooping" rather than “listening” >> here (and in section 1) would make this unambiguous. I leave it up to you >> whether to make that change or not. > > "listening" has been changed to "snooping". > >>>> 4) Section 7 on Manageability Considerations only states that in >>>> order for the solution to work requires the availability of a >>>> directory service, which seems a bit redundant when the entire >>>> document is about "Directory Assisted TRILL Encapsulation”. Is this >>>> section required? >>> >>> I agree that the Manageability Considerations section should have >>> some material added concerning configuration or be dropped. >> >> I see this section now includes "TRILL-EN have the same configuration >> options as any pull directory client.” Is there a suitable document you >> could informatively reference here that describes/discusses the >> configuration options for a pull directory client? > > A reference to the appropriate section of RFC 8171 has been added. > >> Minor nit: should the sentence start “TRILL-ENs”? > > OK. > >> Other editorial nits I found reading -07: >> >> Section 3, para 2 says "If a destination is not known to be attached to one >> or more RBridge edge nodes”. I struggled to parse this without reading it >> multiple times, I think what you mean to say is "If it is not known whether >> a destination is attached to one or more RBridge edge nodes”? > > That seems to be better wording. > >> Section 3, para 4: s/don’t/doesn’t/ >> >> Section 3, para 8: s/and perform/and performs/ >> >> Section 5.1, para 2: s/data frames with TRILL header/data frames with TRILL >> headers/ >> >> Section 7, para 1: s/TRIL-ENs/TRILL-ENs/ > > The above editorial fixes have been made. > > Thanks, > Donald > =============================== > Donald E. Eastlake 3rd +1-508-333-2270 (cell) > 155 Beaver Street, Milford, MA 01757 USA > [email protected] <mailto:[email protected]> > >> Regards >> Ben
_______________________________________________ trill mailing list [email protected] https://www.ietf.org/mailman/listinfo/trill
