Early Review of draft-ietf-idr-bgp-ct-srv6-03
review-ietf-idr-bgp-ct-srv6-03-opsdir-early-nainar-2024-03-27-00

Hi,

I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written with the intent of improving the operational aspects of
the IETF drafts per guidelines in RFC5706.

Comments that are not addressed in last call may be included
in AD reviews during the IESG review.  Document editors and WG chairs should
treat these comments just like any other last call comments.

Overall Summary:

This draft is a experimental track proposing the relevant mechanisms to apply
"Intent Driven Service Mapping" for SRv6 dataplane along with the relevant
extensions.

Based on my reading (version-03), there are a few gaps that needs to be
clarified or addressed from the operational aspects. I am marking it as "Has
issues" to get some clarification on the below:

A BGP CT node that does not support MPLS forwarding advertises the
   special label 3 (Implicit NULL) in the [RFC8277] MPLS Label field.
   The Implicit NULL label carried in BGP CT route indicates to
   receiving node that it should not impose any BGP CT label for this
   route.  Thus a pure SRv6 node carries Implicit NULL in the MPLS Label
   field in RFC8277 BGP CT NLRI.

<Nagendra> The above appears to be a bit misleading. What if a node supports
MPLS forwarding but is not enabled or if the node is connected to MPLS domain
and SRv6 domain?. I think that the intention of the above statement is to
instruct the receiver to use SRv6 encap. So I think it is better to rephrase
the above based on the encapsulation intent (SRv6 vs MPLS vs etc) instead of
mentioning it based on the protocol support.

The BGP Classful Transport route update for SRv6 MUST include an
   attribute containing SRv6 SID information, with Transposition scheme
   disabled.

<Nagendra> The being a normative MUST statement, I think it is better to
clarify more about what "attribute" are we referring here. The MUST is for
including "an attribute" or to to "disable the transposition scheme" or both?.
I think it is better to clarify the same - Something like "...SRv6 MUST include
BGP Prefix-SID attribute with SRv6 Service SUb-TLV and MUST disable the
transposition scheme".

The BGP Prefix-SID
   attribute as specified in [RFC9252] is used to carry this information
   correctly.

<Nagendra> I think RFC8669 defines BGP prefix-SID attribute and RFC9252
introduces SRv6 Service TLVs for Prefix-SID attribute.

If the [RFC9252] Prefix-SID attribute also contains a "SRv6 SID
   Structure Sub-Sub-TLV",

<Nagendra> The above comment is applicable for this one as well.

If the [RFC9252] Prefix-SID attribute also contains a "SRv6 SID
   Structure Sub-Sub-TLV", the Transposition Length is set to 0 and
   Transposition Offset is set to 0.

<Nagendra> Based on the normative MUST statement defined in Section 4, I
believe the above should be "MUST set the Transposition Length to 0"?. Or is it
not mandatory here?

A Next hop Resolution Scheme similar to that of BGP CT [BGP-CT] is
   used on IPv6 Unicast family to resolve “Colorful Prefix” locator
   routes that carry a mapping community to intent-aware paths in each
   domain.

<Nagendra> By "mapping community", are you referring to the "color extended
community" defined in draft-ietf-idr-cpr or the mapping community in Section
5.1 of draft-ietf-idr-bgp-ct?

If a BGP speaker considers a received BGP CT route invalid for some
   reason, but is able to successfully parse the NLRI and attributes,
   Treat-as-withdraw approach from [RFC7606] is used

<Nagendra> I think section 6 should refer section 7 of RFC9252 for additional
error handling related to SRv6 service SUb-TLVs included in the BGP-Prefix SID
attribute.

Thanks,
Nagendra