Early Review of draft-ietf-idr-bgpls-segment-routing-epe-11
review-ietf-idr-bgpls-segment-routing-epe-11-rtgdir-early-singh-2017-04-16-00

Request Review of draft-ietf-idr-bgpls-segment-routing-epe
Requested rev. no specific revision (document currently at 19)
Type Early Review
Team Routing Area Directorate (rtgdir)
Deadline 2017-03-02
Requested 2017-02-15
Requested by Susan Hares
Draft last updated 2017-04-16
Completed reviews Rtgdir Early review of -11 by Ravi Singh (diff)
Secdir Early review of -17 by Carl Wallace (diff)
Opsdir Early review of -17 by Sheng Jiang (diff)
Genart Last Call review of -18 by Joel Halpern (diff)
Comments
Starting WG LC.

Assignments

Review
review-ietf-idr-bgpls-segment-routing-epe-11-rtgdir-early-singh-2017-04-16

Hi
I had been designated as the RTG-DIR reviewer for this draft.

Overall the draft is clear.
However, it could be use some editorial revision for improved readability.

Specific comments listed below:

1.       Document title could be less heavy on consecutive adjectives: some suggestions:
a.       BGP-LS extensions for Segment Routing BGP Egress Peer Engineering
b.      BGP-LS extensions for "BGP-EPE using SR"
c.       Segment Routing BGP Egress Peer Engineering extensions for BGP-LS
...
2.       Section 1:
"   This document defines new types of segments: a Peer Node segment
   describing the BGP session between two nodes; a Peer Adjacency
   Segment describing the link (one or more) that is used by the BGP
   session; the Peer Set Segment describing an arbitrary set of sessions
   or links between the local BGP node and its peers."
Above has an unintended meaning. This should instead have stated that this doc defines BGP-LS extensions to communicate the above listed SIDs that are defined in "draft-ietf-spring-segment-routing"

3.       Section 3:
"
   This document defines the BGP-EPE Peering Segments:

   o  Peer Node Segment (Peer-Node-SID)

   o  Peer Adjacency Segment (Peer-Adj-SID)

  o  Peer Set Segment (Peer-Set-SID)"

Same issue as listed above for section 1 above.
Section 5 has the same issue.

4.       Section 4.2: I'd suggest that this be broken into 2 separate sub-sections: each listing the mandatory/optional sub-TLVs for the local and remote node descriptors respectively. That will make for improved readability.

5.       Section 4.2: please clarify in text as to Why no "BGP-LS ID" in link NLRI in the remote node descriptor.

6.       Section 4.3:
a.       values of the flags are specified only for the Per-Adj-SID. Explicit text should be listed for per-node-SID and per-set-SID.
b.      "
   The Peer-Node-SID MUST be present when BGP-LS is used for the use
   case described in [I-D.ietf-spring-segment-routing-central-epe] and
   MAY be omitted for other use cases."
Is there really a need to state what other use-cases might do, considering that this doc is specific to BGP-LS for SR-EPE? Perhaps can be reworded. Similarly for "Peer-Adj-SID and Peer-Set-SID SubTLVs MAY" in next paragraph.

7.       There is some repetition of info between sections 4 & 5. eg. What local and remote node descriptors contain. Please reword these sections to avoid the repetition while still presenting the info.

8.       Sections 5.1 & 5.2: have lots of repetitive text. Tabular presentation of the text in these sections would allow describing per-node-sid and per-adj-sid without the repetition.

9.       Section 9: typo in language in first sentence.

10.   Section 10: please explicitly call out any additional (beyond rfc7752) security considerations or include text stating that none exist.

11.   Typos
a.        Section 3:
                                       i.            "an BGP-EPE" -> "a BGP-EPE"
b.      Section 4:
                                       i.            "Link-type" NLRI -> "link-state" NLRI?

Regards
Ravi