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:
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.
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 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.
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 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.
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 in the new advertisements could be useful. I
therefore find that references to it would be inappropriate.
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.
(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. :-)
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.
(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.
I suppose there is no need to do anything since IANA has figured it out.
(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.
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.
(5) Nit:
And finally a typo in this section: "registery" should be "registry".
[Les:] ACK
Les
Best wishes,
Paul
_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art
