Early Review of draft-ietf-idr-bgp-prefix-sid-04
review-ietf-idr-bgp-prefix-sid-04-rtgdir-early-decraene-2017-06-16-00

Request Review of draft-ietf-idr-bgp-prefix-sid
Requested rev. no specific revision (document currently at 27)
Type Early Review
Team Routing Area Directorate (rtgdir)
Deadline 2017-06-28
Requested 2017-06-14
Requested by Susan Hares
Authors Stefano Previdi, Clarence Filsfils, Acee Lindem, Arjun Sreekantiah, Hannes Gredler
Draft last updated 2017-06-16
Completed reviews Rtgdir Early review of -04 by Christian Hopps (diff)
Rtgdir Early review of -04 by Bruno Decraene (diff)
Secdir Early review of -13 by Brian Weis (diff)
Rtgdir Telechat review of -11 by Tony Przygienda (diff)
Genart Last Call review of -10 by Peter Yee (diff)
Genart Telechat review of -11 by Peter Yee (diff)
Rtgdir Last Call review of -21 by Bruno Decraene (diff)
Genart Last Call review of -21 by Peter Yee (diff)
Assignment Reviewer Bruno Decraene
State Completed
Review review-ietf-idr-bgp-prefix-sid-04-rtgdir-early-decraene-2017-06-16
Reviewed rev. 04 (document currently at 27)
Review result Has Issues
Review completed: 2017-06-16

Review
review-ietf-idr-bgp-prefix-sid-04-rtgdir-early-decraene-2017-06-16

I've read the document (-04) and have the following comments.

-----
§2
'A BGP-Prefix-SID is always global within the SR/BGP domain'

Do you imply that this extension may only be used within one SR/BGP domain?
IOW all involved ASes must coordinate their BGP SID allocation?

This would fit the MSDC use case discussed in draft-ietf-spring-segment-routing-msdc, but may be a restriction for other uses cases (e.g. Inter AS VPN option C requiring an inter-AS MPLS LSP).
Could the document clarify/elaborate on this?
One option may be for the ASBR to be configured to shit the received index by an offset (e.g. +1000). (this possibility could be referred to in section 6.1)
--
§2
'A BGP-Prefix-SID is always global within the SR/BGP domain'

On a side note,
- on the BGP side, IMHO, a BGP domain probably means an AS (looking back up to RFC 1771 I'm not seeing the definition of a BGP domain)
- on the SR side, I think you mean that all ASes in the DC are in the same SR domain

Hence SR domain does not equal a BGP domain.
-----

§2: "the newly proposed BGP Prefix-SID attribute can be attached to prefixes from AFI/SAFI: »

§3: "The BGP-Prefix-SID attached to a BGP prefix P"

§3.1: "this TLV can be used to advertise the label index of a given prefix"



IINM, BGP Attributes are not attached to (IP) prefixes but shared by all prefixes of a given BGP UPDATE.
In addition to the terminology point, this has consequences as:
- [RFC3107] and [RFC4760] allow the advertisement of multiple IP prefixes per BGP UPDATE
- draft-ietf-idr-bgp-prefix-sid would add the new requirement that each IP prefix be advertised in a dedicated BGP UPDATE. (unless multiple Label-Index TLB must be advertised)

If this extension requires the use of one BGP update message per IP prefix, I think this point should be highlighted and discussed in both draft-ietf-idr-bgp-prefix-sid and draft-ietf-spring-segment-routing-msdc

---
§2
"the newly proposed BGP Prefix-SID attribute can be attached to prefixes from AFI/SAFI:
      Multiprotocol BGP labeled IPv4/IPv6 Unicast ([RFC3107<https://tools.ietf.org/html/rfc3107>;]).
      Multiprotocol BGP ([RFC4760<https://tools.ietf.org/html/rfc4760>;]) unlabeled IPv6 Unicast."

If this attribute is found on routes of different AFI/SAFI, how is this handled? Is this considered an error (hence attribute is discarded as per §7)
If so, this does not seem to work in Carrier's carrier VPN case (where the CE use AFI/SAFI 1/4 routes, may attach the Prefix-SID attribute; then the PE will transform this route into a AFI/SAFI 1/128 with the same Prefix-SID attribute attached).
If not, it must be clear that the BGP-Prefix-SID Index must not be used on the VPN route.

Can this also be clarified in section 7 (error handling)?

---

§4

" The BGP Prefix SID attribute is an optional, transitive BGP path attribute. »



Hence this attribute may spread very far, e.g. received over Internet routes.

IPv6 Internet routes may be converted in AFI/SAFI 2/4 routes (labeled IPv6), in particular for AS running 6PE (RFC 4798). Hence this AS would interpret the Prefix SID and the received index would likely (or even deliberately) collision with Index provisioned by this AS.

I think the use of a transitive attribute is debatable given that this attribute is not required for the service/network to work. i.e. it's only "nice to have" in order for each node to use the same label (and assuming they can be configured with the same SRGB). So the document is trading security risk for a nice to have feature.



---

§5.1

"   A BGP Prefix-SID attribute is called "unacceptable" for a speaker M

   if the derived label value L lies outside the SRGB configured on M.

   Otherwise the Label Index attribute is called "acceptable" to speaker  M. »

The case where two different IP Prefixes are received with the same Index (but with different labels) does not seem to be correctly handled. As per current text, both Prefix would incorrectly be merged into the same FEC (incoming label).
Also, the index in the BGP Prefix-SID may collision with the same index, received from a different protocol (e.g. IS-IS) with a different prefix. How is this handled?
This seems specifically an issue when combined with the previous point (comment).
---
§5.2 (IPv6 SID)


"      *  S flag: if set then it means that the BGP speaker attaching the

         Prefix-SID Attribute to a prefix is capable of processing the

         IPv6 Segment Routing Header (SRH,

         [I-D.ietf-6man-segment-routing-header<https://tools.ietf.org/html/draft-ietf-idr-bgp-prefix-sid-04#ref-I-D.ietf-6man-segment-routing-header>;]) for the segment

         corresponding to the originated IPv6 prefix"

A similar text has just been removed in the latest version of the SR ISIS extension published yesterday (draft-ietf-isis-segment-routing-extensions-11). (cf "H-Flag").
Hence, are you sure that this SRv6 specific part is still applicable/up to date?


---
"9.  Security Considerations

   This document introduces no new security considerations above and beyond those already specified in [RFC4271] and [RFC3107]."

Some of the comments above may probably have security considerations.

---
Nits:
§4.1

OLD: None are defined at this stage of the document.

NEW: None are defined by this document.
--
§2

OLD: the newly proposed BGP Prefix-SID attribute

NEW: the BGP Prefix-SID attribute defined in this document

--

"11.  Change Log



   Initial Version:  Sep 21 2014"



This section does not seem that useful and could either be removed or completed with the change log of all versions.

--
Name of the attribute is not consistently spelled (e.g. BGP-Prefix-SID, BGP Prefix SID, BGP Prefix-SID)

Thanks
Regards,
--Bruno