Telechat Review of draft-ietf-idr-bgp-prefix-sid-11
review-ietf-idr-bgp-prefix-sid-11-rtgdir-telechat-przygienda-2018-02-02-00

Request Review of draft-ietf-idr-bgp-prefix-sid
Requested rev. no specific revision (document currently at 27)
Type Telechat Review
Team Routing Area Directorate (rtgdir)
Deadline 2018-02-06
Requested 2018-01-12
Requested by Alvaro Retana
Draft last updated 2018-02-02
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 Tony Przygienda
State Completed
Review review-ietf-idr-bgp-prefix-sid-11-rtgdir-telechat-przygienda-2018-02-02
Reviewed rev. 11 (document currently at 27)
Review result Has Nits
Review completed: 2018-02-02

Review
review-ietf-idr-bgp-prefix-sid-11-rtgdir-telechat-przygienda-2018-02-02

I have been selected to do a routing directorate “early” review of draft-ietf-idr-bgp-prefix-sid  draft.

Document: draft-ietf-idr-bgp-prefix-sid
Reviewer: Tony Przygienda
Review Date: 1/30/18
Intended Status: Standards Track

Summary:
Choose from this list...

I have some comments about this document that I think should be resolved before it is submitted to the IESG.

Comments, mostly clarification and readability improvement

1. The introduction would make the document significantly more readable by pointing out that SPRING SID can only be carried over RFC8277 and SRv6 SID only over RFC4760. Yes, it is described later while the elements are described but makes the reading of the draft hard without this information upfront. 
2. The sentence “The BGP Prefix-SID attached to a BGP prefix P represents the instruction "go to Prefix P" along its BGP best path (potentially ECMP-enabled)” is confusing and would need rewriting IMO. It is the prefix that advises to go to the prefix by lookup and forwarding obviously (if one would use such language) and not the “prefix sid attached to the prefix”.  Maybe something like “BGP Prefix SID present in the segment list should trigger the action of …. and with that ultimately forward the packet to the prefix the SID is associated with along the best ….”
3. The document should explain whether 
a. It is valid to receive Originator SRGB TLV without Label Index TLV being present 
b. It is valid to receive both SRv6 and SPRING SID for the same prefix 
c. It is valid to receive SRGB TLV while having obtained it by other topology distribution mechanism and which one has preference
4. The document could explain for readability that it is assumed that multiple ranges in SRGB represent logically speaking really a single range to which the index is applied and not multiple ranges of which one is chosen (yes, it could go a long way to not call everything here range). Or reference an RFC where such a thing is clearly explained. 
5. Those two sentences seem to contradict each other directly. “The mechanisms through which a given label index value is assigned to a given prefix are outside the scope of this document.  The label-index value associated with a prefix is locally configured at the BGP node originating the prefix.”
6. Not English (important for next point)
a. “The label index gives the information to the receiving node on which local/incoming label the BGP speaker SHOULD assign.”
7. The unparsable sentence in point 6. Makes understanding the forwarding behavior difficult. So is the normal MPLS label used and when and for what (except when SID cannot be derived as described)?  If not, why is the null label signaling needed? In fact what does that mean “Specifically, a BGP speaker receiving a prefix with a BGP Prefix-SID attribute and a label NLRI field of Implicit NULL from a neighbor MUST adhere to standard behavior and program its MPLS dataplane to pop the top label when forwarding traffic to the prefix.  The label NLRI defines the outbound label that MUST be used by the receiving node.”. If we got a NULL and strip what’s there left and what label do we use as the “The label NLRI defines the outbound label” if the label NLRI is implicit NULL. Am I confused?  A small set of examples would go a long way.