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

[Mingui Zhang <[e-mail]>]

Hi Julien, 

Thanks for pointing it out. That typo will be fixed together with other comments we would receive.

Thanks,
Mingui

> -----Original Message-----
> From: Julien Meuric [mailto:[e-mail]]
> Sent: Tuesday, May 10, 2016 4:34 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,
> 
> This version seems to address my comment.
> 
> Please note a new typo on page 8: s/any LPS/any LSP/
> 
> Thanks,
> 
> Julien
> 
> 
> May. 10, 2016 - [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
> >>
> >> 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
> >>>>>