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

Robert Sparks <rjsparks@nostrum.com> Fri, 16 May 2014 20:59 UTC

Return-Path: <rjsparks@nostrum.com>
X-Original-To: rtg-dir@ietfa.amsl.com
Delivered-To: rtg-dir@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id EA6171A02D3 for <rtg-dir@ietfa.amsl.com>; Fri, 16 May 2014 13:59:13 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.551
X-Spam-Level:
X-Spam-Status: No, score=-2.551 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-0.651] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GiP9_Jbwkz-w for <rtg-dir@ietfa.amsl.com>; Fri, 16 May 2014 13:59:10 -0700 (PDT)
Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id EAF0D1A0192 for <rtg-dir@ietf.org>; Fri, 16 May 2014 13:59:09 -0700 (PDT)
Received: from unnumerable.local (pool-173-57-107-66.dllstx.fios.verizon.net [173.57.107.66]) (authenticated bits=0) by nostrum.com (8.14.8/8.14.7) with ESMTP id s4GKwrbn023800 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=OK); Fri, 16 May 2014 15:58:54 -0500 (CDT) (envelope-from rjsparks@nostrum.com)
X-Authentication-Warning: raven.nostrum.com: Host pool-173-57-107-66.dllstx.fios.verizon.net [173.57.107.66] claimed to be unnumerable.local
Message-ID: <53767C0D.6090805@nostrum.com>
Date: Fri, 16 May 2014 15:58:53 -0500
From: Robert Sparks <rjsparks@nostrum.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.5.0
MIME-Version: 1.0
To: adrian@olddog.co.uk, "Fedyk, Don" <don.fedyk@hp.com>, draft-ietf-l2vpn-vpls-ldp-mac-opt@tools.ietf.org, Ben Niven-Jenkins <ben@niven-jenkins.co.uk>, "rtg-dir@ietf.org" <rtg-dir@ietf.org>
References: <5342FF4F.4040906@nostrum.com> <031f01cf54d7$b9432180$2bc96480$@olddog.co.uk>
In-Reply-To: <031f01cf54d7$b9432180$2bc96480$@olddog.co.uk>
Content-Type: text/plain; charset="ISO-8859-1"; format="flowed"
Content-Transfer-Encoding: 7bit
Archived-At: http://mailarchive.ietf.org/arch/msg/rtg-dir/w10i04qsrmvaORO0b8IiEx6TIyA
Subject: Re: [RTG-DIR] Gen-art LC review: draft-ietf-l2vpn-vpls-ldp-mac-opt-11
X-BeenThere: rtg-dir@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Routing Area Directorate <rtg-dir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/rtg-dir/>
List-Post: <mailto:rtg-dir@ietf.org>
List-Help: <mailto:rtg-dir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 16 May 2014 20:59:14 -0000

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:l2vpn-bounces@ietf.org] On Behalf Of Robert Sparks
>> Sent: 07 April 2014 20:41
>> To: General Area Review Team;
> draft-ietf-l2vpn-vpls-ldp-mac-opt@tools.ietf.org;
>> l2vpn@ietf.org
>> 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?


>>
>> 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?

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? :

    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


>>
>> 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.
>>
>>
>>
>