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

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.

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?

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.

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

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?

:   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.  If this stuff is intended to be scoped with such
consequences, normally this would require a different AFI/SAFI, or something
like BGP capabilities.

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

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.

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

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.

Section 6.0 has a similar resolvability check as noted in the comments
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?

-- 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
> Idr@ietf.org
> https://www.ietf.org/mailman/listinfo/idr