Re: [trill] RtgDir review: draft-ietf-trill-mtu-negotiation-02

[Mingui Zhang <[e-mail]>]

Hi Julien,

I have uploaded the 04 version. Please take a look.

Thanks,
Mingui

> -----Original Message-----
> From: Julien Meuric [mailto:[e-mail]]
> Sent: Wednesday, May 04, 2016 6:36 PM
> To: Mingui Zhang
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: RtgDir review: draft-ietf-trill-mtu-negotiation-02
> 
> Hi Mingui,
> 
> I have looked at the diff of your update and have noticed that several "should"
> have been replaced by "need[s] to": I appreciate that you acknowledge my
> comment on this, but none of them being RFC 2119 keyword, I still doubt this
> matches Standards Track expectations.
> 
> I caught a nit in section 3: s/is sent to/is set to/
> 
> Regards,
> 
> Julien
> 
> 
> May. 03, 2016 - [e-mail]:
> > Hi Julien,
> >
> > The updated version has just been uploaded.
> >
> > Thanks,
> > Mingui
> >
> >> -----Original Message-----
> >> From: Julien Meuric [mailto:[e-mail]]
> >> Sent: Monday, May 02, 2016 3:51 PM
> >>
> >> Hi Mingui,
> >>
> >> About the "updated RFCs" issue below, my point is: "please make sure
> >> the text and the hearder are complete". I just had a quick look at
> >> those references, you know better than me which are actually relevant.
> >>
> >> I am looking forward to the updated version.
> >>
> >> Thanks,
> >>
> >> Julien
> >>
> >>
> >> Apr. 29, 2016 - [e-mail]:
> >>> 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:[e-mail]]
> >>>> 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
> >>>