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

From: Julien Meuric <[email-masked]>
Date: Mon, 2 May 2016 09:51:03 +0200

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 - [email-masked]:
> 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-masked]]
>> 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
>

---

From: Mingui Zhang <[email-masked]>
Date: Tue, 3 May 2016 03:37:09 +0000

Hi Julien,

The updated version has just been uploaded.

Thanks,
Mingui

> Sent: Monday, May 02, 2016 3:51 PM
> To: Mingui Zhang
> Cc: [email-masked]; [email-masked]; [email-masked];
> [email-masked]
> Subject: Re: RtgDir review: draft-ietf-trill-mtu-negotiation-02
> 
> 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.
> 
> >> 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.
> >> 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.
> >>
> >> 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.
> >
> > 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.
> >
> >> - Section 3 specifies an algorithm. Even if it does not rely on a
> >> formal language, consistency would be valuable. My mind compiler would
> suggest:
> >>       * 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";
> >> 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.
> >
> >> - The list may be extended with RFC RFC 6325, RFC 7176 and maybe even
> >> RFC 7780.
> >> ------
> >> Abstract
> >> ---
> >> - "It updates RFC..." duplicates header: either to drop or make more
> >> specific to point to precise sections/mechanisms.
> >> ------
> >> Section 1.
> >> ---
> >> "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]
> >
> >> - "tested size can be advertised": "can" is to be addressed as part
> >> of the loose RFC 2119 wording comment.
> >
> >> - 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.
> >
> >

---

From: Julien Meuric <[email-masked]>
Date: Wed, 4 May 2016 12:36:01 +0200

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

---

From: Mingui Zhang <[email-masked]>
Date: Tue, 10 May 2016 02:35:37 +0000

Hi Julien,

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

Thanks,
Mingui

> Sent: Wednesday, May 04, 2016 6:36 PM
> To: Mingui Zhang
> 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.
> 
> >> 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.
> >>
> >>> Please see in-lines below. An updated version will soon be uploaded
> >>> to resolve
> >> the comments.
> >>>
> >>> Thanks,
> >>> Mingui
> >>>
> >>>> 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.
> >>>>
> >>>> I have some minor concerns about this document that I think should
> >>>> be resolved before publication.
> >>>>
> >>>> *Comments:*
> >>>> - 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.
> >>>
> >>> Actually, I don't think this document updates RFC7176. It simply
> >>> makes use of
> >> the MTU Sub-TLV defined in RFC 7176.
> >>>> That should be either acknowledged or clarified. The 4th paragraph
> >>>> of the introduction tries to tackle that topic, but it is not clear
> >>>>        * 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";
> >>>> - The list may be extended with RFC RFC 6325, RFC 7176 and maybe
> >>>> even RFC 7780.
> >>>> ------
> >>>> Abstract
> >>>> ---
> >>>> - "It updates RFC..." duplicates header: either to drop or make
> >>>> more specific to point to precise sections/mechanisms.
> >>>> ------
> >>>> Section 1.
> >>>> ---

---

From: Julien Meuric <[email-masked]>
Date: Tue, 10 May 2016 10:33:44 +0200

Hi Mingui,

This version seems to address my comment.

Please note a new typo on page 8: s/any LPS/any LSP/

Thanks,

Julien

---

From: Mingui Zhang <[email-masked]>
Date: Tue, 10 May 2016 11:16:41 +0000

Hi Julien, 

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

Thanks,
Mingui

> Sent: Tuesday, May 10, 2016 4:34 PM
> To: Mingui Zhang
> "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.
> >>
> >>>> 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.
> >>>>
> >>>>> Please see in-lines below. An updated version will soon be
> >>>>> uploaded to resolve
> >>>> the comments.
> >>>>>
> >>>>> Thanks,
> >>>>> Mingui
> >>>>>
> >>>>>> 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
> >>>>>> ADs, it would be helpful if you could consider them along with
> >>>>>> any other IETF Last Call comments that you receive, and strive to
> >>>>>> I have some minor concerns about this document that I think
> >>>>>> should be resolved before publication.
> >>>>>>
> >>>>>> *Comments:*
> >>>>>> and one "SHOULD", many of them inherited from specifications out
> >>>>>> of the referenced documents. On the other side, "must" and "should"
> >>>>>> 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.
> >>>>>
> >>>>>>         * "If" is followed by "then" only once: "then" keyword to
> >>>>>> be
> >> dropped?
> >>>>>> - "limited to 1470 to 65,535 bytes": I cannot parse it, is it
> >>>>>> meant to be a
> >> range?
> >>>>>> ------
> >>>>>> Section 4.
> >>>>>> - OLD
> >>>>>> [current wording suggests it is adding an exception to a
> >>>>>> mandatory behavior, which AFAIU it does not]
> >>>>>
> >>>>>> - "tested size can be advertised": "can" is to be addressed as
> >>>>>> part of the loose RFC 2119 wording comment.
> >>>>>
> >>>>>> - 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.
> >>>>>
> >>>>>

---

From: Alia Atlas <[email-masked]>
Date: Tue, 10 May 2016 13:06:03 -0400

Julien,

Thank you very much for your review!

Mingui,

Thank you for addressing the comments.

Regards,
Alia

On Tue, May 10, 2016 at 7:16 AM, Mingui Zhang <[email-masked]>
wrote:

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

---

From: "Susan Hares" <[email-masked]>
Date: Tue, 10 May 2016 17:30:36 -0400

Julien: 

Thank you from one of the TRILL WG chairs. 

Sue 

From: Alia Atlas [mailto:[email-masked]] 
Sent: Tuesday, May 10, 2016 1:06 PM
To: Mingui Zhang
Cc: Julien Meuric; [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: RtgDir review: draft-ietf-trill-mtu-negotiation-02

Julien,

Mingui,

Regards,

Alia

On Tue, May 10, 2016 at 7:16 AM, Mingui Zhang <[email-masked]> wrote:

Hi Julien,

Thanks,
Mingui