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

From: "Susan Hares" <[email-masked]>
Date: Tue, 1 Mar 2022 08:10:34 -0500

Matthew:

Thank you for extending the deadline. 

Sue 

From: Idr [mailto:[email-masked]] On Behalf Of Bocci, Matthew (Nokia -
GB)
Sent: Friday, February 18, 2022 3:05 PM
To: [email-masked]
Cc: <[email-masked]>; [email-masked];
[email-masked]
Subject: Re: [Idr] Review request for draft-ietf-bess-srv6-services-11

IDR WG

Please note that the deadline for comments is Friday 4th March, NOT 26th
Feb.

Matthew

  _____  

From: Idr <[email-masked]> on behalf of Bocci, Matthew (Nokia - GB)
<[email-masked]>
Sent: Friday, February 18, 2022 6:52 pm
To: [email-masked]
Cc: <[email-masked]>; [email-masked];
[email-masked]
Subject: [Idr] Review request for draft-ietf-bess-srv6-services-11 

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

---

From: Jeffrey Haas <[email-masked]>
Date: Fri, 4 Mar 2022 17:01:53 -0500

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

> _______________________________________________
> Idr mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/idr

---

From: Ketan Talaulikar <[email-masked]>
Date: Sat, 5 Mar 2022 15:29:15 +0530

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

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

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

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

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

> Exactly what is an implementation supposed to do with well-formed, but

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

> Labeled Unicast routes.

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

> If this stuff is intended to be scoped with such

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.

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

> of the question about nexthops changing.

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

> you reach this SRv6 Services SID?" similar to Tunnel Encaps Attribute, you

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.

> These scenarios radically increase the likelihood of the issues noted

KT> Previous response applies here as well.

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

KT> as above

> Is the transposition scheme always taking the value from these 20 bits or

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

> > This draft has completed working group last call in the BESS working
> group and is currently being reviewed by the IESG.
> >
> draft-ietf-bess-srv6-services-11 - SRv6 BGP based Overlay Services<
> https://datatracker.ietf.org/doc/draft-ietf-bess-srv6-services/>
> >