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 > i...@ietf.org > https://www.ietf.org/mailman/listinfo/idr _______________________________________________ BESS mailing list BESS@ietf.org https://www.ietf.org/mailman/listinfo/bess
