Re: [spring] AD Review of draft-ietf-spring-segment-routing-central-epe-06

["Gaurav Dawra (gdawra)" <[e-mail]>]

Hey Alvaro,

A new version of the draft has been published addressing your comments. Please see an update for rest of the comments inline…<Gaurav1>.

Thanks again so much for your review.

Regards,

Gaurav

From: Gaurav Dawra <[e-mail]>
Date: Thursday, October 12, 2017 at 5:57 PM
To: "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>
Cc: Bruno Decraene <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, Gaurav Dawra <[e-mail]>
Subject: Re: [spring] AD Review of draft-ietf-spring-segment-routing-central-epe-06

Hey Alavaro,

Thank you so much for the review.

Pls see inline..<Gaurav>

From: spring <[e-mail]> on behalf of "Alvaro Retana (aretana)" <[e-mail]>
Date: Wednesday, August 30, 2017 at 9:35 PM
To: "[e-mail]" <[e-mail]>
Cc: Bruno Decraene <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>
Subject: [spring] AD Review of draft-ietf-spring-segment-routing-central-epe-06

Dear authors:

I just finished reading this document.  I have a couple of Major concerns (see below) which I would like to see addressed before starting the IETF Last Call on this document.

Thanks!!

Alvaro.



Major:

M1. This document mentions in several places that the segment routing information can be programmed at hosts (or “content source”).  As we all know, significant concerns exist in the community about using source routing all the way at the host (even if, as in this case, we’re talking about a centralized programing).  The Architecture document doesn’t explicitly eliminate hosts from an SR domain (the definition is “nodes participating into the source routing model”), but it also doesn’t explicitly include them…but the text can be interpreted as excluding (for example: “the explicit routing information MUST NOT be leaked through the boundaries of the administered domain”, or “Filtering MUST be performed on the forwarding plane at the boundaries of the SR domain”, etc.).  There is nothing specific that tells me that this case (EPE) is different from any other SR application – if hosts are to be explicitly considered part of a domain then that should be explicitly described in the Architecture document.  In short, please take references to hosts out of this document (unless you decide to add a discussion about them in the Architecture document).
 <Gaurav> We are in the process of discussing this point with the authors of the draft, we will get back to you on this one.
<Gaurav1> We have clarified the “Host” in the Segment Routing Architecture document.


M2. The requirements in Section 1.1. (Problem Statement) make non-explicit use of normative language; most of the requirements are non-technical and aspirational in nature.  While I think that the normative language is not used as intended in rfc2119 (“MUST only be used where it is actually required for interoperation or to limit behavior which has potential for causing harm”), I think it is ok in this case to express some requirements.  I would, however, prefer if their use las limited.  Nevertheless, here are some suggestions/questions/comments:

M2.1. What does “MUST NOT make any assumption” mean?
OLD>
     The solution MUST NOT make any assumption on the currently
      deployed iBGP schemes (RRs, confederations or iBGP full meshes)
      and MUST be able to support all of them.

NEW>
      The solution MUST support any deployed iBGP schemes
      (RRs, confederations or iBGP full meshes).
<Gaurav> ACK. Will update in Next version.

M.2.2. Two MUSTs doesn’t make the text better.
OLD>
     The solution MUST be applicable to any type of EPE router.  While
      "Egress Peer Engineering" refers to "External" peering, the
      solution MUST also be applicable to a router having internal
      peers.

NEW>
      The solution MUST be applicable to both routers with external
      and internal peers.
<Gaurav> ACK. Will be addressed in the next update.

M2.3. “The solution SHOULD minimize the need for new BGP capabilities at the ingress PEs.”  What is the explicit requirement, that needs the “SHOULD”, from an interoperability point of view?
<Gaurav> At Ingress PE, this requirement covers that there is need for some minimal configuration or protocol extension for Egress Engineering.

M2.4. “The solution MUST accommodate an ingress BGP-EPE policy at an ingress PE or directly at a source host within the domain.”  “MUST accommodate”??  What are you looking for?  The ability to program at those points?  The ability to instantiate any policy?
<Gaurav> Solution MUST cover the ability to accommodate instantiation and programming of the BGP-EPE policy at Ingress.

The Introduction says that “The exhaustive definition of all the means to program an BGP-EPE input policy is outside the scope of this document.”, so mandating something that is out of scope seems like a contradiction.
<Gaurav> The method to signal or program is out of the scope of this document. However, this document does covers the need to accommodate and program BGP-EPE policy at the Ingress

M2.5. “The solution MUST support automated Fast Reroute (FRR) and fast convergence mechanisms.”  But then section 3.6. (Fast Reroute (FRR)) says that FRR is optional.
<Gaurav>  ACK. This will be updated in next version.

M3. The references to I-D.ietf-idr-bgpls-segment-routing-epe, I-D.ietf-spring-segment-routing and RFC7752 should be Normative.
<Gaurav> ACK. Will be addressed in the next update.

Minor:

P1. As in all the related documents, please take “service chain” out to avoid confusion.
<Gaurav> ACK. Will be addressed in the next update.

P2. The examples in Sections 3.x seem incomplete and inaccurate to me.  Also, the names used don’t match what is specified in draft-ietf-idr-bgpls-segment-routing-epe.  In general, please be consistent with the names!  For example:

Section 3.1. (PeerNode SID to D):
“
   Descriptors:

   o  Node Descriptors (BGP router-ID, ASN): 192.0.2.3, AS1

   o  Peer Descriptors (peer BGP router-ID, peer ASN): 192.0.2.4, AS2

   o  Link Descriptors (IP interface address, neighbor IP address):
      2001:db8:cd::c, 2001:db8:cd::d

   Attributes:

   o  PeerNode SID: 1012
“

Comments>
- Section 5.1 in draft-ietf-idr-bgpls-segment-routing-epe uses “Local Node Descriptor” instead of simply “Node Descriptor”, and the BGP-LS ID is missing above.
- s/Peer Descriptors/Remote Node Descriptor
- The Link Descriptor uses the terms “IPv6 Interface Address” and “IPv6 Neighbor Address”…
- s/Attributes/Link Attribute
 <Gaurav> ACK> Will compare and address in next update.

P3. Section 3.6. (Fast Reroute (FRR)): “A BGP-EPE enabled border router MAY allocate a FRR backup entry on a per BGP Peering SID basis (assuming inter-AS agreement on the FRR strategy/policy).”  Why is an “inter-AS agreement” needed?  FRR is a local decision, and, assuming that the border router is at the edge of the SR domain…why would the next AS need to agree?  Am I missing something?
<Gaurav> ACK. Will be addressed in the next update.


P4. References:
- Please add a reference for BMP and IPFIX.
- Put the reference to BGP-LS on first mention (and not just way down in Section 9).
- Replace the reference to RFC3107 with draft-ietf-mpls-rfc3107bis – and it can be made Informative.
- The reference to RFC6241 should be Informative.
<Gaurav> ACK. Will be addressed in the next update.

P5. From Section 9. (Manageability Considerations): “…the advertisement of EPE information MUST conform to standard BGP advertisement and propagation rules (iBGP, eBGP, Route-Reflectors, Confederations).”  What does this text mean?  As far as I can tell, there’s no change to BGP to be able to instantiate EPE…
 <Gaurav> ACK Will address in next update.

Nits:

N1. The second paragraph in the Abstract seems unnecessary.
<Gaurav> ACK

N2. Please avoid using “we”.
<Gaurav> ACK

N3. Section 5.2 seems to introduce this new notation: “IP route L/8 set next-hop T1”… please explain.  L/8 is “hidden” in Figure 1, and not obvious since it looks like an IPv4 prefix, but the examples are all IPv6.
<Gaurav> Similar to 5.3. In this case, NH instead is a tunnel T1, where T1 is configured with Labels as {64, 1042}. "64" being the SID of PE C.