Segment Routing Prefix Segment Identifier Extensions for BGP
draft-ietf-idr-bgp-prefix-sid-27

Thanks for Acee for addressing my DISCUSS - cleared.

Major(ish) issue:
Section 5.  Advertising BGP Prefix-SID Attribute
   "The BGP Prefix-SID attribute MAY be attached to labeled BGP prefixes
   (IPv4/IPv6) [RFC8277] or to IPv6 unicast prefixes [RFC4760].  In
   order to prevent distribution of the BGP Prefix-SID attribute beyond
   its intended scope of applicability, attribute filtering SHOULD be
   deployed."
Thank you for including this -- however, as I'm sure you know, the state of BGP filtering in the wild is, er, poor. Can you please provide some additional guidance? For example, could you include an appendix with examples? Or simply a bit more text on determining the scope of applicability? Apologies if I missed this, but should this by default be filtered on eBGP sessions? The included text is a great start, but some more (so people don't miss it and shoot themselves in the foot would be much appreciated.

Nits:
Section 1:
" the SID consists of a label while when SR is applied to the IPv6 dataplane the SID consists of an IPv6 address."
I think that "of a label, while" would make this much more readable. It took me a few tries without it.

Section 4:
" A BGP speaker receiving a BGP Prefix-SID attribute from an EBGP
   neighbor residing outside the boundaries of the SR domain, MUST
   discard the attribute unless it is configured to accept the attribute
   from the EBGP neighbor."
Spurious comma between domain and MUST (yeah, I want a comma above, I don't want one here, no pleasing some people. They are nits :-))

Please expand "AFI/SAFI" on first use.

I tend to agree with Mirja that you either don't currently need registry for flags or need it with a stricter policy than First Come First Served.

Nits:

4.  Receiving BGP Prefix-SID Attribute

   A BGP speaker receiving a BGP Prefix-SID attribute from an EBGP
   neighbor residing outside the boundaries of the SR domain MUST
   discard the attribute unless it is configured to accept the attribute
   from the EBGP neighbor.  A BGP speaker MAY log an error for further
   analysis when discarding an attribute.

Is there any reason why logging is just MAY? This would help big time in debugging issues.
You are also using "SHOULD log" in other places in the document. So I suggest:

MAY log an error ==> SHOULD log an error

(Actually "MAY log" appears twice in the document)


I suggest you use a more common "extensibility" instead of "extendibility", but maybe this is just me :-).

Is there a different, less controversial example that could be given in the first paragraph of the document than "pass through deep packet inspection"?

(The document has been updated since I made my review notes. Apologies if any of my comments no longer apply.)

Substantive:

- 1: I agree with Alissa's comment about finding a less controversial example than DPI.

-9: last paragraph: Can you offer a citation for the "standard BGP mechansism (filters)"? I assume they are in one of the documents cited in the first paragraph of this section, but it would help to cite the specific document and section.

Editorial and nits:

- Abstract and section 1: Both are missing an article (probably "The")  before "Segment Routing (SR) architecture"

   BGP Prefix-SID attribute, it MUST program the derived label as the
   local label for the prefix in its MPLS dataplane.  In case of any
   error, a BGP speaker MUST resort to the error handling rules
This seems over-strong, as the next paragraph suggests that you might reject it due to policy.


   This document defines a new BGP attribute in order to address the use
   case described in [I-D.ietf-spring-segment-routing-msdc].  It i
   assumed that the new attribute (BGP Prefix-SID) advertisement is
Nit: "is assumed", i think?


   inherits the security considerations expressed in: [RFC4271] and
   [RFC3107].
This section seems like it needs to discuss how the security of this mechanism interacts with BGPSEC, etc. Maybe it's just "the same as everything else" but perhaps not...

I support Warren's discuss and Alexey's comment on logging.

Minor commets:

1) Maybe spell out NLRI on first occurrence (sec 2).

2) Given there are currently no flags defined, wouldn't it be sufficient to use the 8-bit reserved field for flags? Also do you really want to create an empty registry on a first-come first-serve basis for the flags now? Have there been any use cases discussed already? Is it likely that assignments will be made soon and is first-com first-serve the right policy for these expected assignments? Note that such registries can also be created at a later point when really needed.

I don't see how the MUST in this receiving behavior in Section 4.2.

"However, a BGP Prefix-SID attribute corresponding to the BGP IPv6 address family without an IPv6 SID TLV MUST be ignored."

makes sense with the MAY in the sending behavior 

"A BGP speaker that originates an IPv6 prefix with the BGP Prefix-SID attribute MAY include the IPv6 SID TLV."

Can you please clarify?