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