Subject: [Lsr] Benjamin Kaduk's Discuss on draft-ietf-lsr-isis-srv6-extensions-14: (with DISCUSS and COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 18 May 2021 13:29:58 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-lsr-isis-srv6-extensions-14: Discuss

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/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

The prose and tabular IANA considerations in §11.3 are inconsistent
about whether the End.X/LAN End.X SID sub-TLVs are allowed to appear in
TLV 25.  (I may have noted all instances in the prose, in my COMMENT,
but it's worth checking for others.)

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

ABSTAIN

Once my discuss point is resolved, I intend to change my position to
Abstain.  The relationship of SRv6 SIDs to the IPv6 addressing
architecture (RFC 4291) was quite controversial during the processing of
what since became RFC 8986.  I was able to reconcile the two at the time
by virtue of noting that the substructure of the SID can be considered
to be logically local to the node advertising the SID and therefore not
an observable part of the protocol.  In that sense, the wire-visible use
and processing of SIDs remains that of normal IPv6 addresses.  However,
introducing a sub-sub-TLV to specifically carry the structure of the SID
causes that reasoning to no longer apply, and seemingly re-opens the
question of whether the SID substructure is consistent with the IPv6
addressing architecture.  In the absence of some compelling use case, I
cannot support publishing a mechanism that triggers this controversy.
Indeed, no motivating use case is presented in the document at all (the
usage is "outside of the scope of this document"), which invites
questions about why this mechanism should be defined in a
standards-track RFC at all (the relevant registry procedures are merely
"expert review").

COMMENT

(I agree with John that this doesn't inherently seem like an update to
RFC 7030, but I have nothing further to add that he hasn't said already,
so let's keep that topic in his ballot thread.)

Thank you for noting (repeatedly) that multiple TLVs may be needed in
order to advertise all the SIDs for a given neighbor/endpoint/etc. --
the one-byte length field for TLVs is a bit cramped when we have 16-byte
SIDs!

Section 4

   Link MSDs are advertised in a sub-TLV of TLVs 22, 23, 141, 222, and
   223.

The list in RFC 8491 includes TLV 25 as well.  Is TLV 25 somehow not
relevant for this document?

Section 4.3

Does the SRH Max H.encaps MSD type have any bearing on the application
of H.Encaps.Red?  (I assume the L2 encapsulations from RFC 8986 are not
quite so relevant.)

Section 5

   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

Others already covered the duplication/mangled nature of this paragraph,
but please also expand MTID on first use.

   Locators associated with Flexible Algorithms (see Section 4 of
   [I-D.ietf-lsr-flex-algo]) SHOULD NOT be advertised in Prefix
   Reachability TLVs (236 or 237).  Advertising the Flexible Algorithm
   locator in regular Prefix Reachability TLV (236 or 237) would make
   the forwarding for it to follow algo 0 path.

Perhaps this is more properly a matter for -flex-algo itself, but if
these locators are not to be advertised in TLVs 236/237 with the goal of
having forwarding for those prefixes not follow the algo(rithm) 0 path,
does that mean that the flexible algorithms can only be deployed in
networks where all routers support the flexible algorithm in question?
Bringing things closer to this document, would the presence of a mixed
deployment give grounds to violate the SHOULD NOT?

   are therefore advertised as sub-TLVs in TLVs 22, 23, 222, 223, and
   141.

[the same list that does not include TLV 25, again]

Section 6

   The A-flag and the N-flag MUST NOT both be set.  If both N-flag and
   A-flag are set in the prefix/SRv6 Locator advertisement, the
   receiving routers MUST ignore the N-flag.

This seems rather backwards-incompatible, since nodes that do not know
about this document will interpret only the N flag in this case.  Thus,
in a mixed network, if a prefix is advertised with both A- and N-flags
set, some nodes will treat it as identifying a node and other nodes will
treat it as an anycast address.  Since this is inherently an error
situation (it violates the quoted "MUST NOT"), what is the benefit from
having the 'A' bit take precedence?  It would seem equally valid, and
provide more uniform treatment across the network, to have the 'N' bit
take precedence in this case.  (Though, given that the N flag is ignored
for non-host-prefixes, I guess the A flag would take precedence in some
cases anyway, so maybe it is not quite so simple.)

Section 7.1

   The SRv6 Locator TLV has the following format:

       0                   1                   2                   3
       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |   Type        |     Length    |R|R|R|R|    MT ID              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

I suggest adding some placeholder to the figure for "further entries
follow"; the current formulation suggests that only these four bytes
comprise the TLV.

        Metric: 4 octets. As described in [RFC5305].

I think more specificity is needed in the reference, since RFC 5305
describes both a 3-octet metric field in sub-TLV 18 and a 4-octet metric
field in TLV 135.

        Algorithm: 1 octet. As defined in [RFC8665].

Earlier we used 8667 as the reference for the SR-Algorithm values, since
that's the IS-IS document (8665 is what created the IANA registry, but
is otherwise about OSPF).

        Optional sub-TLVs: Sub-TLVs 1, 2, 4, 5, 11, 12 are allowed.
        Any other Sub-TLVs MUST be ignored.

I don't really understand why we hardcode a list here, given that we
also update the corresponding IANA registry in this document.  Don't we
want the IANA registry to be controlling in the future?

Section 8.1

      Algorithm: 1 octet.  As defined in [RFC8665].

[same comment as above]

Section 12

I'd also reference the security considerations of
draft-ietf-6man-spring-srv6-oam and draft-ietf-lsr-flex-algo.

Are there any new security considerations relating to anycast
announcements?  E.g., if the nodes advertising the same prefix/locator
are configured differently and traffic sent to the one vs the other gets
different treatment?

Given that I failed to convince the authors of RFC 8986 to add any text
about the security considerations of SID sub-structure, I don't expect
to be successful here, either.  But I note that being explicit about the
breakdown into func/arg/etc. makes it easier to construct SIDs that are
not advertised but might be processed on receipt, by combining a
known-valid function code with an argument value that provides semantics
that the advertising endpoint implements but did not choose to advertise
in this case.

Section 14.1

If we switch to using RFC 8667 as the reference for SR Algorithms for
IS-IS, then RFC 8665 could be relegated to an informative reference.

NITS

Abstract

   called "segments".  Segment routing architecture can be implemented
   over an MPLS data plane as well as an IPv6 data plane.  This document

I think an article ("the" or "a" at the start of this sentence would
help.

Section 4.4

      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.

I think s/other/outer/

Section 6

   All the nodes advertising the same anycast locator MUST instantiate
   the exact same set of SIDs under such anycast locator.  Failure to do
   so may result in traffic being black-holed or mis-routed.

s/such/that/

Section 7.2

      Optional Sub-sub-TLVs.

A forward reference to the registry created in Section 11.5 might help.

Section 8

   This document defines two new sub-TLVs of TLV 22, 23, 222, 223, and
   141 - namely "SRv6 End.X SID sub-TLVs" and "SRv6 LAN End.X SID sub-
   TLVs".

I suggest sorting the list of TLV numbers.
[TLV 25, once again, does not appear in this list, as mentioned
previously.]

Section 8.1

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

RFC 8355 doesn't mention "IPFRR", so I think a reference for IPFRR is in
order.

Also, naively I would have expected a flag named "backup" to be set on
the thing that is the backup path, not on the primary path that is
eligible for protection.  So maybe a different mnemonic would be better?

         Reserved bits: MUST be zero when originated and ignored when
         received.

Using "MUST be ignored" would reduce the diff against §8.2.

      Sub-sub-TLV-length: 1 octet.  Number of octets used by sub-sub-
      TLVs.

As above, a forward-reference to the registry would be helpful.

Section 8.2

Putting the note that multiple TLVs might be needed for the same DIS
neighbor at the end of the section would reduce the diff against §8.1

      Sub-sub-TLV-length: 1 octet.  Number of octets used by sub-sub-
      TLVs.

As above, a forward-reference to the registry would be helpful.

Section 10

   registry defined in [RFC8986].  If this behavior is advertised it
   MUST only be advertised in the TLV[s] as indicated by "Y" in the

In a table with multiple entries, "this behavior" is not well defined;
"a behavior" would be more appropriate.

---

From: Peter Psenak <[email-masked]>
Date: Thu, 20 May 2021 13:54:14 +0200

Hi Benjamin,

thanks for your comments, please see inline (#PP):

> https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-srv6-extensions/

##PP
good catch, I added it to all places.

##PP
RFC8986 introduced the SRv6 SID structure. This document is only 
introducing the ISIS extension for its signalling. Please see more on 
this and the use cases in my response to Erik

> the one-byte length field for TLVs is a bit cramped when we have 16-byte

##PP
not much I can do with the ISIS TLV length. ISIS allows to advertise 
multiple TLVs of the same type, so we can advertise as many SIDs as we 
need as soon as we have space in the LSP set.

> relevant for this document?

##PP
fixed

> of H.Encaps.Red?  (I assume the L2 encapsulations from RFC 8986 are not

##PP
H.Encaps.Red omits the addition of the first SID in the SRH. Hence 
H.Encaps.Red supports one more SID than what is advertised in the 
H.Encaps MSD.

I have modified text to:

"The Maximum H.Encaps MSD Type signals the maximum number of SIDs that
can be added to the Segment List of an SRH as part of any "H.Encaps"
behavior as defined in RFC8986.

> but please also expand MTID on first use.

##PP
done

> networks where all routers support the flexible algorithm in question?

##PP
no, flex-algo does not require all routers to understand it. Only those 
ones that understands the flex-algo can participate in it though.

> deployment give grounds to violate the SHOULD NOT?

##PP
no.

> [the same list that does not include TLV 25, again]

##PP
fixed

> cases anyway, so maybe it is not quite so simple.)

##PP
Having both A and N bits sets is an error. If someone sets the A bit, it 
means the prefix is anycast and as such can not be a unique per node. 
Preferring the N flag and ignoring the A flag would be dangerous thing 
to do as that could result in prefix being used for purposes that are 
incompatible with A property.

>     The SRv6 Locator TLV has the following format:

##PP
done

> describes both a 3-octet metric field in sub-TLV 18 and a 4-octet metric

##PP
done

> is otherwise about OSPF).

##PP
Earlier we referred to ISIS specific Algorithm Sub-TLV.
Here we are referring to algorithm values, that have been defined by 
RFC8665.

I changed to:

"As defined in IGP Algorithm Types registry [RFC8665]" in all places.

> want the IANA registry to be controlling in the future?

##PP
I gave a reference to IANA registry section that specifies the supported 
Sub-TLVs for Locator TLV and removed the list from here.

>        Algorithm: 1 octet.  As defined in [RFC8665].

##PP
same response and change made.

> draft-ietf-6man-spring-srv6-oam and draft-ietf-lsr-flex-algo.

##PP
done.

> are configured differently and traffic sent to the one vs the other gets

##PP
that would be a mis-configuration. We are not defining the anycast 
functionality here, we are only providing a signalling for it. The 
signaling itself does not pose any security concern. The prefix can be 
anycast without the signaling.

> IS-IS, then RFC 8665 could be relegated to an informative reference.

##PP
please see my previous responses to the above matter.

> I think an article ("the" or "a" at the start of this sentence would

##PP
that section has been corrected as pointed out by others.

>        other IPv6 header contains an SRH.

##PP
right, this has been commented by others and I have fixed it already.

>     so may result in traffic being black-holed or mis-routed.

##PP
done

> A forward reference to the registry created in Section 11.5 might help.

##PP
done
> I suggest sorting the list of TLV numbers.

##PP
done

> [TLV 25, once again, does not appear in this list, as mentioned

##PP
fixed

> RFC 8355 doesn't mention "IPFRR", so I think a reference for IPFRR is in

##PP
done

> eligible for protection.  So maybe a different mnemonic would be better?

##PP
it's consistent with:

https://datatracker.ietf.org/doc/html/rfc8667#section-2.2.1

We are signaling the eligibility, not the backup state.

> Using "MUST be ignored" would reduce the diff against §8.2.

##PP
done

> As above, a forward-reference to the registry would be helpful.

##PP
done
> neighbor at the end of the section would reduce the diff against §8.1

##PP
done

> As above, a forward-reference to the registry would be helpful.

##PP
done

> "a behavior" would be more appropriate.

##PP
done

thanks,
Peter

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 21 May 2021 15:28:59 -0700

Hi Peter,

> good catch, I added it to all places.

Sounds good.  (I wasn't actually sure, myself, whether it made sense to put
these in TLV 25.)

> this and the use cases in my response to Erik

I'm happy to keep this discussion in Erik's ballot thread.

> behavior as defined in RFC8986.

Ah, good point, thanks for explaining and the updated text.

> incompatible with A property.

Agreed that both bits set is an error.
Also agreed that using the prefix for things incompatible with anycast is
dangerous.  I was just noting that it's also dangerous to have different
interpretations by different nodes on the (mixed) network.  I guess some
danger is unavoidable in the face of senders producing garbage...

> "As defined in IGP Algorithm Types registry [RFC8665]" in all places.

Okay.

> Sub-TLVs for Locator TLV and removed the list from here.

Thanks!

> https://datatracker.ietf.org/doc/html/rfc8667#section-2.2.1

Okay, thanks for the reference.

And thanks for all the other updates and replies.

I have cleared my discuss position in accordance with the updates in the
-15.

-Ben