[Idr] Re: AD review of draft-ietf-idr-bgp-car-10

[Dhananjaya Rao <[e-mail]>]

Hi John,

Thank you for your detailed review and feedback. We will check the comments
and respond/update the draft.

Regards,
-Dhananjaya

On Mon, Jan 20, 2025 at 6:24 PM John Scudder <[e-mail]> wrote:

> Hi Authors, WG,
>
> Thanks for your patience as I mostly completed this review. I say “mostly”
> because I haven’t reviewed the appendices yet. These may clear up some of
> the questions I’ve asked, and it’s also possible I may have further
> questions about the appendices, but I thought it better to get the review
> of the main body shipped off now without further delay.
>
> I’ve supplied my questions and comments in the form of an edited copy of
> the draft. Minor editorial suggestions I’ve made in place without further
> comment, more substantive questions and comments are done in-line and
> prefixed with “jgs:”. You can use your favorite diff tool to review them;
> I’ve attached the iddiff output for your convenience if you’d like to use
> it. I’ve also pasted a traditional diff below in case you want to use it
> for in-line reply.
>
> Thanks,
>
> —John
>
> --- draft-ietf-idr-bgp-car-10.txt       2025-01-20 19:28:43
> +++ draft-ietf-idr-bgp-car-10-jgs-comments.txt  2025-01-20 21:18:34
> @@ -7,6 +7,19 @@
>  Intended status: Experimental                              Cisco Systems
>  Expires: 28 October 2024                                      Co-authors
>                                                                Section 13
> ++--
> +jgs: While I applaud your creativity, please stick with the accepted
> +practice of listing additional contributors in the "Contributors"
> +section, and don't invent a new category of back-page "co-authors".
> +
> +If you'd like, you can follow the example of
> +https://www.rfc-editor.org/rfc/rfc9681.html#name-contributors
> +and include a statement such as,
> +
> +   The following people gave substantial contributions to the content of
> +   this document and should be considered as coauthors:
> +
> ++--
>                                                             26 April 2024
>
>
> @@ -17,6 +30,15 @@
>
>     This document describes a BGP based routing solution to establish
>     end-to-end intent-aware paths across a multi-domain transport
> ++--
> +jgs: I think it would be useful to state someplace early on what
> +"transport" means in the context of this document, since it will
> +also be reviewed by people who think "transport" means UDP, TCP,
> +and QUIC. This goes double for "transport layer" which is more
> +commonly understood to mean OSI reference model layer 4. That one
> +at least, I think should go in the Terminology section, I'll add
> +a stub.
> ++--
>     network.  The transport network can span multiple service provider
>     and customer network domains.  The BGP intent-aware paths can be used
>     to steer traffic flows for service routes that need a specific
> @@ -282,9 +304,16 @@
>  Internet-Draft        BGP Color-Aware Routing (CAR)           April 2024
>
>
> -    |             | concept w.r.t. routing beyond best-effort,        |
> +    |             | concept with respect to routing beyond best-effort,
>       |
>      |             | compared to intent as declarative abstraction in  |
>      |             | [RFC9315].                                        |
> ++--
> +jgs: I take it the definition is deliberately limiting. I can't
> +immediately think of something else that wouldn't be covered by
> +a combination of path, service insertion, and PHB, but I wanted
> +to check that you intend and prefer that any other (hypothetical?)
> +thing would be out of scope.
> ++--
>      +-------------+---------------------------------------------------+
>      +-------------+---------------------------------------------------+
>      | Color       | A 32-bit numerical value associated with an       |
> @@ -373,6 +402,11 @@
>      |             | IGP Flex-Algo, BGP CAR) or traditional routing/TE |
>      |             | path (e.g.  RSVP-TE, IGP/LDP, BGP-LU).            |
>      +-------------+---------------------------------------------------+
> ++--
> +jgs: Transport
> +     Transport Network
> +     Transport Layer
> ++--
>
>                                    Table 1
>
> @@ -574,7 +608,20 @@
>           for an intent (i.e, IP Prefix == Intent or Color) distinguishes
>           the color-aware route.  Color is not needed in NLRI key as a
>           distinguisher.
> ++--
> +jgs: The use of non-descriptive names is a particular bugaboo of mine.
> +E.g., I always have to go back and look up what the differences are
> +between Options A, B, and C. And let's not even start on all the
> +different BESS NLRI variants all named Type-foo.
>
> +I don't insist, but please consider whether you could use meaningful
> +names for these throughout your document and not name them after their
> +code points, which names will have no meaning to anyone not immersed in
> +the document. I acknowledge it would be a large diff set, but it's
> +potentially a straightforward search-and-replace job so not that much
> +labor.
> ++--
> +
>     *  NLRI non-key encapsulation data: Data such as MPLS label stack,
>        Label Index and SRv6 SID list associated with NLRI.  Contained in
>        TLVs as described in section 2.9.2.1 - 2.9.2.3.
> @@ -624,6 +671,10 @@
>     *  Key length field: specifies the key length.  It allows new NLRI
>        types to be handled opaquely, which permits transitivity of new
>        route types through BGP speakers such as Route Reflectors.
> ++--
> +jgs: See my comments later in the doc about needing to cover garbage
> +bits.
> ++--
>
>     *  TLV-based encoding of non-key part of NLRI: This allows the
>        inclusion of additional non-key fields for a prefix to support
> @@ -787,14 +838,21 @@
>
>
>     Additional AIGP extensions may be defined to signal state for
> -   specific use-cases: MSD along the BGP CAR route advertisement,
> +   specific use-cases: Maximum SID Depth (MSD) along the BGP CAR route
> advertisement,
>     Minimum MTU along the BGP CAR advertisement.  This is out of scope
>     for this document.
> ++--
> +jgs: One of your use cases is "along the BGP CAR route advertisement"
> +whereas the other is "along the BGP CAR advertisement". Is there some
> +subtle difference between these that led you to omit the word "route" in
> +the second? If so, please help me understand. If not, please make
> +consistent.
> ++--
>
>  2.7.  Native MultiPath Capability
>
>     The (E, C) route definition inherently provides availability of
> -   redundant paths at every BGP hop, identical to BGP-LU or BGP IP.  For
> +   redundant paths at every BGP hop identical to BGP-LU or BGP IP.  For
>     instance, BGP CAR routes originated by two or more egress ABRs in a
>     domain are advertised as multiple paths to ingress ABRs in the
>     domain, where they become equal-cost or primary-backup paths.  A
> @@ -802,6 +860,20 @@
>     locally within the domain for faster convergence, without any
>     necessity to propagate the event to upstream nodes for traffic
>     restoration.
> ++--
> +jgs: I removed a comma above, to change the shade of meaning. I think it
> +brings the paragraph closer to reality. With the comma, the impression
> +given is that vanilla BGP "inherently provides availability of redundant
> +paths at every BGP hop", which isn't true -- as we know, one of BGP's
> +core functions is path hiding.
> +
> +It would likely be good to be even clearer, as in,
> +
> +NEW:
> +   The (E, C) route definition has the same characteristics as
> +   BGP-LU or BGP IP concerning whether redundant paths are
> +   available.
> ++--
>
>     BGP ADD-PATH [RFC7911] SHOULD be enabled for BGP CAR to signal
>     multiple next hops through a transport RR.
> @@ -817,7 +889,11 @@
>     (i.e., they belong to different color domains).  Low-delay in domain
>     2 is color C2, while it is C1 in domain 1 (C1 <> C2).
>
> -   The BGP CAR solution seamlessly supports this rare scenario while
> +   The BGP CAR solution seamlessly supports this scenario while
> ++--
> +jgs: I removed "rare", it appears to be unnecessary and invites the
> +distraction of asking what data you rely on in stating it to be rare.
> ++--
>     maintaining the separation and independence of the administrative
>     authority in different color domains.
>
> @@ -829,6 +905,12 @@
>        Color-Mapping-Extended-Community (LCM-EC) of color C2.
>
>     *  A is aware (as per classic peering agreement) of the intent-to-
> ++--
> +jgs: I think you need more and different words than "classic peering
> +agreement". Probably you mean something like, "(through some out-of-band
> +means not specified here, for example, determined during the business
> +negotiation of a peering agreement)"
> ++--
>        color mapping within domain 2 ("low-delay" in domain 2 is C2).
>
>     *  A maps C2 in LCM-EC to C1 and signals within domain 1 the received
> @@ -872,8 +954,12 @@
>
>     *  In the vast majority of the cases, the color of the NLRI is used
>        for resolution and steering
> ++--
> +jgs: Similar to my comment above, maybe drop the claim of "vast majority".
> +For example, "When colors are congruent, ..."
> ++---
>
> -   *  In the rare case of color incongruence, the local color encoded in
> +   *  In the case of color incongruence, the local color encoded in
>        LCM-EC takes precedence
>
>     Operational consideratons are in Section 11.  Further illustrations
> @@ -946,9 +1032,22 @@
>
>     A route (NLRI) can carry more than one non-key TLV (of different
>     types).
> ++--
> +jgs: Your bullet above calls the trailing data "non-key fields". But
> +the sentence directly above talks about TLVs. Do you intend that
> +all NLRI, both current and future, should use a uniform TLV system
> +for extra data? If so, the trailing data should be called "non-key
> +TLVs". On the other hand if you do intend complete freedom for each
> +variant NLRI, then the sentence above needs to be revised.
>
> +Also, if you intend a uniform TLV system (and that seems like a good
> +idea to me) then consider breaking out the TLV format description
> +into its own subsection, instead of redefining it per NLRI. I'll re-
> +flag this where relevant.
> ++--
>
>
> +
>  Rao, et al.              Expires 28 October 2024               [Page 17]
>
>  Internet-Draft        BGP Color-Aware Routing (CAR)           April 2024
> @@ -967,7 +1066,20 @@
>     in an opaque manner for handling of unknown or unsupported NLRI
>     types.  This can help deployed Route Reflectors (RR) to propagate
>     NLRI types introduced in the future in a transparent manner.
> ++--
> +jgs: The problem with the way you've specified this is that with NLRI
> +formats that can have unused bits, for example all the formats you
> +have in this document, where for example a /9 prefix has 7 unused bits,
> +the values of the unused bits are uninteresting to implementations that
> +understand the format, but are very interesting to implementations
> +treating the NLRI as opaque.
>
> +So I think if you want to deliver on the promise of opaque NLRI, you
> +have to state that all bits covered by the key length MUST be
> +specified, even if it's to say that unused bits must be zero. I'll flag
> +this again later.
> ++--
> +
>     The key length also helps error handling be more resilient and
>     minimally disruptive.  The use of key length in error handling is
>     described in Section 2.11.
> @@ -1021,7 +1133,12 @@
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>     Followed by optional TLVs encoded as below:
> ++--
> +jgs: as noted above, I suggest you specify TLV encoding once instead
> +of per NLRI type. Then you'd say "encoded as per Section foo"
> ++--
>
> +
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>     |     Type      |    Length     |    Value (variable)          //
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> @@ -1072,6 +1189,11 @@
>        -  The last octet has enough trailing bits to make the end of the
>           field fall on an octet boundary.  Note that the value of the
>           trailing bits is irrelevant.  The size of the field MUST be
> ++--
> +jgs: nope, see comment above. Probably go with "trailing bits MUST be set
> +to zero" although if you want to say something about how they need not be
> +inspected by the receiver you can.
> ++--
>           less than or equal to 4 for IPv4 (AFI=1) and less than or equal
>           to 16 for IPv6 (AFI=2).
>
> @@ -1216,7 +1338,7 @@
>     provides much better packing efficiency by carrying Label Index in
>     NLRI instead of in the BGP Prefix-SID Attribute (Appendix D).
>
> -   Label Index TLV MUST not be carried in the Prefix-SID attribute for
> +   Label Index TLV MUST NOT be carried in the Prefix-SID attribute for
>     BGP CAR routes.  If a speaker receives a CAR route with Label Index
>     TLV in the Prefix-SID attribute, it SHOULD ignore it.  The BGP
>     Prefix-SID Attribute SHOULD NOT be sent with the labeled color-aware
> @@ -1244,6 +1366,13 @@
>     connectivity using Segment Routing over IPv6 (SRv6) [RFC8402].
>     [RFC8986] specifies the SRv6 Endpoint behaviors (e.g.  End PSP) which
>     MAY be leveraged for BGP CAR with SRv6.  The SRv6 SID TLV is used for
> ++--
> +jgs: The use of the RFC 2119-style MAY implies that any other endpoint
> +behaviors that may be defined elsewhere MUST NOT be leveraged. I bet you
> +don't mean that. If you don't mean to be limiting, change to "may" or
> +better still, "can". If you do mean it to be limiting, make that clear
> +with a MUST NOT clause.
> ++--
>     advertisement of color-aware routes along with their SRv6 SIDs and
>     has the following format:
>
> @@ -1261,7 +1390,7 @@
>        multiple of 16.
>
>     *  SRv6 SID Information: field of size as indicated by the length
> -      that either carries the SRv6 SID(s) for the advertised color-aware
> +      that carries the SRv6 SID(s) for the advertised color-aware
>        route as one of the following:
>
>        -  A single 128-bit SRv6 SID or a stack of 128-bit SRv6 SIDs.
> @@ -1311,6 +1440,10 @@
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>     Followed by optional TLVs encoded as below:
> ++--
> +jgs: as noted above, I suggest you specify TLV encoding once instead
> +of per NLRI type. Then you'd say "encoded as per Section foo"
> ++--
>
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>     |R|T|   Type    |    Length     |    Value (variable)          //
> @@ -1362,6 +1495,9 @@
>        -  The last octet has enough trailing bits to make the end of the
>           field fall on an octet boundary.  Note that the value of the
>           trailing bits is irrelevant.  The size of the field MUST be
> ++--
> +jgs: Again, something like "trailing bits MUST be set to zero."
> ++--
>           less than or equal to 4 for IPv4 (AFI=1) and less than or equal
>           to 16 for IPv6 (AFI=2).
>
> @@ -1371,7 +1507,18 @@
>        IP Prefix NLRI type.
>
>  2.9.4.  Local-Color-Mapping (LCM) Extended-Community
> ++--
> +jgs: Your Color-Aware Route NLRI Type ("Type-1") encodes Color as
> +part of the NLRI... presumably to optimize for packing. But by
> +carrying this mapping thingy in a path attribute, you eliminate
> +any packing advantage you created, for any route that needs to
> +carry an LCM. Hopefully it's self-evident why I say this.
>
> +Why is it both so important to protect packing that you make color
> +a first-class part of the NLRI, but yet also so unimportant that
> +you abandon it when the route crosses an AS boundary?
> ++--
> +
>     This document defines a new BGP Extended-Community called "LCM".  The
>     LCM is a Transitive Opaque Extended-Community with the following
>     encoding:
> @@ -1418,7 +1565,32 @@
>     If two BGP paths for a route have different LCM values, it is
>     considered an error and the route is not considered for bestpath
>     selection.
> ++--
> +jgs: I think you mean "... routes for a prefix" or "... routes for a
> +destination". As a reminder, "route" has a well-defined meaning in BGP.
> +From RFC 4271:
>
> +   Route
> +      A unit of information that pairs a set of destinations with the
> +      attributes of a path to those destinations.  The set of
> +      destinations are systems whose IP addresses are contained in one
> +      IP address prefix carried in the Network Layer Reachability
> +      Information (NLRI) field of an UPDATE message.  The path is the
> +      information reported in the path attributes field of the same
> +      UPDATE message.
> +
> +That also implies it should be "... the destination is not considered".
> +
> +But apart from getting the terminology right, I am also concerned with
> +the dynamics this implies. If I understand you right, if I have route A
> +for destination D with LCM blue, that's fine and it's installed. If
> +route B for destination D with LCM green comes along, now neither route
> +can be considered and I have to uninstall route A. If route A is later
> +withdrawn, now I can install route B. Is that right? If so, why shouldn't
> +that be concerning to me? For that matter, does it open a new security
> +implication that should be reported in the Security Considerations?
> ++--
> +
>     If present, LCM-EC contains the intent of a BGP CAR route.  LCM-EC
>     Color is used instead of the Color in CAR route NLRI for procedures
>     described in earlier sections such as route validation (Section 2.4),
> @@ -1464,7 +1636,7 @@
>     in Section 2.8, and an example is given in Appendix B.3.
>
>     Both LCM-EC and BGP Color-EC may be present at the same time with a
> -   BGP CAR route.  For axample, a BGP CAR route (E, C1) from color
> +   BGP CAR route.  For example, a BGP CAR route (E, C1) from color
>     domain D1, with LCM-EC C2 in color domain D2, may also carry Color-EC
>     C3 and next hop N in a transit network domain within D2 where C2 is
>     being resolved via an available intra-domain intent C3 (See the
> @@ -1576,6 +1748,9 @@
>        considered invalid and not eligible for best path selection.
>        Treat-as-withdraw may be used, though it is recommended to keep
>        the NLRI for debugging purposes.
> ++--
> +jgs: Can you help me understand why the T bit is crucial here?
> ++--
>
>  3.  Service Route Automated Steering on Color-Aware Path
>
> @@ -1589,6 +1764,15 @@
>
>     An egress PE may request intent through the transport for service
>     routes using the BGP Color-EC [RFC9012].  An ingress PE steers
> ++--
> +jgs: The phrasing of the first sentence is ambiguous. This is because
> +of the overloading of the word "transport". Even ignoring the OSI Layer 4
> +meaning, this document uses it to mean *at least* two things, one being
> +a layer in the packet forwarding virtualization hierarchy, and the other
> +being the control plane. If you could rewrite the sentence without using
> +the word "transport" that would probably fix it, or anyway, please
> +help me understand what you mean.
> ++--
>     service traffic over a CAR Type-1 route using the service route's
>     next hop and BGP Color-EC.
>
> @@ -1597,6 +1781,10 @@
>     [RFC9256].  All the steering variations described in [RFC9256] are
>     applicable to BGP CAR color-aware path: on-demand steering, per-
>     destination, per-flow, CO-only.  For brevity, please refer to
> ++--
> +jgs: What's "CO-only"? You haven't defined "CO" anywhere, and it's also
> +not defined in RFC 9256.
> ++--
>     [RFC9256] Section 8.
>
>     Appendix A provides illustrations of service route automated steering
> @@ -1604,6 +1792,9 @@
>
>     An egress PE may request intent through the transport for service
>     routes by allocating the SRv6 Service SID from a routed intent-aware
> ++--
> +jgs: see my comment above.
> ++--
>     locator prefix (Section 3.3 of [RFC8986]).  Steering at an ingress PE
>     is via resolution of the Service SID over a CAR Type-2 IP Prefix
>     route.  Service Steering over BGP CAR SRv6 transport is described in
> @@ -1627,6 +1818,9 @@
>
>
>     RTC [RFC4684] may also be applied to the CAR SAFI, where Route Target
> ++--
> +jgs: please expand "RTC"
> ++--
>     ECs [RFC4360] can be used to constrain distribution of CAR routes.
>     RT assignment may be via user policy, for example an RT value can be
>     assigned to all routes of a specific color.
> @@ -1663,7 +1857,16 @@
>
>     On-demand subscription and filtering procedures are outside the scope
>     of this document.
> ++--
> +jgs: I'm bamboozled by this section. If it's out of scope, why have the
> +section at all?
>
> +On the other hand, if you do want to keep the section, it seems to be
> +not up to snuff -- it doesn't provide enough detail, by a long chalk.
> +So I'd say either remove the section (easy, quick) or let's have a
> +conversation about how to fix it (not easy, not quick).
> ++--
> +
>  5.  Scaling
>
>     This section analyses the key scale requirement of
> @@ -1750,7 +1953,10 @@
>
>     *  When a transport RR is used within the domain or across domains,
>        ADD-PATH is enabled to advertise paths from both egress BRs to
> -      it's clients.
> +      its clients.
> ++--
> +jgs: What's a "transport RR"? Define or reword.
> ++--
>
>     *  Egress PE E2 advertises a VPN route RD:V/v with BGP Color extended
>        community C1 that propagates via service RRs to ingress PE E1.
> @@ -2140,6 +2346,10 @@
>        most complex data-plane option for the ingress PE.
>
>  5.4.  Scaling Benefits of the (E, C) BGP Subscription and Filtering
> ++--
> +jgs: per my comments on Section 4.1, either remove this section, or
> +let's do the work to fix Section 4.1.
> ++--
>
>     The (E, C) subscription scheme from Section 4.1 provides the
>     following theoretical scaling benefits for the models in Section 5.2
> @@ -2262,6 +2472,9 @@
>        PEs.
>
>     *  An IP Prefix CAR route (Type-2) is defined to distribute SRv6
> ++--
> +jgs: Thank you for using a meaningful name here!
> ++--
>        locator prefixes and described in Section 2.9.3 and Section 8.
>
>     *  A BGP CAR advertised SRv6 locator prefix may also be used for
> @@ -2559,6 +2772,13 @@
>     route/NLRI.  When traversing network domain(s) where a different
>     intent/color is used for next-hop resolution, BGP Color-EC may
>     additionally be used as in Section 2.10.
> ++--
> +jgs: This reminder actually reminds me that I find the overlap (not to
> +say conflict) between LCM-EC and Color-EC to be confusing. For now I'm
> +suspending judgment so I can ship this review, but TBH I'm still trying
> +to make sense of it and hoping the appendices (not yet reviewed) will
> +help me.
> ++--
>
>     A special case of intent is best-effort which may be represented by a
>     color and follow above procedures.  But to be compatible with
> @@ -2676,7 +2896,10 @@
>     different customers being advertised by the PE.
>
>  9.1.  Format and Encoding
> -
> ++--
> +jgs: Is it true that in all cases RD has to be zero? If so, why
> +are we burning 8 bytes for sending constant zero?
> ++--
>     BGP VPN CAR SAFI leverages the BGP multi-protocol extensions
>     [RFC4760] and uses the MP_REACH_NLRI and MP_UNREACH_NLRI attributes
>     for route updates by using the SAFI value 84 along with AFI 1 for
> @@ -2721,7 +2944,12 @@
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>     Followed by optional TLVs encoded as below:
> ++--
> +jgs: as noted above, I suggest you specify TLV encoding once instead
> +of per NLRI type. Then you'd say "encoded as per Section foo"
> ++--
>
> +
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>     |R|T|  Type     |    Length     |    Value (variable)          //
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> @@ -2736,6 +2964,11 @@
>
>     *  Route Distinguisher: An 8-octet field encoded according to
>        [RFC4364].
> ++--
> +jgs: per my question above, if RD really is always zero in this
> +application, then state MBZ here. (But then I still want to know why
> +we even have this field.)
> ++--
>
>
>
> @@ -2766,7 +2999,12 @@
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>     Followed by optional TLVs encoded as below:
> ++--
> +jgs: as noted above, I suggest you specify TLV encoding once instead
> +of per NLRI type. Then you'd say "encoded as per Section foo"
> ++--
>
> +
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>     |R|T|   Type    |    Length     |    Value (variable)          //
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> @@ -2780,6 +3018,9 @@
>        the RD, Prefix Length field and IP Prefix field.
>
>     *  Route Distinguisher: 8 octet field encoded according to [RFC4364].
> ++--
> +jgs: Same question as earlier.
> ++--
>
>     *  Type-Specific Non-Key Fields: Encoded as per Type-Specific Non-Key
>        Fields of IP Prefix NLRI Type in Section 2.9.3.  Label TLV, Label
> @@ -2806,8 +3047,8 @@
>
>     IANA has assigned SAFI value 83 (BGP CAR) and SAFI value 84 (BGP VPN
>     CAR) from the "SAFI Values" sub-registry under the "Subsequent
> -   Address Family Identifiers (SAFI) Parameters" registry with this
> -   document as a reference.
> +   Address Family Identifiers (SAFI) Parameters" registry. IANA is
> requested
> +   to update the reference to this document.
>
>  10.2.  BGP CAR NLRI Types Registry
>
> @@ -2896,13 +3137,21 @@
>
>     *  Check IDR implementation reports for two implementation of new
>        non-Key TLV type.
> +
> ++--
> +jgs: For both of the above I suggest adding something like,
>
> +   *  Check that all bits of the key portion are defined, unused
> +      subfields MUST have required values (e.g. must be zero).
> ++--
> +
>  10.5.  BGP Extended-Community Registry
>
>     IANA has assigned the sub-type 0x1b for "Local Color Mapping (LCM)"
>     in the "Transitive Opaque Extended Community Sub-Types" registry
>     located in the "Border Gateway Protocol (BGP) Extended Communities"
> -   registry group.
> +   registry group. IANA is requested to update the referent to this
> +   document.
>
>
>
> @@ -2941,7 +3190,7 @@
>     conflicts in either network (Appendix A.7).
>
>     It should be noted that the color assignments coordination are only
> -   necessary for routes specific to the shared service IP.  Colors used
> +   necessary for routes specific to the shared service IP address.
> Colors used
>     for intra-domain or for inter-domain intents associated with unique
>     IP addresses do not need any coordination.
>
> @@ -2989,6 +3238,14 @@
>     routing information carried within a new SAFI, BGP origin validation
>     [RFC6811] and BGPsec [RFC8205] could be used as means to increase
>     assurance that the information has not been falsified.
> ++--
> +jgs: I don't think BGPsec does what you think it does. BGPsec signs the
> +AS path, period. It doesn't protect anything else carried within the
> +message. Similarly, origin validation only tells me that the origin AS
> +is the one that matches the prefix in the NLRI. Can you help me
> +understand how these would "mitigate any risk of manipulating the
> +routing information carried within a new SAFI"?
> ++--
>
>     Since CAR SAFI is a separate BGP SAFI that carries transport or
>     infrastructure routes for routers in the operator network, it
> @@ -3031,8 +3288,18 @@
>     Furthermore, as long as the filtering and appropriate configuration
>     mechanisms discussed above are applied diligently, risk of the
>     diversion of the traffic is eliminated.
> ++--
> +jgs: It's awfully daring to say a "risk ... is eliminated", especially
> when
> +the "elimination" is through the hard crunchy exterior, completely trusted
> +interior security model. I would suggest deleting the "furthermore"
> sentence
> +which is likely to draw fire and doesn't seem needed.
> ++--
>
>  13.  Co-authors
> ++--
> +jgs: As noted up top, please make this part of your Contributors section,
> +optionally inserting a statement such as I provided.
> ++--
>
>     Clarence Filsfils
>     Cisco Systems
> @@ -3113,6 +3380,16 @@
>
>     The authors would like to acknowledge the review and inputs from many
>     people.TBD
> ++--
> +jgs: There can't be any TBD in a document I take for IESG review (other
> than
> +placeholders for code point allocations). Please finish.
> +
> +BTW I noticed that the final paragraph of your Security Considerations is
> +almost verbatim identical to the final paragraph of
> draft-ietf-idr-bgp-ct-33.
> +I have no idea who cribbed from whom, or if the paragraph came from a
> third
> +party, but take this as a suggestion to consider acknowledging sources of
> +things like that. (I'll also make this suggestion to the authors of
> bgp-ct.)
> ++--
>
>  16.  References
>
>
>
>
>