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

[Benjamin Kaduk <[e-mail]>]

Hi Peter,

On Thu, May 20, 2021 at 01:54:14PM +0200, Peter Psenak wrote:
> Hi Benjamin,
> 
> thanks for your comments, please see inline (#PP):
> 
> On 18/05/2021 22:29, Benjamin Kaduk via Datatracker wrote:
> > 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.)
> 
> ##PP
> 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.)

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

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

> 
> > 
> > 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!
> 
> ##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.
> 
> > 
> > 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?
> 
> ##PP
> fixed
> 
> > 
> > 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.)
> 
> 
> ##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.

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

> 
> > 
> > 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.
> 
> ##PP
> done
> 
> > 
> >     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?
> 
> ##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.
> 
> 
> > Bringing things closer to this document, would the presence of a mixed
> > deployment give grounds to violate the SHOULD NOT?
> 
> ##PP
> no.
> 
> > 
> >     are therefore advertised as sub-TLVs in TLVs 22, 23, 222, 223, and
> >     141.
> > 
> > [the same list that does not include TLV 25, again]
> 
> ##PP
> fixed
> 
> > 
> > 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.)
> 
> ##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.

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

> > 
> > 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.
> 
> 
> ##PP
> done
> 
> 
> > 
> >          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.
> 
> ##PP
> done
> 
> 
> > 
> >          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).
> 
> 
> ##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.

Okay.

> 
> 
> > 
> >          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?
> 
> 
> ##PP
> I gave a reference to IANA registry section that specifies the supported 
> Sub-TLVs for Locator TLV and removed the list from here.

Thanks!

> 
> > 
> > Section 8.1
> > 
> >        Algorithm: 1 octet.  As defined in [RFC8665].
> > 
> > [same comment as above]
> 
> ##PP
> same response and change made.
> 
> 
> > 
> > Section 12
> > 
> > I'd also reference the security considerations of
> > draft-ietf-6man-spring-srv6-oam and draft-ietf-lsr-flex-algo.
> 
> ##PP
> done.
> 
> > 
> > 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?
> 
> ##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.
> 
> 
> > 
> > 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.
> 
> ##PP
> please see my previous responses to the above matter.
> 
> > 
> > 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.
> 
> ##PP
> that section has been corrected as pointed out by others.
> 
> 
> > 
> > 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/
> 
> ##PP
> right, this has been commented by others and I have fixed it already.
> 
> > 
> > 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/
> 
> ##PP
> done
> 
> > 
> > Section 7.2
> > 
> >        Optional Sub-sub-TLVs.
> > 
> > A forward reference to the registry created in Section 11.5 might help.
> 
> ##PP
> done
> > 
> > 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.
> 
> ##PP
> done
> 
> 
> > [TLV 25, once again, does not appear in this list, as mentioned
> > previously.]
> 
> ##PP
> fixed
> 
> 
> > 
> > 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.
> 
> ##PP
> done
> 
> > 
> > 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?
> 
> ##PP
> it's consistent with:
> 
> 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

> We are signaling the eligibility, not the backup state.
> 
> 
> 
> > 
> >           Reserved bits: MUST be zero when originated and ignored when
> >           received.
> > 
> > Using "MUST be ignored" would reduce the diff against §8.2.
> 
> ##PP
> done
> 
> > 
> >        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.
> 
> ##PP
> done
> > 
> > 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
> 
> 
> ##PP
> done
> 
> > 
> >        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.
> 
> ##PP
> done
> 
> > 
> > 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.
> 
> ##PP
> done
> 
> thanks,
> Peter
> 
> 
> > 
> > 
> > 
> > 
> > 
>