Hi Elwyn, Seasons Greetings!
On Mon, Dec 23, 2013 at 5:00 PM, Elwyn Davies <elw...@dial.pipex.com> wrote: > I am the assigned Gen-ART reviewer for this draft. For background on > Gen-ART, please see the FAQ at > > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > > Please resolve these comments along with any other Last Call comments > you may receive. > > Document: draft-ietf-trill-rfc6327bis-02.txt > Reviewer: Elwyn Davies > Review Date: 23 December 2013 > IETF LC End Date: 23 December 2012 > IESG Telechat date: (if known) - > > Summary: > Almost ready for the IESG. There is one minor issue that seems as > if it needs a better way to explain how the defense against rogue > native packets is supposed to work and a number of nits. Apologies It’s not actually defense “against rogue native packets” but defense against a native packet being multiply ingressed or egressed... > if the festive spirit got the better of a few comments. No apologies needed... > Major issues: > None. Thanks. > Minor issues: > s2.2, last para: >> The primary TRILL defense mechanism against such loops, which is >> mandatory, is to assure that, as far as practically possible, >> there is only a single RBridge that is in charge of ingressing >> and egressing native frames... > This seems to be difficult to implement: Implementing some defense > mechanism is stated to be mandatory but the proposed defense is at > best, best efforts. It strikes me that given the expected > catastrophic results of failure to have a robust defense, as > delineated in the previous para, indicates that this either needs > some better explanation or possibly a better defense. I can't tell, > but I guess we may be talking about some extraneous measure beyond > this spec - please explain. The mandatory mechanism is explained in greatest detail in RFC 6439. Perhaps the references to that RFC should be stronger and clearer. (It requires very peculiar and unlikely events for there to be a problem and, while such a problem will be bad, it will be transient - cleared up as soon as Hellos are exchanged on the link.) > Nits/editorial comments: > General: Adding titles/caption to figures and tables would be helpful. OK. > General: s/byte/octet/ I would agree that it is important for this to be consistent within the document. There seem to currently be 2 occurrences of "octet" and 12+ occurrences of "byte". Also, "byte" seems to be the preferred term for IS-IS TLV sub-field diagrams. So, I would prefer to change the two occurrences of "octet" to "byte". > Abstract: s/IS-IS adjacency between/IS-IS adjacencies between/ > s1, para 1: multipathing?? horrid non-word!! Suggest: > OLD: > support for multipathing of both unicast and multicast traffic in > multi-hop networks with arbitrary topology. > NEW: > support for transmission of both unicast and multicast traffic > taking advantage of multiple paths in multi-hop networks with > arbitrary topology. OK. > s1, para 1: 'and a header that includes a hop count': A header for > what? Please make it clear what the header is added to. "a header" -> "a header in TRILL data packets" > s1, para 1, next to last sentence: Has two 'ands' > Suggest s/and optimization/as well as optimization/ OK. > s1, table: Probably worth expanding BFD again (even though it is > in the abstract and terminology). OK. > s1: Definition and explanation of 'pseudonode': There is no > explanation of the pseudonode term until s7 - and even there it is > buried a couple of paras down in the text. A definition/explanation > either in s1 or s1.2 would be helpful. "pseudonode" is a fundamental concept of IS-IS and I'm really not sure that it should be the role of this draft to explain it. I can add a reference to ISO 10589 after the first occurrence of "pseudonode" in Section 1. > s2.1, para 1, sentence 2: This sentence is difficult to parse: After > two or three reads, I realized that the material starting at > 'devices or services' is not part of the 'includes' and ceased > looking for an 'and'. Suggest reversing the components of the main > body thus: > OLD: > Because this includes general bridged LANs > [802.1Q], devices or services that can restrict VLAN connectivity, > such as [802.1Q] bridges or carrier Ethernet services, can be part of > a link between RBridges. > NEW: > Because this includes general bridged LANs > [802.1Q], the links between RBridges may contain devices or services that > can restrict VLAN connectivity, such as [802.1Q] bridges or carrier > Ethernet services. OK, I think that's an improvement. > s2.4, para 1: s/reasonably restricted MTU/relatively restricted MTU/ I think it really means that the restriction of MTU is reasonable, as opposed to unreasonable. > s3.3, Event A4: I was initially slightly confused as to how one timer > expiring resulted in both being expired in the broadcast case. Maybe > reword a bit: > OLD: > A4. Either (1) the Hello holding timer expires on a point-to-point > adjacency or (2) one or both Hello holding timers expire on a > broadcast LAN adjacency resulting in them both being in the > expired state for that adjacency. > NEW: > A4. Either (1) the Hello holding timer expires on a point-to-point > adjacency or (2), on a broadcast LAN adjacency, both Hello timers > expire together or, after one timer expired previously (see > Event A5), the second timer expires resulting in them both being > in the expired state for that adjacency. There is yet another possibility: One timer could expire while the other is still in its initial expired state so it would be incorrect to speak about it having "expired previously". How about the following: NEWER: A4. Either (1) the Hello holding timer expires on a point-to-point adjacency or (2), on a broadcast LAN adjacency, (2a) both Hello timers expire simultaneously or (2b) one Hello timer expires when the other Hello timer is already in the expired state. > s5, para 3: Although it isn't wrong, given that RFC 2119 language is > called out in s1.2, it might be better to explicitly use MUST > instead of mandatory here. MUST could also be used about the 1,470 > bytes (octets) minimum in para 2 I don't particularly like using "MUST" here. The RFC 2119 MUSTs are in the TRILL base protocol specification, RFC 6325. > s5, para 5: s/If it tests MTU/If it tests the MTU size/ OK. > s6, (not the) definition of BFD Enabled TLV: The figure shows a > fixed value of 3 for the Length field and exactly one topology entry > (2 octets) and a Protocol ID (1 octet). The text indicates that > there could be several, it isn't specific as to whether there has to > be a match between topology and protocol entries but the length sort > of indicates that there might be. Frankly this is a mess (probably > due to not being the definition of the TLV which isn't totally > obvious). The BFD enabled TLV is specified in RFC 6213 that is referenced elsewhere in this draft but, curiously, not in Section 6. A reference should be added there. > So if multiple topology and protocol entries are present: > - must there be a matching number of them (the length text sort of > implies this)? Yes. The text should clarify this. > - do they go in matched pairs: (topology, proto), (topology, proto), etc or > all topologies first and then all protocols? Matched pairs. > - Where topology ID's come from? A few are pre-assigned but mostly they are made up by the network's management. See RFC 5120 that should probably be referenced here. > - Where do NLPIDs come from? From ISO/IEC (see ISO9577), but there is a range available to other organizations, including the IETF. See RFC 6328 that is referenced here in the draft. > All of this can probably be done by citing the right reference as I > eventually realized. > s7, para 3: >> It is anticipated that many links between RBridges will actually be >> point-to-point, in which case using a pseudonode merely adds to the >> complexity. > Does this actually mean they actually be point-to-point even though they are > on and using a broadcast LAN TRILL setup? If so this should be clarified. How about replacing "... will actually be point-to-point, ..." with "... will actually be point-to-point even if the link technology supports operation as a multi-access broadcast link, ...". > s8.2.1, para 1: s/should not be included/SHOULD NOT be included/ OK. Thanks, Donald ============================= Donald E. Eastlake 3rd +1-508-333-2270 (cell) 155 Beaver Street, Milford, MA 01757 USA d3e...@gmail.com _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
