Re: [Isis-wg] IETF Last Call Gen-Art review of draft-ietf-isis-rfc4971bis-01

["Les Ginsberg (ginsberg)" <[e-mail]>]

Dale -

Thanx for your detailed review. I have elected to copy the WG on my reply as you also sent a copy of your review to the WG.

Overall, while I either agree with or have no objection to many of your comments, most of them could be applied to RFC 4971 itself. When generating the bis version we elected NOT to modify the original RFC except where it was necessary to address the issue of an IPv6-only router. It certainly can be argued that your suggestions improve the consistency and readability of the document, but they also make it deviate from RFC 4971 where no intent to define a functional change is intended. It therefore has to be considered whether making many of the changes you suggest might unintentionally suggest a substantive change where none is intended.

Be interested in your thoughts on this.

After we reach agreement on the above point I will spin a new version addressing your comments.

Specific responses inline.

> -----Original Message-----
> From: Dale R. Worley [mailto:[e-mail]]
> Sent: Thursday, August 04, 2016 1:00 PM
> To: [e-mail]; [e-mail]; [e-mail]
> Subject: IETF Last Call Gen-Art review of draft-ietf-isis-rfc4971bis-01
> 
> 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-rfc4971bis-01
> Reviewer: Dale R. Worley
> Review Date: 4 August 2016
> IETF LC End Date: 15 August 2016
> IESG Telechat date: 18 August 2016
> 
> Summary:
> 
> This draft is basically ready for publication, but has nits that should be fixed
> before publication.
> 
> * Editorial items
> 
> How is "capability TLV" to be capitalized?  I count the following occurrences of
> the different capitalizations:
> 
>     24 CAPABILITY(IES) TLV(s)
>      5 Capability(ies) TLV(s)
>      4 capability(ies) TLV(s)
> 
> The practice in other RFCs seems to be "Capability TLV".
> 
> This also affects two occurrences of "TLV named CAPABILITY".

[Les:] As "CAPABILITY" was the dominant form in RFC 4971 I would be inclined to use that. Also this is the form used in http://www.iana.org/assignments/isis-tlv-codepoints/isis-tlv-codepoints.xhtml

   " IS-IS Router CAPABILITY TLV"

But I agree consistency is desirable no matter what form we choose.

> 
> Abstract
> 
>    This document defines a new optional Intermediate System to
>    Intermediate System (IS-IS) TLV named CAPABILITY, formed of multiple
>    sub-TLVs, which allows a router to announce its capabilities within
>    an IS-IS level or the entire routing domain.
> 
> I think s/formed of/containing/ -- the TLV also contains the Router ID and
> Flags fields.
> 

[Les:] This text is unchanged from RFC 4971.

> 2.  IS-IS Router CAPABILITY TLV
> 
>    The IS-IS Router CAPABILITY TLV is composed of 1 octet for the type,
>    1 octet that specifies the number of bytes in the value field, and a
>    variable length value field that starts with 4 octets of Router ID,
>    indicating the source of the TLV, and followed by 1 octet of flags.
> 
> Should some note be put here that if Capability is used as an extended TLV,
> the type and length are increased to 2 bytes?  That is a general fact about
> TLVs, of course, but strictly, the above sentence isn't always correct.  (Or is
> the "extended" version available only for TLVs at some higher level of the
> hierarchy?)
>

[Les:] You refer here to the extended TLVs defined in RFC 7356  (pretty good find for someone who is not supposed to be an IS-IS expert :-) ).
I am reluctant to require IS-IS RFCs in general to have to define both formats. That seems unnecessary. I am even more reluctant to do so in a bis version of an existing RFC.

 
> I think s/and followed by/followed by/.  The subject of "followed" is
> "4 octets of Router ID", leaving "and" to join "indicating the source of the
> TLV" with "followed by 1 octet of flags", which is not a parallel construction.
> Instead, omit "and" to make the construction "... value field that starts with 4
> octets ... followed by ..."
> 

[Les:] No objection - though again this is the original text from RFC 4971 which the RFC editor seemed happy with at the time.

> 3.  Elements of Procedure
> 
>    Router ID sub-TLV [RFC5316] MUST be present in the TLV.  Router
>    CAPABILITY TLVs which have a Router ID of 0.0.0.0 and do NOT have the
>    IPv6 TE Router ID sub-TLV present MUST be ignored.
> 
> Given that "ignored" is used later to describe processing of unknown sub-
> TLVs, which must not be interpreted but which may need to be leaked,
> perhaps "ignored" here should be amplified.  Perhaps "ignored, including not
> leaked".

[Les:] Later in the same section where the draft discusses not using the TLV the draft says 

" Note that leaking a Capabilities
   TLV is one of the uses that is prohibited under these conditions."

We could repeat similar text above, but I would rather revise the text to indicate whatever the reason for not using the TLV (unusable router-id or stale TLV) that leaking is prohibited so this is stated in one place.
I will come up w revised text.

> 
>       Example: If Level-1 router A generates a Capability TLV and floods
>       it to two L1/L2 routers, S and T, they will flood it into the
>       Level-2 domain.  Now suppose the Level-1 area partitions, such
>       that A and S are in one partition and T is in another.  IP routing
>       will still continue to work, but if A now issues a revised version
>       of the CAP TLV, or decides to stop advertising it, S will follow
>       suit, but T will continue to advertise the old version until the
>       LSP times out.
> 
> I think you need to expand the last sentence to "... but without the above
> prohibition, T would continue ...".  Or otherwise qualify the entire example
> with "without the above prohibition".
> 

[Les:] Agree that something like your suggestion would improve the text, but again this is original RFC 4971 text.

>    Routers in other areas have to choose whether to trust T's copy of
>    A's capabilities or S's copy of A's information and, they have no
>    reliable way to choose.  By making sure that T stops leaking A's
>    information, this removes the possibility that other routers will use
>    stale information from A.
> 
> This second paragraph is part of the example, and should be indented the
> same way as the first paragraph.
>

[Les:] Agree.

 
> Similarly, in the first sentence, s/have to/would have to/.
> 
> Remove the comma after "and".
> 
> The "this" in the last sentence doesn't have an antecedent.  Change to "this
> prohibition".
> 

[Les:] Again, editorial changes to original RFC 4971 text to which I have no objection.

>    How partial support may impact the operation of the
>    capabilities advertised within the Router CAPABILITY TLV is outside
>    the scope of this document.
> 
> Is it worth specifying that the documents that define the sub-TLVs are
> responsible for considering the impact of partial support?
> 

[Les:] I am fine with the current text. Adding what you suggest might make some existing RFCs which define sub-TLVs non-compliant. I am reluctant to do that. If the review of those RFCs did not feel the need to comment on the lack of discussion of this point I would rather not introduce a compliance issue after the fact. It is not that your suggestion is poor - just that since it was not stated in RFC 4971 it seems problematical to introduce it in the bis version.

>    If leaking of the CAPABILITY TLV is required, the entire CAPABILITY
>    TLV MUST be leaked into another level even though it may contain some
>    of the unsupported sub-TLVs.
> 
> I think the final phrase would be better as "... even though it may contain
> sub-TLVs that are not supported by the router leaking the TLV."
> 

[Les:] No objection.

> 6.  IANA Considerations
> 
>    IANA assigned a new IS-IS TLV code-point [...]
> 
> The usual form is "codepoint".  But perhaps that question is better left to the
> RFC Editor.
> 

[Les:] This text was added by IANA originally -not by the authors of RFC 4971 - so I will leave it to IANA/RFC editor to deal with this.

> 7.  Acknowledgements
> 
>    For the original version of RFC 4971 the authors thanked Jean-Louis
>    Le Roux, Paul Mabey, Andrew Partan, and Adrian Farrel for their
>    useful comments.
> 
> "the original version of RFC 4971" is incorrect.  There are a number of ways to
> fix this, but I suggest "the original version of this document, RFC 4971, ...".
>

[Les:] Agree
 
>    For this new version the authors would like to thank Kris Michielsen
>    for calling the problem associated w an IPv6 only router to our
>    attention.
> 
> Change "w an IPv6 only" to "with an IPv6-only".
> 
> It would be easier to read as "... for calling to our attention the problem
> associated ...".
> 

[Les:] Agree

    Les

> Dale