Re: [Idr] Opsdir early review of draft-ietf-idr-bgp-ct-srv6-03

[Kaliraj Vairavakkalai <[e-mail]>]

Hi Narendra,

Thanks for the review and comments. Sorry for the delay in response.

Please see inline. KV>

Thanks
Kaliraj



Juniper Business Use Only
From: Idr <[e-mail]> on behalf of Nagendra Nainar via Datatracker <[e-mail]>
Date: Wednesday, March 27, 2024 at 4:35 PM
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: [Idr] Opsdir early review of draft-ietf-idr-bgp-ct-srv6-03
[External Email. Be cautious of content]


Reviewer: Nagendra Nainar
Review result: Has Issues

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

KV> A BGP-CT route can include multiple encapsulations (e.g. MPLS, SRv6, UDP-tunneling) based on what the
KV> advertiser supports, and is enabled for from perspective of tunnel egress functionality of the CT route being advertised.
KV> So, it is perfectly OK for a CT route to have both MPLS and SRv6 encap info. Sender is not restricting the receiver
KV> to chose any specific encap. The receiving BGP speaker can use either of these based on what receiver
KV> supports and its order of preference. This helps with migration and interop scenarios, as explained in the drafts
KV> (Section 6.3 and Section 11.3.2 of draft-bgp-ct], which is referred to in Sec 3 of draft-bgp-ct-srv6).

I think that the intention of the above statement is to
instruct the receiver to use SRv6 encap.

KV> As explained above, that is not the intent. The intent of above para is to just say that if an advertising BGP speaker
KV> does not support MPLS (or is not enabled for MPLS forwarding), and it supports SRv6, thus advertises only SRv6 encap info,
KV> such a node puts the Implicit NULL special label in RFC8277 NLRI label field of the BGP CT route.

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.

KV> It is the Prefix-SID attribute or it can be MNH<https://www.ietf.org/archive/id/draft-ietf-idr-multinexthop-attribute-00.html#section-5.4.3.3> in future. Currently we mention only Prefix-SID
KV> attribute, because MNH is still in the works.
KV> We see there is some repetition in text. We will rephrase it as:

The BGP Classful Transport route update for SRv6 MUST include an attribute containing SRv6 SID information with Transposition scheme disabled.
               The BGP Prefix-SID attribute as specified in [RFC9252<https://www.ietf.org/archive/id/draft-ietf-idr-bgp-ct-srv6-03.html#RFC9252>] is used for this purpose today.

If the [RFC9252<https://www.ietf.org/archive/id/draft-ietf-idr-bgp-ct-srv6-03.html#RFC9252>] Prefix-SID attribute contains a "SRv6 SID Structure Sub-Sub-TLV", the Transposition Length MUST be set to 0 and
Transposition Offset MUST be set to 0. This indicates nothing is transposed and that the entire SRv6 SID value is encoded in the
"SRv6 SID Information Sub-TLV".

KV> Hope that works

The MUST is for
including "an attribute" or to to "disable the transposition scheme" or both?.

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

KV> pls see above text.

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.

KV> Yes. And this draft needs the variant with ‘SRv6 SID information’ support, hence the reference is to RFC-9252.

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.

KV> by ‘above comment’ do you mean the comment about RFC8669?
KV> Since ‘SRv6 SID   Structure Sub-Sub-TLV’ is defined in 9252 only,
KV> I think we need to refer to 9252 and not 8669. Pls let me know
KV> if I am missing something.

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?

KV> Pls see text suggestion above. It is mandatory, so we have added MUST.

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?

KV> It is the mapping community. But as explained in bgp-ct sec 5.1<https://www.ietf.org/archive/id/draft-ietf-idr-bgp-ct-30.html#section-5.1>, Color-extended-community IS-A Mapping-community.
KV> the Mapping-Community is not an IANA-type, it is a ‘role’ that any BGP community can play.

KV> In this text, we can put (Sec 5.1, [BGP-CT] after the mapping community.

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.

KV> Ack!. We will put additional text referring to RFC9252 sec 7.

Thanks,
Nagendra


_______________________________________________
Idr mailing list
[e-mail]
https://urldefense.com/v3/__https://www.ietf.org/mailman/listinfo/idr__;!!NEt6yMaO-gk!HqgTNcsPmk5O-t-Ed-VopxVGIMfnGtNaf0BsL0LIsBDERQh8G6tQKJWuclEOuz7ZCdplf2-TKissxZU$<https://urldefense.com/v3/__https:/www.ietf.org/mailman/listinfo/idr__;!!NEt6yMaO-gk!HqgTNcsPmk5O-t-Ed-VopxVGIMfnGtNaf0BsL0LIsBDERQh8G6tQKJWuclEOuz7ZCdplf2-TKissxZU$>