Early Review of draft-ietf-trill-smart-endnodes-02
review-ietf-trill-smart-endnodes-02-rtgdir-early-meuric-2015-10-05-00

Request Review of draft-ietf-trill-smart-endnodes
Requested rev. no specific revision (document currently at 11)
Type Early Review
Team Routing Area Directorate (rtgdir)
Deadline 2015-10-05
Requested 2015-09-08
Draft last updated 2015-10-05
Completed reviews Rtgdir Early review of -02 by Julien Meuric (diff)
Genart Telechat review of -08 by Robert Sparks (diff)
Assignment Reviewer Julien Meuric
State Completed
Review review-ietf-trill-smart-endnodes-02-rtgdir-early-meuric-2015-10-05
Reviewed rev. 02 (document currently at 11)
Review result Has Issues
Review completed: 2015-10-05

Review
review-ietf-trill-smart-endnodes-02-rtgdir-early-meuric-2015-10-05

Hello,



I have been selected as the Routing Directorate QA reviewer for this 


draft. For more information about the Routing Directorate, please see 


€‹

http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir








At this stage, the intend of the following is not to discuss the WG's 


decision about the I-D, but rather to help improving its content.






Please not that I am deeply familiar with TRILL, but this was an 


interesting opportunity for me to browse a few RFCs from the WG and 


learn about the technology. Bottom line: I may raise some rookie's 


comments, feel free to educate me if appropriate.




_Summary_



The I-D is well focused on a protocol specification, addressing a 


requirement with clear motivations. I must acknowledge a strong 


improvement from 01 to 02, both in terms of readability and 


specification. Beyond the list of minor issues below, I have one major 


concerns with 02. All the same, from where I stand, it does not look 


like a deep change and may not be that difficult to address.




_Main Issue_



As far as I understand, the I-D now defines a new protocol among those 


to be carried within the RBridge Channel. This is consistent along 


sections 4.1 (encapsulation) and 8 (IANA allocations). What is now 


inconsistent is the use of IS-IS TLVs and sub-TLVs, probably because the 


former version seemed more IS-IS based. Several related problems:


1- The TRILL GENINFO mentioned in the I-D is an IS-IS TLV, the way it 


should be used in the Smart-Hello protocol is unclear.


2- The I-D is requesting a new APPsub-TLV from the GENINFO above, i.e., 


within the IS-IS registry. (a) A new protocol being defined, if the 


IS-IS registry is to be used, this needs to be clarified. (b) It is not 


clear either why the GENINFO top TLV is still useful since the 


information is not conveyed using IS-IS.


3- The I-D specification reuses yet another IS-IS TLV (242) to be 


included in "Smart-Hellos". While defining a TLV content aligned on an 


existing IS-IS TLV seems reasonable (the PCE WG faces a same situation 


with PCEP sometimes pointing to RSVP registry), this TLV must have its 


codepoint allocated as part of the "Smart-Hello" protocol, and values do 


not have to match.


4- Note also that IS-IS registry associates TLV with messages types 


(i.e., IIH, LSP, SNP, etc), that IIH does not support TLV 242 and that 


Smart-Hellos are not part of the list. This is likely to be solved as 


part of addressing 3-, once that TLV is allocated in a Smart-Hello registry.




_Minor Issues_
------
Global
---


- The I-D is currently ST. For a protocol specification, there are too 


few occurrences of RFC 2119 keywords. Either the I-D is not really ST 


and the field should be updated, or the keywords, as well as RFC 2119 


reference (which got dropped from 01 to 02) and introductory quote, 


should be included.


- Capitalized 1st letters on "Smart Endnode" and "Smart-Hello" have been 


generalized, just a few ones were missed (e.g., in sections 2, 4.1, 


etc.). The choice is less clear on the "e/Edge RBridge" phrase: please 


make it consistent along the I-D.



- Some character spacing should be fixed.
------
Header
---
- All authors' first names have a trailing period to be dropped.
------
Section 2
---


- The section extensively use some of the acronyms only defined later. 


The terminology section should be moved before it.



- s/solution proposed/solution defined/
- s/the below figure/the figure below/
- s/attached RB/attached RBridge/
- s/if there was no endnode entry/in the same situation/
- Figure 1: acronyms are enough, full phrases useless and heavy.
- s/RB 1/RB1/ (on figure 1)
- s/issues a Smart-Hello/issues a message called "Smart-Hello"/


- required acronym expansions (fine since terminology if moved before): 


"LSP" (the Routing area loves RFC 5513!), "E-L1FS FS LSP"



------
Section 3
---
- To be moved and extended (see above and below)


- The "TRILL switch" phrase appears in various definitions: is it still 


needed beyond defining "RBridge"? I imagine it was required in early 


days of TRILL work, but now that has its own terminology, sticking to 


"[edge/transit] RBridge" looks clearer. Obviously, this should be 


enforced along the I-D, i.e., avoid "TRILL switch" in the remainder of 


the I-D.



------
Section 4
---
- s/smart end node/Smart Endnode/


- s/both for Smart-Hellos sent by Smart Endnodes and for Smart-Hellos 


sent by Edge RBridges/for Smart-Hellos sent by both Smart Endnodes and 


Edge RBridges/



- s/an 8-bit size and type field/an 8-bit size and 8-bit type fields./


- As pointed out as "main issue", RFC 6823 does not seem applicable here 


(though the idea may).



- s/Endndoes/Endnodes/
- s/APPsub-TLv/APPsub-TLV/


- I am rather used to 32-bit-aligned TLV figures, but the display format 


used here does not harm.


- I wonder why not choosing a fixed header format, using the larger 


size, i.e., 6-bit "RESV" and 24-bit "VLAN/FGL Data Label" in all cases. 


I see more trouble in handling 2 different sizes rather than carrying 


padded values. Moreover, the 2-bit RESV option may quickly become too 


small, since using one of these bits is already considered in section 6.


- s/IS/node/ (Note that if the IS acronym was really to be used, it 


should join the terminology section.)



------
Section 5
---
- RFC 2119 keywords to be considered all along.
- s/nicknamae/nickname/
- s/D either queries/SE1 either queries/ (if I understood well)
- s/DRB/designated RBridge/ (used once, no need for the acronym)


- In section 5.1, the very last sentence of the last paragraph should 


become penultimate: easier to read all expected behaviors first, and the 


crash/restart case only then.


- The phrase "port with a [S]mart [N]ode", used a couple of times, read 


weird. "Port connected to a Smart Node" would ease parsing.



- s/MAC (or Data Label)/MAC or Data Label/


- "would be dropped": an example of loose wording, RFC 2119 keywords are 


expected.



- s/TRILL encapsulation/TRILL packet/


- "Normal endnodes" is not defined (implying smart ones are abnormal?), 


"non-smart endnodes" should be preferred and used each time.



- s/decapsulate the frame/decapsulate the TRILL-encapsulated frame/
- s/two copies/two formats/
- s/A Smart Ennodes/a Smart Endnode/
- s/A normal (non-smart) endnode/A non-smart endnode/
- s/complexity/load/


- s/RECOMMENDED/recommended/ (That is deployment guideline, not protocol 


specification.)



- "Another solution is..." Again, does not fit ST.
------
Section 6
---


- Supposing that the I-D is actually on ST, not less than 3 possible 


solutions are listed! If there is relevant work in progress, then point 


to it and stop. If a mechanism is really missing, then define only one.



- s/RBridge RB1 and RB2/RBridges RB1 and RB2/
- s/multi- homing/multi-homing/
- S/RSV/RESV/
------
Section 8
---


- Including a value is usually a bad idea at this stage. Please use 


"TBD" or start an early allocation procedure to have a stable value to 


mention.



- As mentioned above:
    * Is it really an APPsub-TLV from IS-IS GENINFO that is needed?
    * IS-IS TLV242-like should be allocated as part of Smart-Hello.
------
Section 10
---


- Are those references all normative? I feel I have managed to 


understand the I-D without diving into all of them. (And beware of 


normative I-Ds...)



------


Best regards,

Julien


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.