Skip to main content

Last Call Review of draft-ietf-isis-prefix-attributes-03

Request Review of draft-ietf-isis-prefix-attributes
Requested revision No specific revision (document currently at 04)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2016-01-01
Requested 2015-12-03
Authors Les Ginsberg , Bruno Decraene , Stefano Previdi , Xiaohu Xu , Uma Chunduri
I-D last updated 2015-12-28
Completed reviews Genart Last Call review of -03 by Paul Kyzivat (diff)
Genart Last Call review of -04 by Paul Kyzivat
Secdir Telechat review of -02 by Yaron Sheffer (diff)
Opsdir Telechat review of -01 by Carlos Pignataro (diff)
Assignment Reviewer Paul Kyzivat
State Completed
Request Last Call review on draft-ietf-isis-prefix-attributes by General Area Review Team (Gen-ART) Assigned
Reviewed revision 03 (document currently at 04)
Result Ready w/issues
Completed 2015-12-28
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

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


This draft is on the right track but has open issues, described in the

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"

   (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

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