ietf
[Top] [All Lists]

RE: IETF Last Call Gen-Art review of draft-ietf-isis-rfc4971bis-01

2016-08-08 08:34:32
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:worley(_at_)ariadne(_dot_)com]
Sent: Thursday, August 04, 2016 1:00 PM
To: gen-art(_at_)ietf(_dot_)org; ietf(_at_)ietf(_dot_)org; 
draft-ietf-isis-rfc4971bis(_dot_)all(_at_)ietf(_dot_)org
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