Subject: Re: [Gen-art] Gen-art Yuletide LC review of draft-ietf-trill-rfc6327bis-02.txt

From: Jari Arkko <[email-masked]>
Date: Mon, 6 Jan 2014 04:51:57 -0800

Thanks for your review, Elwyn, and Donald for your response and new version. I have placed a no-obj position for this document for the upcoming IESG telechat.

Jari

On Dec 24, 2013, at 10:34 AM, Donald Eastlake <[email-masked]> wrote:

> Hi Elwyn,
> 
> Seasons Greetings!
> 
> On Mon, Dec 23, 2013 at 5:00 PM, Elwyn Davies <[email-masked]>
> 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
> [email-masked]
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art