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

[Paul Kyzivat <[e-mail]>]

On 7/6/16 5:18 PM, Donald Eastlake wrote:
> Hi Paul,
>
> On Wed, Jul 6, 2016 at 1:34 PM, Paul Kyzivat <[e-mail]> wrote:
>> Donald,
>>
>> On 7/5/16 9:51 PM, Donald Eastlake wrote:
>>>
>>> Hi Paul,
>>>
>>> Version -09 has been uploaded with the intent that it resolve your
>>> comments.
>>
>>
>> Looks good except for one thing:
>>
>> Some of the new text in section 7 has me baffled:
>>
>>    The following processes should be followed in interpreting sets of
>>    AFN values in an IA APPsub-TLV to synthesize addresses. These apply
>>    whether the AFN values came from sub-sub-TLVs or appeared within an
>>    Address Set or came from both sources. In general, the processing is
>>    applied separately to each Address Set as supplemented by any Fixed
>>    Address sub-sub-TLVs that are present.
>> ...
>>    Typically, an OUI would be provided as a Fixed Address sub-sub-TLV
>>    (see Section 3.2) using the OUI AFN but there is no prohibition
>>    against one or more OUIs appearing in an Address Set.
>>
>>    Each Address Set, after being supplemented by any Fixed Address sub-
>>    sub-TLVs, is processed by combining each OUI in the Address Set with
>>    each MAC/24 and each MAC/40 address in the address set.
>>
>> I get it when an OUI comes as a Fixed Address. That value is combined with
>> each MAC/24 and MAC/40 in the address set, creating a new column in the
>> address set. But I have no idea what it means if an OUI appears in the
>> address set. In that case there will be a whole column of them, with a
>> distinct value in each row. Does each one combine with MAC/*s in *every*
>> row, or only with the MAC/*s in the *same* row? That latter seems like the
>> more sensible interpretation, but the intent isn't clear. Frankly this kind
>> of usage seems more like nonsense.
>
> I don't see any reason to make the processing more complex with
> special cases just because some uses seem very unlikely.

I certainly agree. I expect I'm not understanding the text way you 
intend it to be understood.

>      You take the addresses from an Address Set and then you
> supplement them from any Fixed Address sub-sub-TLVs that might be
> present in the same IA APPsub-TLV. After that step, you have a set of
> addresses and it no longer makes any difference where they came from.
> You then perform certain processing on OUI, MAC/24, and MAC/40
> addresses that are present, if any, to produce some number (possibly
> zero) of 48 and/or 64 bit MAC addresses. You then perform a second
> stage of processing using any IPv6/64 addresses, 48-bit MAC addresses,
> and/or 64-bit addresses that are present, if any, to produce some
> number (possibly zero) of IPv6 addresses.

I think my confusion is that I have been thinking of these things as two 
dimensional tables, so that the processing adds columns, while you are 
thinking of them as providing a list of sets, so that the rules are 
describing how to transform each set.

Additionally, because I was thinking of them as tables, I was thinking 
that if you put an OUI in the table (as a column) then the recipient 
ends up storing that, even though it isn't a useful thing to keep. I 
think I now get it that the recipient is probably simply processing each 
row and then storing away the parts that are interesting to it, so any 
uninteresting parts are discarded.

So feel free to leave things as-is if you believe it will be clear to 
those who care.

	Thanks,
	Paul

>       Why bother making the above description more complex by
> prohibiting OUIs in Address Sets? (or saying they are ignored in
> Address Sets or the the entire Address Set with an OUI is ignored or
> ..., which would probably be better to say since otherwise you would
> want to say what to do about the (artificial in my opinion) "error"
> condition of finding any OUI in an Address Set which implies it is in
> the template for the Address Sets in that IA-APPsub-TLV.)
>
>> But maybe it would make more sense to simply say that OUIs (and the other
>> partial addresses) only make sense in special contexts such as the Fixed
>> Address sub-sub-TLV.
>
> Any OUI in an Address Set is, I admit, pretty odd. But I would expect
> MAC/24 and the like to be in Address Sets. The idea is that you have,
> for example, lots of end station physical Ethernet ports on links
> attached to some RBridge, all from the same manufacturer with the same
> OUI. So you stick that OUI in a Fixed Address sub-sub-TLV and then you
> can save space by just putting a MAC/24 in the Address Set for each of
> those ports in the IA APPsub-TLV originated by the RBridge.
>
> The text already says that "typically" an OUI would be in a Fixed
> Address sub-sub-TLV...
>
> Thanks,
> Donald
> ===============================
>  Donald E. Eastlake 3rd   [phone] (cell)
>  155 Beaver Street, Milford, MA 01757 USA
>  [e-mail]
>
>>         Thanks,
>>         Paul
>>
>>
>>> Thanks,
>>> Donald
>>> ===============================
>>>  Donald E. Eastlake 3rd   [phone] (cell)
>>>  155 Beaver Street, Milford, MA 01757 USA
>>>  [e-mail]
>>>
>>>
>>> On Tue, Jul 5, 2016 at 9:32 AM, Paul Kyzivat <[e-mail]>
>>> wrote:
>>>>
>>>> On 7/4/16 11:35 PM, Donald Eastlake wrote:
>>>>>
>>>>>
>>>>> Hi Paul,
>>>>>
>>>>> I believe we are generally in agreement.
>>>>>
>>>>> On the table in the IANA Considerations, I have read
>>>>>
>>>>> https://tools.ietf.org/html/draft-leiba-cotton-iana-5226bis-15#section-1.1
>>>>> and I can understand why you commented as you did. But, since all the
>>>>> table entries were created by IANA actions, I still think the draft is
>>>>> OK in having that table in the IANA Considerations Section. This is
>>>>> not a case of including some technical specification in with the IANA
>>>>> Considerations.  It's still all code points.
>>>>
>>>>
>>>>
>>>> OK. It is not a big deal.
>>>>
>>>>         Thanks,
>>>>         Paul
>>>>
>>>>
>>>>> Thanks,
>>>>> Donald
>>>>> ===============================
>>>>>  Donald E. Eastlake 3rd   [phone] (cell)
>>>>>  155 Beaver Street, Milford, MA 01757 USA
>>>>>  [e-mail]
>>>>>
>>>>>
>>>>> On Mon, Jul 4, 2016 at 6:50 PM, Paul Kyzivat <[e-mail]>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> 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 <[e-mail]>
>>>>>>> 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   [phone] (cell)
>>>>>>>  155 Beaver Street, Milford, MA 01757 USA
>>>>>>>  [e-mail]
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>