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

From: [email-masked] (Dale R. Worley)
Date: Thu, 04 Aug 2016 16:03:35 -0400

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".

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.

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?)

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 ..."

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".

      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".

   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.

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".

   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?

   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."

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.

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, ...".

   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 ...".

Dale