Re: [Idr] Opsdir early review of draft-ietf-idr-bgp-car-02

["Dhananjaya Rao (dhrao)" <[e-mail]>]

Hello Yingzhen,

Thank you so much for your reviews. Will fix the nit.

Regards,
-Dhananjaya

From: Yingzhen Qu <[e-mail]>
Date: Wednesday, January 3, 2024 at 10:10 PM
To: Dhananjaya Rao (dhrao) <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Re: Opsdir early review of draft-ietf-idr-bgp-car-02
Hi Dhananjaya,

Thanks for updating the draft and addressing my comments. I've read version -05, the overall quality and readability of the document have been improved substantially. All my major concerns have been answered, especially about why type 2 NLRI is needed.

One nits I noticed is that on the front page, it says co-authors are listed in section 11, which should be section 13.

Looking forward to your update presentation in the upcoming interim meeting.

Thanks,
Yingzhen





On Wed, Nov 15, 2023 at 1:03 PM Yingzhen Qu <[e-mail]<mailto:[e-mail]>> wrote:
HI Dhananjaya,

Ack. Thanks for addressing my comments. I'll review the updated draft and get back to you.

Thanks,
Yingzhen

On Wed, Nov 15, 2023 at 2:35 AM Dhananjaya Rao (dhrao) <[e-mail]<mailto:[e-mail]>> wrote:
Hi Yingzhen,

Thank you once again for your thoughtful review.

We’ve made a couple of updates to the draft. I’m hoping they address most of the comments and feedback you had provided.

I’ve also included some clarifications inline (please check for DR#).

From: Yingzhen Qu via Datatracker <[e-mail]<mailto:[e-mail]>>
Date: Wednesday, August 23, 2023 at 1:18 AM
To: [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>
Cc: [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>, [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>
Subject: Opsdir early review of draft-ietf-idr-bgp-car-02
Reviewer: Yingzhen Qu
Review result: Has Issues

Hi,

Thanks for the draft.

I am the assigned OPSDIR reviewer to conduct an "early" review of this draft.

General comments:

There are lots of abbreviations in the draft. I'd suggest to add them
in the the terminology section. For example, I'd assume BR means Border
Router, but there might be different guessing.
DR# Ack, we have expanded on abbreviations.

In this draft, it says E is globally unique, which makes E-C in that
order unique. Can you please explain a bit more about the second unique?
I suppose it's possible to have two different source nodes, E1 and E2,
all reach destination E with color C, correct?
DR# Uniqueness is for the destination (E)’s BGP route. The data model of E,C keeps the route NLRI unique end to end even though color in NLRI may map to another intent in a different color (administrative) domain during propagation, since E is globally unique in provider network and scopes the color. Section 12 discusses the operational/manageability considerations.
DR# I assume by source you mean an ingress node. Both source nodes above E1 and E2 will receive the (E,C) route without any change in the NLRI, and can use it to send traffic to the destination node E.

The draft has an informative reference to
[I-D.hr-spring-intentaware-routing-using-color], which is an important problem
statement for this solution. Will the problem statement draft progress as well?
Even so, to improve the readability of the bgp-car draft, I'd suggest adding
some text for a brief introduction of the problem.
DR# Yes, the problem statement is awaiting adoption call in spring.
DR# We have also expanded the Introduction section.

IP Prefix NLRI was added in version -02. The use case is where a unique
routable IP prefix is assigned a given intent or color. In other words,
the IP is overloaded with a color. The same can be achieved using an IP
with a color. I'm not totally convinced that this type 2 NLRI is needed.
Please clearly specify when it should be used.
DR# We have tried to clarify the usage of Type-2 for different cases in the updated versions, specifically in Section 8.
DR# The expanded intro section also describes the use-case/motivation a bit more.

Please consider adding a section for operation considerations. There are
pieces of information about operation and deployment scattered in the
document, please consider group them together.
DR# We did have a Manageability Considerations section 12, renamed it. SRv6 specific operational considerations are in Section 8.3, since the SRv6 specific items are in Section 8.

There are quite some sentences missing "." at the end. Please do an
editorial pass and fix them.
DR# Done, thanks.

Detailed comments with line # from idnits:

478        The value set (or appropriately incremented) in the AIGP TLV
479        corresponds to the metric associated with the underlying intent of
480        the color.  For example, when the color is associated with a low-
481        latency path, the metric value is set based on the delay metric.

483        Information regarding the metric type used by the underlying intra-
484        domain mechanism can also be set.

comment: This statement lacks a clear definition how the metric should be set.
DR# Updated to indicate it’s the metric value which is set based on the path/metric type.

486        If BGP CAR routes traverse across a discontinuity in the transport
487        path for a given intent, add a penalty in accumulated IGP metric
488        (value by user policy).  For instance, when color C1 path is not
489        available, and route resolves via color C2 path (e.g., Appendix A.3).

How about the case where encapsulations are different? For example, SR
policy in one AS and IGP-FlexAlgo in the other AS vs. SR Policy in both ASes.
DR# metric is independent of encapsulation type. For example delay metric can be provided by IGP FA or SR policy.
DR# That said, the metric value/penalty can be updated differently by user depending on the path type.
Section 2.7
504        The (E, C) route inherently provides availability of redundant paths
505        at every hop, identical to BGP-LU or BGP IP.

"every hop" is a bit confusing here since it may mean an IGP hop within
an AS. To my understanding, this section means ECMP or backup paths can
provide protection in case of failure within an AS domain without impact
other ASes.
DR# Clarified to indicate it’s every BGP hop. And yes.

"Path Availability" as the section title is not very clear. How about something
like "Inherent Path Protection"?
DR# Made it “Native Multipath Capability”, as path protection could be misinterpreted.

513        BGP ADD-PATH should be enabled for BGP CAR to signal multiple next
514        hops through a transport RR.

I'd suggest to change to "SHOULD be enabled".
DR# Done
526        The BGP CAR solution seamlessly supports this (rare) scenario while

I'd suggest adding a small paragraph explaining why this is a rare but
useful case. I would guess the tow domains used to belong to different
administrators, now they're trying to merge under one admin domain.
nits: personally I don't like how "(rare)" with parentheses is used
here, but I'd leave this to the authors.
DR#. Section 10 (manageability) describes some considerations. The use-case/requirement is described in the problem statement draft.
DR# Removed parenthesis.

806        NLRI instead of the BGP Prefix SID attribute.  The BGP Prefix SID
807        Attribute SHOULD be omitted from the labeled color-aware routes when
808        the attribute is being used to only convey the Label Index TLV.
Add a reference to Appendix D?
DR# Done.

848        BGP CAR SRv6 SID TLV definitions provide the following benefits:

850        *  Native encoding of SIDs avoids robustness issue caused by
851           overloading of MPLS label fields.

853        *  Simple encoding to signal Unique SIDs (non-transposition),
854           maintaining BGP update prefix packing

856        *  Highly efficient transposition scheme (12-14 bytes saved per
857           NLRI), also maintaining BGP update prefix packing
minor: I don't think the text belongs to the encoding section. Maybe
part of "Operation Considerations"?
DR# This text describes the reasoning for the CAR encoding design, and was added based on comments during adoption. Hence, we’ve kept it inline to provide context.

1019       *  If multiple instances of same type are encountered, all but the
1020          first instance MUST be ignored.

1022       *  If multiple instances of same type are encountered, all but the
1023          first instance MUST be ignored.
nits: please remove the repetition.
DR# Done, thanks.

1025       *  A TLV is not considered malformed because of failing any semantic
1026          validation of its Value field.
Q: When should a TLV be considered malformed? and how should it be handled?
DR# TLV validation and its handling is already specified above in this section for type-specific Non-key TLVs.

1033    3.  Service route Automated Steering on Color-Aware path
nits: Service Route Automated Steering on Color-Aware Path
Please check to make sure all section titles are consistent.
DR# Ack, done.
1044       destination, per-flow, CO-only.  For brevity, in this revision, we
1045       refer the reader to the [RFC9256] text.
nits: maybe change to something like "For brevity, please refer to [RFC9256]
section X for detail."?
DR# Ack, done.

1047       Salient property: Seamless integration of BGP CAR and SR Policy.
minor: personally I don't think this sentence belong to this section.
DR# Removed.

1055    4.  Intents
The section title and content don't seem to match. I don't quite understand
the purpose of this section.
DR# It’s meant to include illustrations for various intent use-cases. We’ve removed this section as some cases are already included in Appendix.

1085       A separate document will analyze the BGP CAR supports for 3, 5 and 6.
Any reference?
DR# Not yet.

1097    5.1.  (E, C) Subscription and Filtering
Q: how is this subscription sent between routers?
DR# It would be via a BGP route, but the specification/details are out of scope for this document.
1115       *  If A does not have (E2, C1), it will advertise F (E2, C1) to its
1116          peer B
I suppose it meant to be "If A does not have subscription of (E2, C1)"
DR# It’s if A doesn’t have the route.
1124       On-demand filtering procedures are outside the scope of this
1125       document.
what's "on-demand filtering"?
DR# It’s on-demand subscription and filtering – fixed.

1138       Two key principles used to address the scaling requirements are a
1139       hierarchical network and routing design, and on-demand route
1140       subscription and filtering.
Q: on-demand filtering is claimed to be out of the scope. (line #1124)

DR# The hierarchical design is the primary technique which is described. The subscription is optional for control plane optimization. it’s included for completeness of the framework.

1342       Note: E1 does not need the BGP CAR (451, C1) route
Q: what's the benefit?
DR# The benefit is explained in Section 6.3 last bullet where it’s the case of “E1 needs BGP CAR (451, C1).”
“the ingress PE needs an additional BGP transport level recursion and pushes a BGP VPN label and two BGP transport labels.  It may also need to handle load-balancing for the egress ABRs.  This is the most complex dataplane option for the ingress PE.”

1545    7.  Routing Convergence
comments: Maybe section 2.7 and 7 should be put together somehow, but I'll
leave this to the authors.
DR# We’ve kept it as is for now to avoid reorganizing the section.

1602       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
1603       |  NLRI Length  |  Key Length   |   NLRI Type   |Prefix Length  |
1604       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Q: what's value of the NLRI Type here?
DR# Same as for CAR SAFI.

1664       service route is advertised by the egress PE with a Color Ext-Comm C.
nits: "Color Ext-Comm" is only used here once while "Color Extended-Community"
elsewhere.
DR# Done.

Nits: this section 9.2.1 and 9.2.2 needs some editorial work. For
example, each bullet point should be a sentence finished with a ".".
DR# Done.

1796       CAR SAFI may also be used for best-effort routes in addition to
1797       intent-aware routes.
Q: Should a color be specified here? or use the IP Prefix NLRI Type 2?
DR# Its Type-2 without color. Clarified in Section 8.

2008       This extension defines a new SAFI within a BGP and therefore does not
it should be two new SAFIs: BGP CAR/83 and BGP VPN CAR/84

2307       The examples use MPLS/SR for the transport data plane.  Scenarios
2308       specific to other encapsulations will be added in subsequent
2309       versions.
nits: this should be removed.
DR# Ack, fixed.
Regards,
-Dhananjaya

Thanks,
Yingzhen