Early Review of draft-ietf-trill-mtu-negotiation-02
review-ietf-trill-mtu-negotiation-02-rtgdir-early-meuric-2016-05-20-00

• Status
• Email expansions
• History

• Versions
• 00

Review
review-ietf-trill-mtu-negotiation-02-rtgdir-early-meuric-2016-05-20

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, and 


sometimes on special request. 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-mtu-negotiation-02.txt
Reviewer: Julien Meuric
Review Date: April 27, 2016
IETF LC End Date: April 5, 2016

Intended Status: Standards Track


*Summary:*


I have some minor concerns about this document that I think should be 


resolved before publication.




*Comments:*


Even though it requires to browse some other TRILL (normative) 


documents, the mechanism itself is simple and well described. 


Nevertheless, the specification deserves some improvement when it comes 


to obligations and options: this was part of my expectation after I 


realized it was upgrading a short section of the base document (RFC 


6325), which needs to be emphasized as well.




*Minor Issues:*


- The document is ST and references RFC 2119. There a some "MUST" and 


one "SHOULD", many of them inherited from specifications out of the 


referenced documents. On the other side, "must" and "should" are 


commonly used. This MUST be brought up to ST expectations.


- The document claims to only update RFC 7177. It seems however that it 


also updates RFC 6325 (section 4.3.2), RFC 7176 and maybe even RFC 7780. 


That should be either acknowledged or clarified. The 4th paragraph of 


the introduction tries to tackle that topic, but it is not clear enough 


in defining the position of the document with respect to previously 


defined mechanisms.


- The 3rd paragraph of the introduction duplicates the beginning of the 


following section 2. A possible way to limit this repetition effect may 


be to summarize that part of the introduction.


- Section 3 specifies an algorithm. Even if it does not rely on a formal 


language, consistency would be valuable. My mind compiler would suggest:



    * "If" is followed by "then" only once: "then" keyword to be dropped?


    * The algorithm relies on a break/stop or continue principle; as 


such, the instance of "Else" at the end should be replaced by "<line 


break> 4) Repeat Step1";



    * "is set to" and "<--" seem to be similar: please pick one or clarify;


    * to improve readability, I would drop the double naming introduced 


with X, X1 and X2 and rely on explicit variable names all along, as in 


the text: e.g., "linkMtuSize" instead of X, "lowerBound" for X1 and 


"upperBound" instead of X2.




*Nits:*
------
"Updates" field in header
---
- I think the "RFC" acronym should appear.


- The list may be extended with RFC RFC 6325, RFC 7176 and maybe even 


RFC 7780.



------
Abstract
---
- s/campus wide MTU feature/campus-wide MTU feature/
- s/campus wide capability/campus-wide capability/
- s/link local packets/link-local packets/
- s/link local MTUs/link-local MTUs/


- "It updates RFC..." duplicates header: either to drop or make more 


specific to point to precise sections/mechanisms.



------
Section 1.
---
- s/link scope PDUs can/link-scoped PDUs can/
------
Section 2.
---
- s/campus wide Sz MTU/campus-wide Sz MTU/
- s/area wide scope/area-wide scope/
- s/domain wide scope/domain wide scope/
- s/L1 Circuit Scoped/L1 Circuit-Scoped/


- "limited to 1470 to 65,535 bytes": I cannot parse it, is it meant to 


be a range?



------
Section 4.
- OLD


"while RB1 normally ignores link state information for any IS-IS 


unreachable RBridge RB2, originatingL1LSPBufferSize is an exception."



  NEW


"while in most cases RB1 ignores link state information for IS-IS 


unreachable RBridge RB2, originatingL1LSPBufferSize is meaningful."


[current wording suggests it is adding an exception to a mandatory 


behavior, which AFAIU it does not]



------
Section 7.
---


- "tested size can be advertised": "can" is to be addressed as part of 


the loose RFC 2119 wording comment.



------
Section 8.
---


- "value [...] had been reported": "reported" puzzles me, maybe "tested" 


was meant?


- The 3rd paragraph "For an Lz-ignorant [...] link-wide Lz." should be 


moved up to become the second paragraph, so as to clarify what an 


Lz-ignorant is.


- "The extension of TRILL MTU negociation...": this is an explicit 


positioning which should be mentioned earlier in the I-D.



------
Section 10.
---
- RFC 7180 bis is now RFC 7780.
---

Regards,

Julien