Subject: Re: [RTG-DIR] Gen-art LC review: draft-ietf-l2vpn-vpls-ldp-mac-opt-11

From: Robert Sparks <[email-masked]>
Date: Tue, 03 Jun 2014 13:41:23 -0500

Hi Don (sorry for the slow reply):

I think we've converged on everything but how the document is, as you 
say, walking the line on what it's recommending for incremental deployment.
I still find the text inconsistent on that point, but will defer to 
Adrian on whether anything needs to be done about it.

Thanks for the effort you've put into getting this finished!

RjS

On 5/19/14, 2:30 PM, Fedyk, Don wrote:
> Hi Robert
>
> I've updated to address Sue's points to try and keep that part clear I'll do that in another email.
> Since your questions are a bit different I have addressed them here inline.
>
> Inline [Don]
>
> Regards,
> Don
> -----Original Message-----
> From: Robert Sparks [mailto:[email-masked]]
> Sent: Friday, May 16, 2014 4:59 PM
> To: [email-masked]; Fedyk, Don; [email-masked]; Ben Niven-Jenkins; [email-masked]
> Subject: Re: Gen-art LC review: draft-ietf-l2vpn-vpls-ldp-mac-opt-11
>
> Hi Don -
>
> Comments inline based on the diff you sent in another thread:
>
> On 4/10/14, 11:12 AM, Adrian Farrel wrote:
>> Thanks Robert,
>>
>> Will be interested to hear author/shepherd responses to this.
>>
>> Adrian
>>
>>> -----Original Message-----
>>> From: L2vpn [mailto:[email-masked]] On Behalf Of Robert Sparks
>>> Sent: 07 April 2014 20:41
>>> To: General Area Review Team;
>> [email-masked];
>>> [email-masked]
>>> Subject: Gen-art LC review: draft-ietf-l2vpn-vpls-ldp-mac-opt-11
>>>
>>> I am the assigned Gen-ART reviewer for this draft. For background on
>>> Gen-ART, please see the FAQ at
>>>
>>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>>>
>>> Please resolve these comments along with any other Last Call comments
>>> you may receive.
>>>
>>> Document: draft-ietf-l2vpn-vpls-ldp-mac-opt-11
>>> Reviewer: Robert Sparks
>>> Review Date: 7Apr2014
>>> IETF LC End Date: 8Apr2014
>>> IESG Telechat date: not yet scheduled
>>>
>>> Summary: This draft is almost ready for publication as a Proposed
>>> Standard, but has minor issues (primarily editorial) that should be
>>> addressed.
>>>
>>> I found this document very difficult to read. It asks the reader to hop
>>> between sections in unusual ways (for instance, it sends the reader to
>>> the problem statement section for details on normative behavior).
> That part has been fixed. It's much less of a struggle to follow now,
> but it's
> still not easy (at least for me, not working this particular technology
> all the time).
>    I don't see simple things to change to improve that though.
>>>    I
>>> strongly encourage an editorial pass focusing on document structure.
>>>
>>> There are many instances of SHOULD in the document where the text should
>>> just be using prose instead. It's not always clear when an
>>> implementation would choose to ignore the SHOULD, and what the
>>> consequences of that choice would be.
> Thanks for the general inspection of SHOULDs here.
>
> You changed the SHOULD in the first paragraph of 5.1.3 to a MUST, but
> you left the one in the second paragraph as a SHOULD. Is there a case where
> the recipient _wouldn't_ follow the rules in this section?
> [Don]  Hmm. In general you are correct.  To me, in my experience,  MAC flushing is a bit of an area where creativity comes in.   Some devices can move MACs to other interfaces instead of flushing so to me having a spec that says you SHOULD flush allows other options but saying you MUST flush is restrictive.
>
>
>>> The document is inconsistent about the level of support needed in the
>>> network before trying to use this extension.
>>> Section 5.1.2 says the assumption is everything understands it before
>>> it's turned on. Section 6 points back to figure 2 and says
>>> to use the extension over the pw where you administratively know the
>>> peer supports the extension, and fall back to 4762 for
>>> everything else. Which of those did you intend?
> This is still confusing to me. I must be misreading something.
> In 5.1.2, you have:
>
>      Note that the assumption is the MAC flush TLV is understood by all
>      devices before it is turned on in any network.
>
> Doesn't that preclude the kind of incremental deployment you talk about
> in section 6 with:
>
>      Thus in the topology described
>      in figure 2, an implementation could support PE1-rs sending optimized
>      MAC Flush towards the PE-rs devices that support the solution and
>      PE2-rs device initiating [RFC4762] style of MAC Flush towards the PE-
>      rs devices that do not support the optimized solution during
>      upgrades.
>
> "devices that do not support" and "understood by all devices" don't seem
> to go together. Is the sentence in 5.1.2 perhaps not scoped as intended?
>
> [Don] the text is walking the line between:
> 1. How we recommend to deploy
> 2. What happens (at worst) if you don't deploy the way we recommended.
>
> Stated another way, why does the very next paragraph in 5.1.2 have a SHOULD
> instead of a MUST if the assumption is that all devices understand the
> optimization before you turn it on? :
> [Don] It is the style of the text.
> If you want to deploy you SHOULD do it this way but MUST is not necessary if you are willing to live with the consequences.
>   
>
>      The MAC withdraw procedures defined in [RFC4762], where either the
>      MTU-s or PE2-rs send the MAC Withdraw message SHOULD be used in cases
>      where the network is being upgraded until all devices are capable of
>      understanding the optimized MAC flush.
>
>
> For the rest, things that I don't comment on are sufficiently addressed
> - thanks!
>
>>> Specific comments in document order:
>>>
>>> Section 3.2 paragraph 1: This paragraph would benefit from being broken
>>> into several. It's hard to find its point. The SHOULD in this paragraph
>>> is probably not a 2119 SHOULD (this section isn't defining the
>>> protocol). It would be useful in this overview to explicitly say _why_
>>> "This cannot be achieved with ... 4762]" at this point in the document.
>>>
>>> Section 3.2 paragraph 2: This SHOULD _is_ defining protocol - shouldn't
>>> it be in section 5?
> You turned this 'SHOULD' into a 'should', but I don't find where the
> analogous
> normative statement is in section 5. (searching for the word "affected"
> doesn't
> turn it up anyhow - I could just be not seeing it from reading to
> quickly - is it
> easy to point to?)
>>> Section 4.1.1 paragraph 3: It took me some time to find Z on the figure.
>>> It might help to introduce it similar to how you introduce X.
>>>
>>> Section 4.1.2: paragraph 1: I think you meant to reference 4.1.1
>>>
>>> Section 5: The first sentence talks about requirements in section 4.
>>> Section 4 describes a problem using some examples but doesn't explicitly
>>> call out requirements. Doing so would help the document.
>>>
>>> Last sentence in 5.1.1 (and several other places in the document):
>>> Please add an article before "MAC Flush message".  (I apologize for such
>>> a small nit, but each of these instances made making sure I was reading
>>> what the sentence intended significantly more difficult).
> Thanks for making the changes in the new document - they really help.
>
> Just in case, please check these places to see if an article would be
> helpfule there too?
>
>      This section describes the processing rules of MAC Flush TLV that
>
>      full mesh with MAC Flush TLV that carries N=1.  Other PE-rs devices
>
>      - The usage and processing rules of MAC Flush Parameters TLV in the
>
>      If MAC Flush Parameters TLV is received by a Backbone Edge Bridges
>
>      As mentioned before, if MAC Flush Parameters TLV is not understood by
>
>
> [Don] Will update.
>
>
>>> Section 5.1.2 first paragraph: This section is defining behavior - why
>>> are you sending the reader back into the problem statement for detail on
>>> the behavior?
>>>
>>> 5.1.2 paragraph 2: You meant section 6, not 5.
>>>
>>> 5.1.2 paragraph 3: I can't follow this paragraph's structure. I think
>>> you're trying to say "An MTU-s or PE2-rs SHOULD send MAC withdraw
>>> messages as defined in [RFC4762] in cases where the network is being
>>> upgraded and devices are not capable of understanding the optimized MAC
>>> flush." (But if so, the next sentence is redundant.) Why is this SHOULD?
> (see above)
>>> 5.1.3 paragraph 1: Why is this a SHOULD and not a MUST? (Similar
>>> question for the SHOULD in paragraph 2). It's not clear if you're trying
>>> to avoid "Some things won't implement this spec" or "Don't do this if
>>> you haven't administratively ensured every element understands this
>>> extension first" or something else?
>>>
>>> 5.1.3 paragraph 3: You say "unless specified otherwise". Do you ever
>>> specify otherwise? Why is this disclaimer here?
>>>
>>> 5.1.3 last paragraph: You meant section 6 not 5.
>>>
>>>
>>>