Re: Gen-ART LC review of draft-ietf-isis-node-admin-tag-08

[Pushpasis Sarkar <[e-mail]>]

Hi Peter,

Thanks a lot for the comments. I will address them soon in the next version.

Best Regards,
-Pushpasis

On Saturday 30 April 2016 02:11 PM, Peter Yee wrote:
> 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".
>