[Lsr] John Scudder's No Objection on draft-ietf-lsr-isis-srv6-extensions-14: (with COMMENT)

[John Scudder via Datatracker <[e-mail]>]

John Scudder has entered the following ballot position for
draft-ietf-lsr-isis-srv6-extensions-14: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-srv6-extensions/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Below are some questions and comments. I'd appreciate a reply, thanks.

1. Abstract

   The Segment Routing (SR) allows for a flexible definition of end-to-

You either have too many, or not enough words here. Either “segment routing“
(remove “the“), or something like “the segment routing architecture“.

   called "segments".  Segment routing architecture can be implemented

And here you do need “the“ in front of “segment”.

   over an MPLS data plane as well as an IPv6 data plane.  This document

“An” -> “the” (two places)

   over an IPv6 data plane.

“An” -> “the”

   This documents updates RFC 7370 by modifying an existing registry.

“Documents” -> “document”

Also, this doesn’t seem to me like an update to RFC 7370. It’s normal for an
RFC to update an IANA registry, without saying it updates a previous RFC that
established that registry. I think the “updates” just confuses matters and
clutters things up, and should be removed.

2. Section 1

   This documents updates [RFC7370] by modifying an existing registry
   (Section 11.1.2).

“Documents” -> “document”

See also comment #1 regarding update.

3. Section 4.3

   A non-zero SRH Max H.encaps MSD indicates that the headend can insert
   an SRH up to the advertised value.

“Up to the advertised value” doesn’t make sense. I guess you mean “can insert
an SRH with up to the advertised number of SIDs”?

4. Section 4.4

   The Maximum End D MSD Type specifies the maximum number of SIDs
   present in an SRH when performing decapsulation.

That makes sense.

              These includes, but
   not limited to, End.DX6, End.DT4, End.DT46, End with USD, End.X with
   USD as defined in [RFC8986].

That doesn’t. How can a number include all those specific things? A number’s
just an integer, right? Maybe you are providing some helpful context about the
types of SIDs that are permitted to be present? If so, I expect this is
specified elsewhere already, and this sentence is not helping; I would suggest
removing it. If it does need to stay, it needs a rewrite for grammar and
clarity. (Maybe you want something like “As specified in [RFC 8986] the
permitted SID types include, but are not limited to, <list>.”)

      If the advertised value is zero or no value is advertised
      then the router cannot apply any behavior that results in
      decapsulation and forwarding of the inner packet if the
      other IPv6 header contains an SRH.

What “other” IP header? Do you mean the outer header? The inner header?
Something else?

5. Section 5

   In cases where a locator advertisement is received in both a Prefix
   Reachability TLV and an SRv6 Locator TLV - (e.g. prefix, prefix-
   length, MTID all being equal and Algorithm being 0 in Locator TLV),

Since “e.g.” means “for example” you’re saying the thing in the parentheses is
only one example of a locator advertisement received in both? What’s another
example? (Maybe the algo being 1 in both cases?)

But in any case the text above appears redundant with the text that immediately
follows. Can’t the text above be deleted?

   In case where the same prefix, with the same prefix-length, MTID, and
   algorithm is received in both a Prefix Reachability TLV and an SRv6
   Locator TLV, the Prefix Reachability advertisement MUST be preferred
   when installing entries in the forwarding plane.  This is to prevent
   inconsistent forwarding entries between SRv6 capable and SRv6
   incapable routers.

“In case” -> “in the case” or “in cases”

6. Section 7.2

   Supported behavior values together with parent TLVs in which they
   area advertised are specified in Section 10 of this document.  Please

“Area” -> “are”

      Length: variable.

      Flags: 1 octet.  No flags are currently defined.

When you write “length: variable“, I think you mean “length: a 1 octet field,
whose value is variable“, don’t you? Just like you write “flags: 1 octet“? I
get that the value placed into the length field is variable, but the way you’ve
written it, it says that the length of the length field is itself variable,
which would make no sense.

This is not the only place in the document you specify a length field this way,
but I guess you should fix all of them. I’m only noting it here.

   The SRv6 End SID MUST be a subnet of the associated Locator.  SRv6
   End SIDs which are not a subnet of the associated locator MUST be
   ignored.

Is a host route (which is what a SID is) technically a “subnet”? Can something
which is not a net, be a subnet? It reads funny that way. If you change it,
possibly “MUST be covered by the associated Locator“? (Although then for
clarity you might need to define “covered“, which might not be any better.)
(Same comment applies to Section 8 which also mentions “subnet”.)

7. Section 8.1

         B-Flag: Backup flag.  If set, the SID is eligible for
         protection (e.g., using IPFRR) as described in [RFC8355].

Please expand IPFRR on first use. And since you say “e.g.” meaning “for
example”, do you contemplate some other kind of protection which is not IPFRR?
If not, I think you could delete the parenthesis without loss of clarity.

8. Section 9

   SRv6 SID Structure Sub-Sub-TLV is used to advertise the as defined in
   [RFC8986].  It has the following format:

You’re missing something between “advertise the” and “as defined”.

      Length: 4 octets.

Similar to my earlier comment, I think what you mean is something like “Length:
a 1 octet field, whose value is 4”.

   associated with it.  It's usage is outside of the scope of this
   document.

“It’s” -> “its”

9. Section 11

   and sub-sub-TLVs as well updating the ISIS TLV registry and defining

“As well as”

   a new registry.

Doesn’t it define several new registries?

10. Section 11.2

Shouldn’t there be a new subsection for the registry creation?