Hi Julien,
Thanks for the comments! Much appreciated!
Please see in-lines below. An updated version will soon be uploaded to resolve
the comments.
Thanks,
Mingui
-----Original Message-----
From: Julien Meuric [mailto:[email protected]]
Sent: Thursday, April 28, 2016 4:34 AM
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.
In the abstract, it's already mentioned as "optional updates" to RFC 6325. I think
"Updates: 6325 " needs to appear in the page header 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 usage of "must" and "should" will be updated as needed. I have checked all
those appearances. The changes would be editorial.
- 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.
Actually, I don't think this document updates RFC7176. It simply makes use of
the MTU Sub-TLV defined in RFC 7176.
The specification about the originatingL1LSPBufferSize of an unreachable
RBridge is a slight update to RFC7780. This will be emphasized.
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 updated to RFC 6325 will be emphasized in this paragraph. The backward
compatibility will be moved to here as well. It will help the positioning.
- 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.
Yes, summarization is the proper way.
- 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.
Sure. Explicit names will be used for the sake of readability.
*Nits:*
The following nits will be fixed as suggested.
------
"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]
OK. Will update the words.
------
Section 7.
---
- "tested size can be advertised": "can" is to be addressed as part of the loose
RFC 2119 wording comment.
Will use the word "MAY" instead.
------
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.
OK. It will be moved up.
- "The extension of TRILL MTU negociation...": this is an explicit positioning
which
should be mentioned earlier in the I-D.
OK. This will be moved to the introduction.
------
Section 10.
---
- RFC 7180 bis is now RFC 7780.
Yes, this will be updated.
---
Regards,
Julien
