Re: [trill] Genart last call review of draft-ietf-trill-mtu-negotiation-05

Donald Eastlake <d3e3e3@gmail.com> Tue, 27 June 2017 03:02 UTC

Return-Path: <d3e3e3@gmail.com>
X-Original-To: trill@ietfa.amsl.com
Delivered-To: trill@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id C5147128896; Mon, 26 Jun 2017 20:02:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.449
X-Spam-Level:
X-Spam-Status: No, score=-2.449 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
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 xKg0B8nQ-tAD; Mon, 26 Jun 2017 20:02:50 -0700 (PDT)
Received: from mail-io0-x229.google.com (mail-io0-x229.google.com [IPv6:2607:f8b0:4001:c06::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 0BC8A128854; Mon, 26 Jun 2017 20:02:50 -0700 (PDT)
Received: by mail-io0-x229.google.com with SMTP id z62so10926960ioi.3; Mon, 26 Jun 2017 20:02:50 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=xk3loawHtrRMTl5VZ0TK1CSeuPzglTz6W37yDd7eJC0=; b=dPQnZkwASPlKAuPArYjDZ9E0Drz3F+8iY5aLfwNCO9aIbQA6XMu+Vu0iNmDfj9duKr tg2v6AolHqZavdRFQJSS/m5wimjHGukZGaD3vUOz2cz/cgrDZ+SLNDcV4VH9meAcxZhp /2q+Bx2Vk9ZaDJ1IfvP6nzj8zS2DTVoZTWKWbkz6ZpXPhXyTRSdybAZ7Y1KsIzC01mc2 B0nQn1E/OTCe8AcRbNTf7BnqZWvRy43svdX/ljY5ufpoEuAPK5LwpI0BebuwDtz+Z+si 3kRnmT1RNH9PVWyiDu+tourp3Hq5XJNSi2fNnm2CpPANcamgpU1oz8QR+3vAjDNlXQeR Wsag==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=xk3loawHtrRMTl5VZ0TK1CSeuPzglTz6W37yDd7eJC0=; b=Gc7Rb+WYQPBcWn0VBOhev2anvCgQY44ehQkhzAS1AaomffB/nPT0InUDu3wWcT1Lr3 h++yd1091/s7h2e8vMgvBdLVKb5NhNHMRxqVGcaYMig4KJPoi61B7zlgtP6+3gC5Ry/j tHRaRBeaXNNqAt1oEwCEhVuMViyH6uzhNVfUmjEZkDicPIUX8Ska1FxhZ14d41aJJAdB QJiVoQkzpuI5JhMH6v04bwd8o/riyTWy2b8JHjD5tFeSuD5NmzprwE6RIMpvfreaJfkW UFqEYrnaUFRdmO0xeWlNr0lYJiLOcrOB9kjE1Tn2j3uy+j2F6yi1BjYMFc4kEKCmPWX2 VZNg==
X-Gm-Message-State: AKS2vOwb3bBOkRjkIRySJsgtit7noINY+IyOvn5A3cF1P6sIhxq3dkh7 gVmtBNkAnUAjxIT2gq4meT3de7QGew==
X-Received: by 10.107.31.209 with SMTP id f200mr4626629iof.231.1498532569237; Mon, 26 Jun 2017 20:02:49 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.107.17.221 with HTTP; Mon, 26 Jun 2017 20:02:33 -0700 (PDT)
In-Reply-To: <149826797586.7852.1637929087755767570@ietfa.amsl.com>
References: <149826797586.7852.1637929087755767570@ietfa.amsl.com>
From: Donald Eastlake <d3e3e3@gmail.com>
Date: Mon, 26 Jun 2017 23:02:33 -0400
Message-ID: <CAF4+nEHfdCVoLcoKO-occ++pC7QUY2AwbKM6iuLU-K1s=YCd3g@mail.gmail.com>
To: Brian Carpenter <brian.e.carpenter@gmail.com>
Cc: "gen-art@ietf.org Review Team" <gen-art@ietf.org>, draft-ietf-trill-mtu-negotiation.all@ietf.org, "trill@ietf.org" <trill@ietf.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://mailarchive.ietf.org/arch/msg/trill/tKoeCxAyjBBeQojY2QP7s7PBeDI>
Subject: Re: [trill] Genart last call review of draft-ietf-trill-mtu-negotiation-05
X-BeenThere: trill@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: "Developing a hybrid router/bridge." <trill.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/trill>, <mailto:trill-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/trill/>
List-Post: <mailto:trill@ietf.org>
List-Help: <mailto:trill-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/trill>, <mailto:trill-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 27 Jun 2017 03:02:53 -0000

Hi Brian,

Thanks for the comments. See below.

On Fri, Jun 23, 2017 at 9:32 PM, Brian Carpenter
<brian.e.carpenter@gmail.com> wrote:
> Reviewer: Brian Carpenter
> Review result: Ready with Issues
>
> Gen-ART Last Call review of draft-ietf-trill-mtu-negotiation-05
>
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
>
> For more information, please see the FAQ at
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>
> Document: draft-ietf-trill-mtu-negotiation-05.txt
> Reviewer: Brian Carpenter
> Review Date: 2017-06-24
> IETF LC End Date: 2017-06-28
> IESG Telechat date: 2017-07-06
>
> Summary: Ready with issues
> --------
>
> Minor issues:
> -------------
>
>> 2. Link-Wide TRILL MTU Size
> ...
>>   ...RBridges MUST support the Extended L1 Circuit-Scoped
>>   (E-L1CS) flooding scope LSP (FS-LSP). They use that flooding to
>>   exchange their maximally supportable value of "Lz".
>
> Where does that value come from? Is it configured, derived from
> the interface in some way, or discovered?

It's somewhat similar to the originatingL1LSPBufferSize which is
talked about in Section 5 of RFC 7780, except that there is no reason
to worry about coordinating across the TRILL campus. How about adding
wording something like:

      The originatingSNPBufferSize for a port is the minimum of the
following two quantities, but not less than 1470 bytes: (1) the
maximum MTU of the port and (2) the maximum LSP size that the TRILL
IS-IS implementation can handle,

>> 2.1. Operations
>>
>>   Lz is reported using a originatingSNPBufferSize TLV that MUST occur
>>   in fragment zero of the RBridge's E-L1CS FS-LSP. An
>>   originatingSNPBufferSize APPsub-TLV occurring in any other fragment
>>   is ignored.
>
> Is that really what you mean? I thought Lz was an optional extra. So I think
> you mean:
>
> 2.1. Operations
>
>    Lz MAY be reported using a originatingSNPBufferSize TLV that occurs
>    in fragment zero of the RBridge's E-L1CS FS-LSP. An
>    originatingSNPBufferSize APPsub-TLV occurring in any other fragment
>    MUST be ignored.

Yes, the "MUST" was just in reference to being in fragment zero, not
that it has to occur, so your wording seems better.

>> 3. Link MTU Size Testing
> ...
>>   Step 0:
> ...
>>      b) Link MTU size is set to 1470, lowerBound is set to 1470,
>>         upperBound is set to the link-wide Lz, linkMtuSize is set to
>>         [(lowerBound + upperBound)/2] (Operation "[...]" returns the
>>         fraction-rounded-up integer.).
>
> This is confusing. "linkMtuSize" was defined as a local variable.
> But what is "Link MTU size"? Is that another local variable?
> If so, how is it different from "linkMtuSize"?
> It is used again in part 2) of step 2 below.

I don't want to say anything about that before checking with the primary author.

> Also, I assume "Lz" is the value previously agreed among the nodes,
> but that should be made clear to the reader.
>
> Nits:
> -----
>
>> 1. Introduction
> ...
>>   topology. While in this document, a new RECOMMENDED link-wide minimum
>>   inter-RBridge MTU size, Lz, is specified. By calculating a using Lz
>>   as specified herein, link-scoped PDUs can be formatted greater than
>>   the campus-wide Sz up to the link-wide minimum acceptable inter-
>>   RBridge MTU size potentially improving the efficiency of link
>>   utilization and speeding link state convergence.
>
> I cannot parse those two sentences. What does the "While" refer to?
> What does "By calculating a using Lz" mean?

I believe the sentences should be

"... In this document, a new RECOMMENDED link-wide minimum
inter-RBridge MTU size, Lz, is specified. By calculating and using Lz
as specified herein, ..."

>> 3. Link MTU Size Testing
> ...
>>      b) Link MTU size is set to 1470, lowerBound is set to 1470,
>>         upperBound is set to the link-wide Lz, linkMtuSize is set to
>>         [(lowerBound + upperBound)/2] (Operation "[...]" returns the
>>         fraction-rounded-up integer.).
>
> This would be easier to understand:
>
> 3. Link MTU Size Testing
> ...
>       b) Link MTU size is set to 1470, lowerBound is set to 1470,
>          upperBound is set to the link-wide Lz, linkMtuSize is set to
>          [(lowerBound + upperBound)/2], rounded up to the nearest integer.

OK.

> Repeat this in the following two cases; a normal reader will not
> remember the rounding rule:
>
> ...
>    1) If RB1 fails to receive an MTU-ack from RB2 after k tries:
>
>          upperBound is set to linkMtuSize and linkMtuSize is set to
>          [(lowerBound + upperBound)/2], rounded up to the nearest integer.
>
>    2) If RB1 receives an MTU-ack to a probe of size linkMtuSize from
>       RB2:
>
>          link MTU size is set to linkMtuSize, lowerBound is set to
>          linkMtuSize and linkMtuSize is set to [(lowerBound +
>          upperBound)/2], rounded up to the nearest integer.

That seems reasonable.

> --

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 155 Beaver Street, Milford, MA 01757 USA
 d3e3e3@gmail.com