Last Call Review of draft-ietf-isis-node-admin-tag-08
review-ietf-isis-node-admin-tag-08-genart-lc-yee-2016-04-30-00
Request | Review of | draft-ietf-isis-node-admin-tag |
---|---|---|
Requested revision | No specific revision (document currently at 11) | |
Type | Last Call Review | |
Team | General Area Review Team (Gen-ART) (genart) | |
Deadline | 2016-04-29 | |
Requested | 2016-04-18 | |
Authors | Pushpasis Sarkar , Hannes Gredler , Shraddha Hegde , Stephane Litkowski , Bruno Decraene | |
I-D last updated | 2016-04-30 | |
Completed reviews |
Genart Last Call review of -08
by Peter E. Yee
(diff)
Genart Last Call review of -08 by Peter E. Yee (diff) Secdir Last Call review of -08 by Catherine Meadows (diff) Opsdir Last Call review of -08 by Jürgen Schönwälder (diff) Rtgdir Early review of -08 by Andrew G. Malis (diff) |
|
Assignment | Reviewer | Peter E. Yee |
State | Completed | |
Request | Last Call review on draft-ietf-isis-node-admin-tag by General Area Review Team (Gen-ART) Assigned | |
Reviewed revision | 08 (document currently at 11) | |
Result | Ready w/issues | |
Completed | 2016-04-30 |
review-ietf-isis-node-admin-tag-08-genart-lc-yee-2016-04-30-00
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 comment. For background on Gen-ART, please see the FAQ at < http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq> Document: draft-ietf-isis-node-admin-tag-08 Reviewer: Peter Yee Review Date: April 27, 2016 IETF LC End Date: April 29, 2016 IESG Telechat date: May 5, 2016 Summary: This draft is basically ready for publication as a Standards Track RFC, but has some issues that should be fixed/considered before publication. [Ready with issues] This draft defines a means to carry additional per-node administrative tags with the IS-IS protocol. These tags can be used along with local policy to simplify the management of routing and path selection. This specification gives informative examples of such tag usage but does not otherwise prescribe the meaning of the tags. This review was generated prior to the release of draft -09 (but not keyed in until April 29th), but many of the issues and nits noted below remain in draft -09. Obviously, some of my comments no longer apply. I'll address draft -09 specifically for the telechat review, but you should look at the points here prior to that review to save time. Given that draft -09 substantially reduces Section 5, I've removed my comments regarding that section as well as in a few other places. Major issues: None Minor issues: Page 4, last partial paragraph: the number 63 is given for the maximum number of per-node administrative tags that can be carried in a sub-TLV. Given the maximum length of a sub-TLV is 250 octets (and 2 octets are otherwise used by type and length), I would argue that the correct number here is 62 (62*4 = 248). Also, I would delete the text starting at "and". In all cases, when more than 62 tags are used, a single sub-TLV will not provide sufficient space. Page 5, 1st partial paragraph, 1st full sentence: Sub-TLV values are given here as cumulative. Is there any need or desire to be able to subtract tags? How would a router disassociate itself from a tag that was no longer relevant to the router? This ability is implied in Section 4.3, 2nd paragraph, but that conflicts with the statement given here. In general, I believe the ability to reset the flooded tags associated with a router or to delete a tag is underspecified. Page 6, 1st partial paragraph, 1st sentence: Care to define "reasonably small"? Previously, the ability to send more than 63 (or perhaps 62, see above) tags was specified in section 3.1. What's the limit to reasonableness? (Not that an exact number seems to matter at all for the purposes of this specification, which is agnostic to the specific number or the use of the tags.) Page 6, Section 4.3, 2nd paragraph: This paragraph implies that a large set (greater than 62 at least) of sub-TLVs will have to be sent in multiple Router CAPABILITY TLVs and those TLVs must(?) occur in a single Link-State PDU. Or how is the receiving router to know that it has the complete set of tags? Also, the implication seems to be that while section 3.1 indicates a strictly cumulative capability, there must be someway of terminating those cumulative changes and allowing a reset. What is that signaling mechanism? Nits: General: The use of capitalization of Per-node administrative tag varies throughout the document. It seems clear that you mean for it to be written as "Per-node Administrative Tag" when referring to the name of sub-TLV. For other uses, I would suggest using "per-node administrative tag" consistently. Use that to replace "Per-node administrative tag". Separate parenthetical elements from the preceding and following words with a space. These aren't function calls. ;-) Replace any use of "per-node admin tag" with "per-node administrative tag". The shortening is fine for the document header which would otherwise become unreadable. And change all of my references to "per-node" to "node", since draft -09 drops the "per-". :-) Replace "TLV 242" with "the Router CAPABILITY TLV". Specific: Page 1, Abstract: delete the first comma in the Abstract. Page 3, first paragraph after the lettered items, 3rd sentence: insert "the" before "Router". Page 3, Section 2, 1st sentence: change "Tag" to "tag". Page 3, Section 2, 3rd sentence: insert "a" before "certain". Page 3, Section 3, 1st paragraph, 1st sentence: change "TLV-242" to "TLV (IS-IS TLV type 242)" and delete the closing parenthesis after "[RFC4971". Page 3, Section 3, 1st paragraph, 3rd sentence: change "the same" to "it". Change "they" to "it". Change "specfied" to "specified". Insert "the" before "originating". Delete "the" before "other". Page 3, Section 3, 1st paragraph, 5th sentence: change "are" to "is". Page 3, Section 3, 1st paragraph, 6th sentence: delete the comma. Page 3, Section 3, 1st paragraph, 7th sentence: change "Operator" to "The operator". Page 4, Section 3, last paragraph, 1st sentence: insert "the" before "Per-node" (after having changed "per-node" to "Per-node"). Page 4, Section 3, last paragraph, 2nd sentence: change "topology specific" to "topology-specific". Page 4, Section 3.1, 1st paragraph, 1st sentence: change "ISIS" to "IS-IS". Page 4, Section 3.1, Length definition: change "A" to "An". Page 4, Section 3.1, Value definition: change "sequence" to "set". Sequence would seem to imply order which is contradicted by Section 4.1. Change "4 octets" to "4-octet values". Page 5, 1st partial paragraph, 1st full sentence: append a comma after "such" and insert "each" before "occurrence". Page 5, Section 4.1, 1st paragraph, 1st sentence: change "Meaning" to "The meaning". Page 5, Section 4.1, 1st paragraph, 2nd sentence: change "Router" to "A router". Page 5, Section 4.1, 2nd paragraph, last sentence: append a comma after "change". Page 5, Section 4.1, 4th paragraph, 2nd sentence: delete "The". Change "TLVs" to "sub-TLVs". Change "adminsitrative" to "administrative". Page 5, Section 4.1, 4th paragraph: the paragraph, starting at "The list of per-node" is pretty much redundant given the admonition in the first sentence of the previous paragraph. Perhaps delete it rather than belabor the point? Page 5, Section 4.1, 1st paragraph, 4th sentence: change "well known" to "well-known". Change "capability" to "CAPABILITY". Page 6, 1st partial paragraph, 2nd sentence: insert "the" before "reachability". Page 6, Section 4.3, 1st paragraph, 1st sentence: delete "(TLV-242)". Page 6, Section 4.3, 1st paragraph, 3rd sentence: change "an" to "a". Based on Section 3.1, I might change "changes" to "adds to" because Section 3.1 specifies that sub-TLVs are cumulative. Page 10, Section 7, 1st paragraph, 3rd sentence: change "ISIS" to "IS-IS". Change "is" to "are". Page 10, Section 7, 2nd paragraph, 2nd sentence: insert "the" before "case". Insert "the" before "forwarding". Insert a space before "and". Page 12, Section 8, 2nd sentence: insert "The" before "YANG". Page 12, Section 8, 3rd sentence: insert "The" before "IS-IS". Insert "the" before "routing". Page 12, Section 9, item i): why is the sub-TLV name hyphenated here and not elsewhere? Page 12, Section 10: append a comma after "Chunduri".