Segment Routing over IPv6 Argument Signaling for BGP Services
draft-ietf-bess-bgp-srv6-args-10

 Thanks to Mališa Vučinić for their secdir review.

# Éric Vyncke, INT AD, comments for draft-ietf-bess-bgp-srv6-args-08
CC @evyncke

Thank you for the work put into this document.

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and one nit.

Special thanks to Zhaohui (Jeffrey) Zhang for the shepherd's write-up including the WG consensus and the justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS (non-blocking)

### Abstract

Suggest removing the first sentence.

Possibly add a reference to RFC 8986 network programming as this is specific to net-pgm and not plain SRv6.

### Section 1

s/SRv6 refers to Segment Routing instantiated on the IPv6 data plane/SRv6 refers to Segment Routing instantiated *over* the IPv6 data plane/ ?

*VERY* ambiguous statement `SRv6 Service Segment Identifier (SID) refers to an SRv6 SID` (recursive and conflicting redefinition of SID)

### Section 2

`As specified in [RFC8986], an SRv6 SID comprises three components:` is not correct as RFC 8402 that defines SID has not this structure. So, let's be specific and states that "RFC 8946 SID comprises three components" or something similar.

### Section 3.1

s/Ethernet Auto-Discovery (A-D) ... are utilized/s/Ethernet Auto-Discovery (A-D) ... *is* utilized/ ?

`::0` is not a canonical format for "::".

s/0:0:0:0:aaaa::/::aaaa:0:0:0/ (also in section 3.3)

### Section 3.3

`BUM` is already define before.

### Being more generic ?

Sections 3.1 and 3.2 seems very specific, but should the described mechanism be more generic ? The bit-wise OR seems indeed too simplistic and this mechanism could be used for other end point behavior beyond those described in section 3.1 and 3.2

## NITS (non-blocking / cosmetic)

### Use of SVG graphics

To make a much nicer HTML rendering, suggest using the aasvg too to generate SVG graphics. It is worth a try especially if the I-D uses the Kramdown file format ;-)

I found no-transport-related concerns in this document.

I have one comment:

“Additionally, as a
   non-zero ARG value is being signaled, the Argument Length (AL) MUST
   be set to the size of the ARG, and the size SHOULD be a multiple of
   8. “
- WHY SHOULD? I may have missed, but did not see any reason why the length is specified this way.
- Perhaps it would be useful to note how receivers are to process an AL size when not a multiple of 8?

Thank you for this document. I have a few nits but nothing major:

  == There are 3 instances of lines with non-RFC3849-compliant IPv6 addresses
     in the document.  If these are example addresses, they should be changed.

16	   RFC9252 defines procedures and messages for BGP Services for Segment

Jim> Note that RFC 9252 says “BGP overlay services” not “BGP services”. Can we make this consistent with the cited reference. 

74	1.  Introduction

76	   SRv6 refers to Segment Routing instantiated on the IPv6 data plane
77	   [RFC8402].  SRv6 Service Segment Identifier (SID) refers to an SRv6

Jim> Shouldn’t this be something like (SSID) rather than (SID) as the abbreviation does not align with the text (?). SRv6 Service SID is a type of SID but not the only one so using SID as the abbreviation seems wrong. It would be better to just say “SRv6 Service SID” which is inline with what RFC 9252 does.

83	   signaling of BGP services including L3VPN, EVPN, and Internet

Jim> Again, s/BGP services/BGP overlay services

84	   services using SRv6 as data plane.

Jim> SRv6 is not really the data plane, IPv6 is. I would suggest to remove “as data plane” and just end the sentence at “SRv6”.

86	   For certain EVPN services, [RFC8986] introduced the End.DT2M SRv6
87	   Endpoint Behavior, which utilizes arguments (i.e., Arg.FE2).

Jim> Please expand the reference to RFC8986 Section 4.12 which is where End.DT2M is defined in that RFC.

142	   As specified in Section 3.2.1 of [RFC9252], the SRv6 SID Structure
143	   sub-sub-TLV MUST be included when signaling an SRv6 SID corresponding

Jim> s/SRv6 SID Structure sub-sub-TLV/SRv6 SID Structure Sub-Sub-TLV

397	           are both non-zero and but not equal, then no usable ARG value

Jim> s/and but not equal/but not equal

Hi Ketan, Syed, Jorge, & Wen,

Thank you for the effort put into this well-written document.

Also, thanks to Joe Clarke for the OPSDIR review. I noted that Ketan promised a revised version ;-)

The document inherits deployment/ops considerations in RFC9252. The document reasonably includes provisions to ease troubleshooting (logging, in particular). Support of tracing-like capabilities would help detecting when inconsistent structures would be interesting to investigate as future work.

Please find below some comments, many are nits:

# Expand SRv6 in the title/abstract

# The abstract should be self-contained. Consider at least adding title of the cited RFC and expand all acronyms.

# “Internet services”: I know that 9252 uses that term but still I don’t parse it :-) Do we meant “IP Connectivity services”? If so, consider updating that.

# Is there a special meaning associated with “BGP Service”? I see 9252 uses both variants “BGP Services” and “BGP services”, though. Likewise, both flavors are used in this spec. Need some clarity here.

# Section 1

## Do we have any public pointer to cite for “implementation and interoperability testing”?

CURRENT:
   implementation and interoperability testing, it was observed that the
   specifications outlined in [RFC9252] lacked sufficient detail,
   leading to ambiguities in interpretation and implementation.

## Can we cite an example of similar endpoint behavior?

CURRENT:
   described herein are also applicable to other similar endpoint
   behaviors with arguments that may be signaled using BGP.

## (nitty nit) TPOS-O and TPOS-L are not used in RFC9252 as such. I understand that this is introduced here for the illustration examples. Maybe add that note as a legend to one of the example and delete these mentions here:

CURRENT:
   Consequently, the Transposition Offset (TPOS-O) and Transposition
   Length (TPOS-L) are set to zero, and references to MPLS label fields

# Section 2

## nit

OLD:
   For SRv6 SIDs
   associated with SRv6 Endpoint Behaviors that do not support argument

NEW:
   For SRv6 SIDs
   associated with SRv6 Endpoint Behaviors that do not support arguments,

## I don’t think the following a new behavior to justify the use of normative language. Is it?

CURRENT:
    Consequently, all bits following
   the FUNC portion MUST be set to zero, and the argument length MUST be
   zero.

## This is already part of 9252 which is updated by this doc. I don’ think the normative language is justified here.

CURRENT:
   As specified in Section 3.2.1 of [RFC9252], the SRv6 SID Structure
   sub-sub-TLV MUST be included when signaling an SRv6 SID corresponding
   to an endpoint behavior that supports argument.  

## I guess this is using “SRv6 SID Structure Sub-Sub-TLV”. Can we say that in the text?

CURRENT:
   Since arguments may be optional, the SRv6 Endpoint Node that owns the
   SID MUST advertise the SRv6 SID Structure along with the LOC:FUNC

# Section 3.1: Check

CURRENT:
   Since the End.DT2M behavior
   supports the use of an ARG, an SRv6 SID Structure sub-sub-TLV MUST be
   included.  

Isn’t that covered by:

   “the SRv6 Endpoint Node that owns the
   SID MUST advertise the SRv6 SID Structure along with the LOC:FUNC
   portion of the SRv6 SID to indicate whether arguments are supported
   for that specific SID”?

# Section 3.3

## (nit) s/The ingress Provider Edge (PE) router/The ingress PE

## (nit) s/Figure 7 below/Figure 7

# Section 4

OLD: specified in document in Section 3.3 MUST be used to correctly derive the SRv6
NEW: specified in Section 3.3 MUST be used to correctly derive the SRv6

Cheers,
Med

Thank you to Mallory Knodel for the GENART review.