Re: [Gen-art] Gen-ART Last Call review of draft-ietf-trill-ia-appsubtlv

Paul Kyzivat <pkyzivat@alum.mit.edu> Mon, 04 July 2016 22:50 UTC

Return-Path: <prvs=699331ff0d=pkyzivat@alum.mit.edu>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D6D16128E19; Mon, 4 Jul 2016 15:50:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -5.627
X-Spam-Level:
X-Spam-Status: No, score=-5.627 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RP_MATCHES_RCVD=-1.426, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
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 07t6uDT5KrwT; Mon, 4 Jul 2016 15:50:50 -0700 (PDT)
Received: from alum-mailsec-scanner-4.mit.edu (alum-mailsec-scanner-4.mit.edu [18.7.68.15]) by ietfa.amsl.com (Postfix) with ESMTP id 2E38312B028; Mon, 4 Jul 2016 15:50:50 -0700 (PDT)
X-AuditID: 1207440f-dabff7000000613e-44-577ae848d1f4
Received: from outgoing-alum.mit.edu (OUTGOING-ALUM.MIT.EDU [18.7.68.33]) by (Symantec Messaging Gateway) with SMTP id D8.5B.24894.848EA775; Mon, 4 Jul 2016 18:50:49 -0400 (EDT)
Received: from Paul-Kyzivats-MacBook-Pro.local (c-73-218-51-154.hsd1.ma.comcast.net [73.218.51.154]) (authenticated bits=0) (User authenticated as pkyzivat@ALUM.MIT.EDU) by outgoing-alum.mit.edu (8.13.8/8.12.4) with ESMTP id u64Momeg018030 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 4 Jul 2016 18:50:48 -0400
To: Donald Eastlake <d3e3e3@gmail.com>
References: <392967e6-b056-ef84-dbe4-5adf7469a641@alum.mit.edu> <CAF4+nEFNUZA6gGA0P3v-CjABYG8gUdW0mqRt13LzQ47-mgSG6A@mail.gmail.com>
From: Paul Kyzivat <pkyzivat@alum.mit.edu>
Message-ID: <3840099b-5151-5be6-c164-91ac8362ea57@alum.mit.edu>
Date: Mon, 4 Jul 2016 18:50:46 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0) Gecko/20100101 Thunderbird/45.1.1
MIME-Version: 1.0
In-Reply-To: <CAF4+nEFNUZA6gGA0P3v-CjABYG8gUdW0mqRt13LzQ47-mgSG6A@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpgleLIzCtJLcpLzFFi42IRYndR1PV8URVu8OK5iMXB7ZoWy6/NYLe4 +uoziwOzx85Zd9k9liz5yRTAFMVtk5RYUhacmZ6nb5fAnbHxjH7BdceKSzPvszYwXjPuYuTk kBAwkXhzfy1zFyMXh5DAVkaJF6tuMkE4T5gk7rb0soJUCQsEScz5uIARxBYRUJN4vXwBC0RR G6PE+6kTgDo4OJgFUiQWX1YEqWET0JKYc+g/C4jNK2Avse8dRC+LgIrEpsdrweKiAmkS61u+ MkLUCEqcnPkELM4pECixsm0OmM0sYCYxb/NDZghbXmL72znMExj5ZyFpmYWkbBaSsgWMzKsY 5RJzSnN1cxMzc4pTk3WLkxPz8lKLdE30cjNL9FJTSjcxQkKTfwdj13qZQ4wCHIxKPLwnFlaF C7EmlhVX5h5ilORgUhLlbe8FCvEl5adUZiQWZ8QXleakFh9ilOBgVhLhrX8GlONNSaysSi3K h0lJc7AoifOqL1H3ExJITyxJzU5NLUgtgsnKcHAoSfA2PgdqFCxKTU+tSMvMKUFIM3Fwggzn khIpTs1LSS1KLC3JiAdFZHwxMCZBUjxAe7eCtPMWFyTmAkUhWk8xKkqJ83aAJARAEhmleXBj YQnnFaM40JfCvBkgVTzAZAXX/QpoMBPQYNbYcpDBJYkIKakGRu/t5Zme06Xb74b+mfX58pLE TtkZxfO85znmBb22yHuwQGJGfaDfn1iDmjW9XeabkgVnHNktM/fc5l+bN+t11Jrpz3W+8ON4 2I3V7R/WTpX/d/nzuxVddnVPriff9EzU+VPj/l9zx/c/fGt29bp3tz6eO3P+ntomjY2bty2K mjF59pdLq7N3rvFVYinOSDTUYi4qTgQAO/1XzRMDAAA=
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/miOSAqSF0d-bEe1n4CwdnvTR-vQ>
Cc: General Area Review Team <gen-art@ietf.org>, draft-ietf-trill-ia-appsubtlv.all@ietf.org
Subject: Re: [Gen-art] Gen-ART Last Call review of draft-ietf-trill-ia-appsubtlv
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 04 Jul 2016 22:50:53 -0000

Donald,

On 7/4/16 5:26 PM, Donald Eastlake wrote:
> Hi Paul,
>
> Thanks for your comments. Sorry for the delay in response.
> Please see below.

No problem. I was just concerned that my review hadn't been received.

> On Mon, Jun 27, 2016 at 6:46 PM, Paul Kyzivat <pkyzivat@alum.mit.edu>
> wrote:
>> 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-ia-appsubtlv
>> Reviewer: Paul Kyzivat
>> Review Date: 2016-06-27
>> IETF LC End Date: 2016-06-28
>> IESG Telechat date: 2016-07-07
>>
>> Summary:
>>
>> This draft is on the right track but has open issues, described in
>> the review.
>>
>> This is a well written document. I was generally able to follow it
>> even though I know nothing about the subject.
>>
>>
>> Issues:
>>
>> Major: 0
>> Minor: 7
>> Nits:  2
>>
>> (1) MINOR: (Section 2)
>>
>> "Addr Sets End" is described as follows:
>>
>>    o  Addr Sets End: The unsigned integer offset of the byte, within
>>       the IA APPsub-TLV value part, of the last byte of the last
>>       Address Set. This will be the byte just before the first
>>       sub-sub-TLV if any sub-sub-TLVs are present ...
>>
>> But the remaining text of this section, and the examples, imply that
>> this is really the length of the leading portion of this TLV ending
>> with the last Address Set. The programmer in me says these differ by
>> one, and that the implied definition is the reasonable one, while
>> the action definition, and the name used to identify it, are wrong.
>>
>> I expect it would be difficult at this point to rename this field,
>> but at least the definition can be rewritten to be consistent with
>> the intended usage.
>
> Right. How about
>
>    Addr Sets End: The unsigned integer byte number, within the IA
>    APPsub-TLV value part, of the last byte of the last Address Set,
>    where the first byte is numbered 1. This will be the number of the
>    byte just before ...

OK. If you count starting from one. (I don't, but it is your draft.)

>> (2) MINOR: (Section 5.1)
>>
>> Normally I would expect this section to request IANA to assign new
>> values from the AFN table for OUI...RBridge Port ID. However it is
>> worded as "IANA has allocated". Perhaps this is because they have
>> already been (pre)allocated. I have no problem with that if IANA is
>> OK with it.
>
> Yup, it say "IANA has allocated" because they are already allocated. See
> http://www.iana.org/assignments/address-family-numbers/address-family-numbers.xhtml

OK.

>> But IMO the references to IPv4...64-bit MAC are gratuitous and
>> inappropriate in an IANA Considerations section. If it is desired to
>> include a list of "useful" AFN values then that belongs in some
>> other portion of the document.
>
> I disagree. It's "IANA Considerations", not "IANA Allocation Actions".
> Someone looking for code points is likely look in the IANA
> Considerations section.  All the values shown are from the same IANA
> registry.  I can see no advantage to splitting this table between two
> different parts of the draft.

When I wrote this comment I had in mind the following that I recently read:

https://tools.ietf.org/html/draft-leiba-cotton-iana-5226bis-15#section-1.1

>> (3) MINOR: (Section 5.1)
>>
>> The "new" values here (OUI, MAC/24, MAC/40, IPv6/64) give "This
>> document" as their reference. But anyone consulting the IANA
>> registry and following it to this document would have difficulty
>> finding any *definition* of these things.
>>
>> Section 6 discusses some operational issues with them, but at best
>> implies a definition. (RFC7042 might be considered a definition of
>> OUI, though it doesn't seem to say how big it would be.)
>>
>> I think what is needed are explicit definitions of all of these,
>> including their widths. (In order to provide enough bits to complete
>> a MAC/24 it must be at least 24 bits wide, but that would be bigger
>> than needed for a MAC/40.  So I guess it must be at least 24 bits,
>> and when used to expand a MAC/24 or MAC/40 an appropriate number of
>> its high order bits are used.)
>>
>> It would be good for there to be a section, appearing in the TOC,
>> for each of these so that someone coming here from the IANA registry
>> will easily be able to find the definition.
>
> This is a good point. Better definitions of these AFN types and better
> references, either to within this document by explicit pointers to a
> section within another document or both, are good points. Probably
> Section 6 should be expanded and sub-sections added to it...

WFM

>> (4) MINOR: (Section 5.2)
>>
>> This section defines a new registry with Expert Review as the
>> procedure for approving new entries. What I don't see is any
>> guidance to the expert on appropriate criteria to use to judge
>> suitability of new entries. Without any guidance, relying on the
>> whim of the expert can lead to variable, and perhaps biased,
>> results.
>>
>> It would be good to give guidance on: what sorts of document
>> reference are acceptable, what information needs to be included in
>> the reference document, whether "special" values may be requested
>> (versus just assignment in order requests are received), and the
>> sorts of properties that are appropriate.
>
> OK. Some guidance can be added.
>
>> (5) MINOR: (Section 6)
>>
>> This section talks about the handling of OUI and IPv6/64 when they
>> appear in a Fixed Address sub-sub-TLV. It says nothing about their
>> meaning if these appear elsewhere, such as in a Template. I presume
>> this kind of usage is nonsense, but it would be better to explicitly
>> state it.
>
> OK, the draft should explain their processing wherever they occur.
>
>> (6) MINOR: (Section 6)
>>
>> The description of IPv6/64 says:
>>
>>    For this purpose, an 48-bit MAC address is expanded to 64
>>    bits as described in [RFC7042].
>>
>> It wasn't entirely apparent to me what part of 7042 covers that. It
>> would be helpful to provide the section where this aspect is
>> specified. (After some study I guess that it is section 2.2.1.)
>
> OK.
>
>> (7) MINOR: (Section A.2)
>>
>> I believe that the values of both 'Length' and 'Address Sets End'
>> are too small by 7 - presumably because they forgot to count the
>> fixed fields. This also applies to the "alternative" using explict
>> AFN encoding.
>
> Thanks for catching that there is an error here.
>
> Length should be the size everything after the 2-byte length
> field. That's
>   7  fixed fields
>  36  three address sets, each 12 bytes
>   7  sub-sub-tlv one
>  14  sub-sub-tlv two
> for a total of 64 so the value is off by 10.
>
> Address Sets End should be the above less the sub-sub-tlvs, so that
> would be 43 and the value shown is also off by 10.

I guess I also got it wrong.

But it was obvious to me that the examples weren't all done the same way.

>> (8) NIT: (Section A.2)
>>
>> Based on a very quick reading, ISTM that section 2.2.1 of 7042
>> suggests that the IPv6 addresses being constructed this way should
>> start with 0x02 rather than 0x20. But I'm far from sure I understand
>> this correctly.
>
> Ahhh, there is indeed an error here but it is in the bottom 64 bits,
> which should be a Modified EUI-64 identifier, as described in Section
> 2.2.1 of RFC 7042. Thus the top byte of the bottom 64 bits of the
> resulting IPv6 addresses should be 0x02. The top byte of the entire
> IPv6 128-bit address should be 0x20 as shown.

OK. Like I said, I didn't really understand the details.

	Thanks,
	Paul

>> (9) NIT: (Section A.2)
>>
>> There seems to be a typo in the following:
>>
>>    The OUI would them be supplied
>>    by a second Fixed Address sub-sub-TLV proving the OUI.
>>
>> I think "proving" should be "providing".
>
> OK.
>
> Thanks,
> Donald
> ===============================
>  Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
>  155 Beaver Street, Milford, MA 01757 USA
>  d3e3e3@gmail.com
>