Hi Paul, Version -09 has been uploaded with the intent that it resolve your comments.
Thanks, Donald =============================== Donald E. Eastlake 3rd +1-508-333-2270 (cell) 155 Beaver Street, Milford, MA 01757 USA d3e...@gmail.com On Tue, Jul 5, 2016 at 9:32 AM, Paul Kyzivat <pkyzi...@alum.mit.edu> 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 +1-508-333-2270 (cell) >> 155 Beaver Street, Milford, MA 01757 USA >> d3e...@gmail.com >> >> >> On Mon, Jul 4, 2016 at 6:50 PM, Paul Kyzivat <pkyzi...@alum.mit.edu> >> 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 <pkyzi...@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 >>>> d3e...@gmail.com >>>> >>> >> > _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
