Re: [bess] [Idr] Review request for draft-ietf-bess-srv6-services-11

[Ketan Talaulikar <[e-mail]>]

Hi Jeff,

Thanks for your review and your feedback. Please check inline below for my
responses as a co-author of the draft.

We've also posted an update to address some of your comments along with
those from the IESG members :
https://datatracker.ietf.org/doc/html/draft-ietf-bess-srv6-services-12


On Sat, Mar 5, 2022 at 3:32 AM Jeffrey Haas <[e-mail]> wrote:

> Matthew,
>
> A few short summary notes before my detailed comments:
> I have concerns that this feature relies on scoping properties to restrict
> distribution of the Prefix SID Path Attribute, or the new TLVs in this
> document, that aren't strictly enforceable during incremental deployment.
> In some circumstances if this attribute bleeds between different SRv6
> domains this may cause forwarding issues.
>
> I have concerns that the re-use of BGP Labeled routes coupled with the
> contents of the new Path Attributes may be fragile in deployments that
> don't
> fully implement these features.  Resets of nexthop or otherwise changing of
> the labels as part of normal BGP procedures on routers ignorant of the
> feature can result in mis-forwarding.
>

KT> Would like to put things into perspective before responding to your
detailed feedback further inline below. We have a (relatively speaking new)
SR over IPv6 dataplane. The scope of this document is to cover the base BGP
signaling mechanism for this SRv6 network.


>
> Here are my comments on draft-ietf-bess-srv6-services:
>
> Meta question: This feature leverages the Prefix-SID feature.  The
> Label-Index TLV is a required  TLV for that feature.  It might be my
> relative unfamiliarity with SRv6 procedures, but where is this sub-TLV used
> in the context of this draft?
>

KT> Perhaps you might have overlooked the fact that RFC8669 specifies the
use of the BGP Prefix-SID attribute for the labelled-unicast SAFI alone? It
allows further documents to extend it for other SAFIs. It specifies the
Label-Index TLV to be required only for the labelled-unicast SAFI.


>
> Section 2:
>
> :   o  RESERVED (1 octet): This field is reserved; it SHOULD be set to 0
> :      by the sender and MUST be ignored by the receiver.
>
> I generally recommend the form of "MUST be set to 0 by the sender and
> SHOULD
> be ignored by the receiver".
>
> The intent is that the original version of the feature sends a predictable
> set of values (0), but permits the reception of values it might not know.
> The text quoted above, using SHOULD, means that you don't know what might
> be
> in the field but you'll never use it because it MUST be ignored.
>
> I'd recommend making this change throughout the document.
>

KT> I have seen all sorts of combinations of SHOULD/MUST being used at the
IETF in this context and heard arguments for all of those usages :-). That
said, I checked that the RFC8669 uses MUST for such situations and since
this document is defining extensions to that attribute, it makes sense to
do the same. We have made this change.


>
> :   A BGP speaker receiving a route containing BGP Prefix-SID Attribute
> :   with one or more SRv6 Service TLVs observes the following rules when
> :   advertising the received route to other peers:
> :
> :   o  if the nexthop is unchanged during the advertisement, the SRv6
> :      Service TLVs, including any unrecognized Types of Sub-TLV and Sub-
> :      Sub-TLV, SHOULD be propagated further.  In addition, all Reserved
> :      fields in the TLV or Sub-TLV or Sub-Sub-TLV MUST be propagated
> :      unchanged.
> :
> :   o  if the nexthop is changed, the TLVs, Sub-TLVs, and Sub-Sub-TLVs
> :      SHOULD be updated with the locally allocated SRv6 SID information.
> :      Any unrecognized received Sub-TLVs and Sub-Sub-TLVs MUST be
> :      removed.
>
> A Prefix-SID Path Attribute may pass through one or more systems that do
> not
> understand either the base RFC 8669 Prefix-SID Path Attribute, or does
> understand that attribute but does not understand these TLVs.
>
> Since this procedure is expected to be effected when nexthop change is
> done,
> how should the later systems behave when the nexthop wasn't in agreement
> and
> these procedures are not done?
>

KT> The document does call out this aspect and takes the approach where
this situation needs to be handled when setting up the peering with routers
that do not support this specification.


>
> If it was the case that the related nexthop was part of these new
> attributes, a downstream system might be able to recognize such cases and
> take an appropriate action.
>
> Section 3.2.1:
> :      The Transposition Offset MUST be less than LBL+LNL+FL+AL
> :
> :      The sum of Transposition Offset and Transposition Length MUST be
> :      less than LBL+LNL+FL+AL
>
> So... these are "MUST".  The Error Handling considerations say that
> semantic
> violations of values aren't malformed attributes.
>
> Exactly what is an implementation supposed to do with well-formed, but
> invalid nonsense?
>

KT> Please refer to the last paragraph of Sec 8. Such paths are not
considered for the best path calculation.


>
> :   BGP speakers that do not support this specification may misinterpret,
> :   on the reception of an SRv6-based BGP service route update, the part
> :   of the SRv6 SID encoded in MPLS label field(s) as MPLS label values
> :   for MPLS-based services.  Implementations supporting this
> :   specification MUST provide a mechanism to control the advertisement
> :   of SRv6-based BGP service routes on a per-neighbor and per-service
> :   basis.  The details of deployment designs and implementation options
> :   are outside the scope of this document.
>
> This is highly problematic.  These attributes are carried in standard BGP
> Labeled Unicast routes.


KT> This document has nothing that applies to SAFI 4.


> If this stuff is intended to be scoped with such
> consequences, normally this would require a different AFI/SAFI, or
> something
> like BGP capabilities.
>

KT> BGP capabilities address some scenarios but not all (e.g., such
misconfigurations when RRs are used can still leave the service broken).
That said, BGP capability is an option that might help to some extent and
there is a proposal for the same in front of the WG. We should let that
progress through the WG process independently.


>
> :   BGP speakers that do not support this specification may misinterpret,
> :   on the reception of an SRv6-based BGP service route update, the part
> :   of the SRv6 SID encoded in MPLS label field(s) as MPLS label values
> :   for MPLS-based services.  Implementations supporting this
> :   specification MUST provide a mechanism to control the advertisement
> :   of SRv6-based BGP service routes on a per-neighbor and per-service
> :   basis.  The details of deployment designs and implementation options
> :   are outside the scope of this document.
>
> Recognizing that there are critical incremental deployment issues without
> having a strategy for dealing with them is negligent.
>
> If this were solely a matter of a node that understands its Attribute and
> underlying TLVs and must figure out what to about it prior to sending it to
> another BGP Speaker, we'd be fine.  The AIGP feature spent considerable
> effort dealing with this problem and used optional non-transitive rather
> than transitive attributes to deal with this.
>

KT> The "negligent" attribution seems unfair here. When taking the example
of AIGP, you may be comparing an apple with an orange when it comes to the
functionalities that these attributes introduce. Comparison with other
specifications that cover signaling of encapsulation information is perhaps
more apt when it comes to the aspect of incremental deployment. You will
find that this proposal of the BESS WG is following precedents.


>
> Section 4:
> :   To achieve efficient packing, this document allows the encoding of
> :   the SRv6 Service SID either as a whole in the SRv6 Services TLVs or
> :   the encoding of only the common part of the SRv6 SID (e.g., Locator)
> :   in the SRv6 Services TLVs and encoding the variable (e.g., Function
> :   or Argument parts) in the existing label fields specific to that
> :   service encoding.  This later form of encoding is referred to as the
> :   Transposition Scheme where the SRv6 SID Structure Sub-Sub-TLV
> :   describes the sizes of the parts of the SRv6 SID and also indicates
> :   the offset of the variable part along with its length in SRv6 SID
> :   value.  The use of the Transposition Scheme is RECOMMENDED for the
> :   specific service encodings that allow it as described further in
> :   Section 5 and Section 6.
>
> This feature can operate on BGP Labeled Unicast, and may pass through nodes
> that are ignorant of the mechanisms in this draft. What do you do when a
> BGP
> Speaker does a next-hop-self and causes the labels to be reset
> without impacting the underlying SRv6 Service SID?  This is the other side
> of the question about nexthops changing.
>

KT> Please see previous comments. Also, this document does not apply to
SAFI 4.


>
> :   When steering for SRv6 services is based on shortest path forwarding
> :   (e.g., best-effort or IGP Flexible Algorithm
> :   [I-D.ietf-lsr-flex-algo]) to the egress PE, the ingress PE
> :   encapsulates the IPv4 or IPv6 customer packet in an outer IPv6 header
> :   (using H.Encaps or H.Encaps.Red flavors specified in [RFC8986]) where
> :   the destination address is the SRv6 Service SID associated with the
> :   related BGP route update.  Therefore, the ingress PE SHOULD perform
> :   resolvability check for the SRv6 Service SID before considering the
> :   received prefix for the BGP best path computation.  The resolvability
> :   is evaluated as per [RFC4271].
>
> BGP route resolvability, normally done on the BGP nexthop field or its
> MP_REACH_NLRI version of it, provides two inputs to BGP route selection:
> - Is the path reachable or not? (Feasibility check.)
> - What is its cost?
>
> We have examples like RFC 9012 for tunnel encapsulation where the
> feasibility check is augmented, but it doesn't bypass the IGP check.
>
> Since this feature may pass through routers ignorant of its contents, there
> can exist routers in the network that provide route selection based on the
> BGP nexthop, and others that do their distance calculation based on the
> SRv6
> services SID.  That could lead to inconsistent route selection in a network
> with this feature partially deployed.
>
> If you instead meant that the feasibility condition was augmented by "can
> you reach this SRv6 Services SID?" similar to Tunnel Encaps Attribute, you
> need different text.
>

KT> The check is augmented to include SRv6 SID reachability. Perhaps you
missed the text in the same section that says that existing BGP procedures
apply for the next-hop.


>
> Section 5.3/5.4 Global IPv4/IPv6 over SRv6 Core:
>
> In these procedures we're attaching the Prefix SID Path Attribute with the
> extensions defined in this document to routes that are likely Internet in
> scope.  With other address families, the likelihood of the Path Attribute
> getting leaked out of scope of networks that can process it, or pass
> through
> BGP Speakers that do not filter it, increases.
>
> These scenarios radically increase the likelihood of the issues noted
> above.
>

KT> Previous response applies here as well.


>
> Section 6.0 has a similar resolvability check as noted in the comments
> above.


KT> as above


>
>
> Section 6.1.2:
> :   o  MPLS Label: 24-bit field carries the whole or a portion of the
> :      Function part of the SRv6 SID when the Transposition Scheme of
> :      encoding (Section 4) is used and otherwise set to Implicit NULL
> :      value.  In either case, the value is set in the high order 20 bits
> :      (e.g., as 0x000030 in the case of Implicit NULL).
>
> I'm not terribly familiar with RFC 8214 procedures so this question is
> asked
> in ignorance.  I can see that VNI may be encapsulated in this label (RFC
> 7438)
> and that VNI may be 24 bits.
>
> The procedure in this specification limits the use to 20 of the 24 bits.
>
> Is there expectation that this value be treated as a traditional MPLS label
> and thus we should see the bottom of stack bit set here?
>
> Is the transposition scheme always taking the value from these 20 bits or
> the full 24 bits?
>

KT> Please refer to the respective sections where the encoding is
discussed. In short, for the cases where the encoding follows RFC3107/8277
it is 20-bits and it is 24-bits for RFC7432 base.

Thanks,
Ketan


>
> -- Jeff
>
>
>
>
>
> On Fri, Feb 18, 2022 at 06:52:12PM +0000, Bocci, Matthew (Nokia - GB)
> wrote:
> > IDR Working Group
> >
> > This draft has completed working group last call in the BESS working
> group and is currently being reviewed by the IESG.
> >
> > We would appreciate review and comments by participants in the IDR
> working group.
> >
> > The latest version of the draft is available here:
> draft-ietf-bess-srv6-services-11 - SRv6 BGP based Overlay Services<
> https://datatracker.ietf.org/doc/draft-ietf-bess-srv6-services/>
> >
> > Please send any comments in reply to this thread by 26th February 2022.
> >
> > Thank you,
> >
> > Matthew
>
> > _______________________________________________
> > Idr mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/idr
>
>