Subject: [Gen-art] Gen-ART Last Call review of draft-ietf-isis-prefix-attributes-03

From: Paul Kyzivat <[email-masked]>
Date: Mon, 28 Dec 2015 14:18:05 -0500

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

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

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

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

(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;

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

(5) Nit:

   And finally a typo in this section: "registery" should be "registry".

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Wed, 30 Dec 2015 17:46:53 +0000

Paul -

Thanx for the review.
Responses inline.

> -----Original Message-----
> From: Paul Kyzivat [mailto:[email-masked]]
> Sent: Monday, December 28, 2015 11:18 AM
> To: [email-masked]
> 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
> This draft is on the right track but has open issues, described in the review.
> 
> Major: 0
> Minor: 4
> Nits:  1
> 
> (1) Minor:
> 

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

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

>     TLVs 135, 235, 236, 237" table within the "IS-IS TLV Codepoints"

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

>    seems to have figured this out, since they already have placeholders

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

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

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

>    And finally a typo in this section: "registery" should be "registry".

[Les:] ACK

   Les

---

From: Paul Kyzivat <[email-masked]>
Date: Wed, 30 Dec 2015 15:31:03 -0500

Hi Les,

Note that at the end of the day my comments are just suggestions. You 
can act on them or not. They only become binding if the IESG decides to 
raise them.

More inline.

On 12/30/15 12:46 PM, Les Ginsberg (ginsberg) wrote:
> 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.

Understood. But the abstract will be seen by many (like me) who don't 
fall into that category. They are left entirely in the dark about what 
this is about. Might it be something they *ought* to be interested in? 
After reading the abstract, the only clue I had about the scope of this 
document was the name of the wg from the draft name. And once this 
becomes an RFC that won't be available as a hint. I had to look up isis 
in the list of WGs to discover that this was in the routing area. Then I 
had to do more searching to figure out what IS-IS was about.

So I'm not suggesting that the document to stand on its own. But just 
enough in the abstract to help the reader decide if he should look 
further would help. And just enough more in the intro to identify other 
documents that provide the context for this one would help the reader 
get the proper context.

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

Yes, this was mostly a tongue-in-cheek comment.

> 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 ad…

I was only looking for another word or two.

> 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 h…

Not a definition, just a disambiguation. Simply replacing "prefix" with 
"network prefix" would have met my need.

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

I guess I wasn't clear. I was suggesting that reference(s) be added to 
the introduction. (References are not permitted in the abstract, but 
they are allowed in the intro.) A reference to the base specification 
for the internet version of IS-IS would have helped me.

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

Yes, it was pretty clear that direct use of the numbers is a well 
established convention. And I don't suggest that you try to change it in 
this draft. Still, to an outside observer it seems like a very user 
unfriendly way to do things.

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

I suppose there is no need to do anything since IANA has figured it out.

> [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…

It is your call. I just imagine those who care about this will most 
often refer to the short names, so having them in the registry makes 
sense to me.

>>     And finally a typo in this section: "registery" should be "registry".

	Best wishes,
	Paul