Paul -

Thanx for the review.
Responses inline.

> -----Original Message-----
> From: Paul Kyzivat [mailto:pkyzi...@alum.mit.edu]
> Sent: Monday, December 28, 2015 11:18 AM
> To: draft-ietf-isis-prefix-attributes....@ietf.org
> Cc: General Area Review Team
> Subject: Gen-ART Last Call review of draft-ietf-isis-prefix-attributes-03
> 
> 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-isis-prefix-attributes-03
> Reviewer: Paul Kyzivat
> Review Date: 2015-12-28
> IETF LC End Date: 2016-01-01
> IESG Telechat date: 2016-01-07
> 
> Summary:
> 
> This draft is on the right track but has open issues, described in the review.
> 
> Major: 0
> Minor: 4
> Nits:  1
> 
> (1) Minor:
> 
>    The abstract and introduction both seem to assume that the
>    reader has a lot of context about the intended scope of this
>    document. For instance:
> 
>    - the abstract starts with "This document introduces new sub-TLVs",
>      without any mention of to what they are subordinate;
> 
>    - the intro starts with "There are existing use cases in which
>      knowing additional attributes of a prefix is useful." The
>      uninitiated reader is left to figure out what sort of prefix
>      (in this case network prefix) is being discussed.
> 
>    It would be helpful to state more of the context. Adding one or more
>    references to the Introduction would also help. Keep in mind that
>    once this is published as an RFC many people may see only the RFC
>    number and the abstract, without even knowing that this is about
>    routing. (And when looking at the title a different meaning of "ISIS"
>    might first come to mind.)
> 
>    (Once I had the proper mind set, and had reviewed some of the related
>    drafts and the relevant IANA registries, the draft finally made sense
>    to me.)

[Les:] Frankly, I have difficulties with your comments. I appreciate that they 
are coming from the POV of someone not familiar w IS-IS. 

There are a large number of RFCs which define extensions to the IS-IS protocol. 
None of these documents stand on their own. If the reader does not know what 
"IS-IS" is then I think it is safe to say the document is not something that 
would be useful to that reader. In regards to "other meanings" for the term 
"ISIS", I would only comment that the protocol and the name originate from work 
done in the 1980's. If in recent years other uses/contexts for the name have 
become part of modern discussion this does not require this draft (or any other 
IS-IS WG document) to address this. (This is as "politically correct" a 
statement as I can make. :-) )

In regards to the abstract, brevity is a stated requirement for an abstract 
(see http://www.ietf.org/id-info/guidelines.html#anchor11). Yes, if you want to 
find out what prefix attributes can be advertised and in what TLVs these 
advertisements may appear you have to read the body of the document. I do not 
apologize for that. IMO the abstract does what it is intended to do - 
succinctly summarize the topics which are covered in the document and I think 
adding additional detail (such as the TLVs which have been affected) would not 
serve the purpose of the abstract.

In regards to the term "prefix", you seem to be expecting the document to 
define that term - but in looking at multiple RFCs I do not see precedent for 
that. It is part of the base knowledge that has been assumed that readers 
understand . Perhaps this is a bad practice - but if so there are many 
documents - not restricted solely to IS-IS related documents - that could be 
critiqued on this basis. I would ask that this comment be viewed in a larger 
context - I don't think this particular draft should be asked to deviate from 
common practice without larger guidance from the IETF community.

In regards to "references to the Introduction", I emphasize that the 
Introduction is neither normative nor exhaustive. It is meant to provide some 
examples of cases where the information contained in the new advertisements 
could be useful. I therefore find that references to it would be inappropriate.

> 
> (2) Minor / meta-editorial:
> 
>    I found it disconcerting that TLVs are referenced by their numeric
>    type value rather than a name. And in this case the new sub-TLVs ar
>    defined in a table that applies jointly to several different TLVs. I
>    think it would be clearer if a name was given to this collection of
>    TLVs, and used to discuss things that apply to all of them. (But,
>    while I bring this up, I don't really expect that it makes sense to
>    address it in the context of this draft. It it perhaps something
>    better saved for a BIS to the entire IS-IS family.)
>
[Les:] The registry being modified is officially named " Sub-TLVs for TLVs 135, 
235, 236, and 237" 
(http://www.iana.org/assignments/isis-tlv-codepoints/isis-tlv-codepoints.xhtml#isis-tlv-codepoints-135-235-236-237
 )- so using TLV numbers is both a common practice and definitive. In the case  
of TLVs used for prefix advertisement because there are multiple TLVs involved 
(including "old style" TLVs 128 and 130 which are NOT modified by the draft), 
the use of numbers is also considerably more succinct. An accurate replacement 
would require the following text:

" Extended IP reachability and MT IP. Reachability and IPv6 IP Reachability and 
MT IPv6 IP Reachability"

Perhaps you can appreciate why we prefer to use numbers. :-)

> (3) Minor:
> 
>    The IANA considerations section says:
> 
>     This document adds the following new sub-TLVs to the registry of sub-
>     TLVs for TLVs 135, 235, 236, 237.
> 
>    It doesn't explicitly indicate which registry this is. I suggest:
> 
>     This document adds the following new sub-TLVs to the "Sub-TLVs for
>     TLVs 135, 235, 236, 237" table within the "IS-IS TLV Codepoints"
>     registry.

[Les:] As stated above, the document uses the official name of the existing 
registry. However, as IANA rewrites these sections anyway I am fine with 
whatever wording they choose to use.

> 
>    (Or some other wording recommended by IANA.) To their credit, IANA
>    seems to have figured this out, since they already have placeholders
>    in the table.

[Les:] The "placeholders" exist because of early allocation requests made as 
defined in RFC 7370.

> 
> (4) Minor:
> 
>    Also in IANA considerations, in the definition of the new bit flags
>    table,
> 
>    - the document ought to explicitly state the name it would like
>      assigned to the new registry;

[Les:] Guilty as charged. IANA has asked the same question in their review - I 
will provide the suggested name in my response to them.


> 
>    - the name given in the IANA considerations section only includes the
>      long name from section 2.1 (e.g., External Prefix Flag), not the
>      short mnemonic name (e.g., X-Flag). Someone reading the IANA table
>      might want to see the short name.
> 
[Les:] The short names exist to aid in the pictorial representation of the 
field. In existing bit registries (e.g. 
http://www.iana.org/assignments/isis-tlv-codepoints/isis-tlv-codepoints.xhtml#isis-tlv-codepoints-19of22
 ) only a long name is used. But I have no objection to including the short 
names.

> (5) Nit:
> 
>    And finally a typo in this section: "registery" should be "registry".

[Les:] ACK

   Les

_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to