Last Call Review of draft-ietf-trill-rfc6327bis-02
review-ietf-trill-rfc6327bis-02-genart-lc-davies-2013-12-23-00

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 if the
festive spirit got the better of a few comments.

Major issues:
None.

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.

Nits/editorial comments:
General: Adding titles/caption to figures and tables would be helpful.

General: s/byte/octet/

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.

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.

s1, para 1, next to last sentence: Has two 'ands'
Suggest s/and optimization/as well as optimization/

s1, table: Probably worth expanding BFD again (even though it is 
in the abstract and terminology).

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.

s2.1, para 1, sentence 2:  This sentence is difficult to parse:  After two or three 
reads, I realised 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.

s2.4, para 1: s/resonably restricted MTU/relatively restricted MTU/

s3.3, Event A4: I was initially slightly confused as to how one timer
expiring resulted in both being expired in the braodcast 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.

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

s5, para 5: s/If it tests MTU/If it tests the MTU size/

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).  
So if multiple topology and protocol entries are present:
- must there be a matching number of them (the length text sort of implies this)? 
- do they go in matched pairs: (topology, proto), (topology, proto), etc or
  all topologies first and then all protocols?
- Where topology ID's come from?
- Where do NLPIDs come from?
All of this can probably be done by citing the right reference as I 
eventually realised.

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.

s8.2.1, para 1: s/should not be include/SHOULD NOT be included/