ietf-wg-idr / draft-ietf-idr-bgp-car

0 stars 0 forks source link

OPS-DIR review on -08 #27

Closed suehares closed 4 months ago

suehares commented 4 months ago

OPS-DIR review of draft-ietf-idr-bgp-car-08.txt (4/23/2024)

Thanks to the authors for their work on this draft. The readability and clarity of have improved a lot since my last review of version -02.

I have the following comments and questions for the authors to consider.

OPS-DIR-01

General comment: Please consider changing "ISIS" to "IS-IS".

OPS-DIR-02

The line numbers are from idnits:

191 BGP CAR SAFI can be enabled on transport devices in a provider 192 network (underlay) to set up color-aware transport/infrastructure 193 paths across the provider network. The multi-domain transport 194 network may comprise of multiple BGP ASes as well as multiple IGP 195 domains within a single BGP AS.

Q: is it possible to set up a path cross multiple service providers?

OPS-DIR-02

331 * BR: An inter-domain Border Router, either for an IGP Area (ABR) or 332 a BGP Autonomous System (ASBR).

I'd suggest change "An inter-domian Border Router" to "Border Router".

OPS-DIR-03

491 the color-aware route. Color is not necessary in NLRI key as a 492 distinguisher.

"not necessary" sounds like color may still be there. Suggest to change to "not needed".

OPS-DIR-04

500 * AIGP Metric [RFC7311]: accumulates color/intent specific metric

Please expand "AIGP" here, since it shows up for the first time.

OPS-DIR-05

831 The key length also helps make error handling more resilient and 832 minimally disruptive.

s/helps make error handling/helps error handling

OPS-DIR-06

1044 If a BGP transport CAR speaker sets itself as the next hop while 1045 propagating a CAR route, it allocates a local label for the specific 1046 prefix and color combination. When the received BGP update has the 1047 CAR Label Index TLV, the speaker SHOULD use that hint to allocate the 1048 local label from the SR Global Block (SRGB) using procedures as 1049 specified in [RFC8669] Section 4.

Q: Should here be allocating a SR SID instead of a local label?

OPS-DIR-07

1207 [I-D.hr-spring-intentaware-routing-using-color]":

nits: please remove the extra ".

OPS-DIR-07

1487 5.2. Deployment model nits: s/model/Model

OPS-DIR-08

1518 Figure 3

nits: please add a title for Figure 3.

OPS-DIR-09

2440 routes must not be filtered, otherwise the desired intent will not be

Should this be "MUST"?

suehares commented 4 months ago

OPS-DIR-01 - review with -09 text

Status: Resolved in -09.txt

dhrao1 commented 4 months ago

We have updated the text, will publish in -09.

suehares commented 4 months ago

DJ: Super. I will check the text in -09.

suehares commented 4 months ago

OPS-DIR-02 - review for -09.txt

status: Closed, resolved

Issue: 331 * BR: An inter-domain Border Router, either for an IGP Area (ABR) or 332 a BGP Autonomous System (ASBR).

I'd suggest change "An inter-domian Border Router" to "Border Router".

suehares commented 4 months ago

OPS-DIR-03 - review in -09.txt

-09-status: Resolved, closed

Issue: 491 the color-aware route. Color is not necessary in NLRI key as a 492 distinguisher.

"not necessary" sounds like color may still be there. Suggest to change to "not needed".

suehares commented 4 months ago

OPS-DIR-04 - review in -09.txt

-09-status: Resolved, Closed

Resolved by adding an abbreviation in section 1.1.

Issue: 500 * AIGP Metric [RFC7311]: accumulates color/intent specific metric

Please expand "AIGP" here, since it shows up for the first time.

suehares commented 4 months ago

OPS-DIR-05 - review in -09

-09-status: Resolved, Closed.

Issue: 831 The key length also helps make error handling more resilient and 832 minimally disruptive.

s/helps make error handling/helps error handling

suehares commented 4 months ago

OPS-DIR-06 -SR SID instead of local label

-09-status: resolved, closed.

Issue:

1044 If a BGP transport CAR speaker sets itself as the next hop while 1045 propagating a CAR route, it allocates a local label for the specific 1046 prefix and color combination. When the received BGP update has the 1047 CAR Label Index TLV, the speaker SHOULD use that hint to allocate the 1048 local label from the SR Global Block (SRGB) using procedures as 1049 specified in [RFC8669] Section 4.

Q: Should here be allocating a SR SID instead of a local label?

suehares commented 4 months ago

OPS-DIR-07 -09-status: Resolved, closed.

1207 [I-D.hr-spring-intentaware-routing-using-color]":

nits: please remove the extra ".

suehares commented 4 months ago

OPS-DIR-08 - review in -09.txt

status: Resolved

1518 Figure 3

nits: please add a title for Figure 3.

suehares commented 4 months ago

OPS-DIR-09 in -09.txt (pre)

-09-status: Resolved, closed

Issue: 2440 routes must not be filtered, otherwise the desired intent will not be

Should this be "MUST"?

suehares commented 4 months ago

Closed issues: OPS-DIR-01, OPS-DIR-02, OPS-DIR-03, OPS-DIR-04, OPS-DIR-05, OPS-DIR-6, OPS-DIR-07, OPS-DIR-08, OPS-DIR-09

suehares commented 4 months ago

All OPS-DIR issues resolved, closing main issue.