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

[Peter Psenak <[e-mail]>]

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.

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


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


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

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



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


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

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


> 
> 
> 
> 
>