Hi Sue, thanks for the review. Some comments below. > On Mar 6, 2017, at 5:25 PM, Susan Hares <shares@xxxxxxxx> wrote: > > Reviewer: Susan Hares > Review result: Has Issues > > The RTG-DIR has the categories: minor concerns or major concerns > regarding "issues", I wil differentiate my issues by this quality. > I also have editorial nits regardign under specified text. > > Major concerns: > 1) The security section is not sufficient for any review by the > Security area > > This draft depends on IDR WG draft (ietf-idr-bgp-prefix-sid) that > defines the BGP Segment attribute. If this attribute is used with > IPv6, this simply gives more infromation about a link to a next. > However, the combination of this information with the information > passed using draft-ietf-idr-bgpls-segment-routing-epe-09 that utilizes > BGP to pass BGP topologies in BGP - requires a better security > section. BGP-LS was described to be an "information gathering" > function handled by a few routers on the edge of the network to obtain > link-state topology information. The BGP peers would carry this > information in a separate informational stream. With this constraint, > it was approved by the IESG. well, we have now different models that have been deployed and assuming that bgp-ls uses a separate stream is not accurate if we look what’s in the industry. However, I take your point and I agree that more text in the security section is required in order to emphasize that the model the draft addresses is internal (DC and interconnected DC over a same-administration network). > draft-ietf-idr-bgpls-segment-routing-epe expands the initial concept > of BGP-LS from "information gathering" to a full-routing scheme of BGP > within BGP for data centers and for data center interconnection to the > network. EPE defines a model where the topology of the peering point (not the network, just the peering point) is advertised to an internal server. > This extension takes it out of the approved range of the > BGP-LS. I don’t know what is the “approved range”. To me, bgp-ls carries topology information. We started with lsdb, then extended to mpls-lsp's, ip tunnels, peering points, and more will come. The security of bgp-ls doesn’t change. It’s the boundary of the network where bgp-ls is applied that matters. > Therefore, the security sections in both the IDR WG drafts > and this draft need to describe the new threat scenarios and describe > threat mitigation strategies for deployments. I will add more text about the information originated by bgp-ls (or the bgp prefix sid) and how it is intended to be consumed internally to a domain. > In addition, the information by BGP-LS > (draft-ietf-idr-bgpls-segment-routing-epe) or in draft-ietf-bgp-sid > may have privacy issues - so these need to be described the security > section. same here. I will emphasize the deployment model and the security boundaries. > 2) through-out the text you use words such as "ebgp3107" or BGP 3107 > updates" > > This phrase is inaccurate. The base RFC3107 support will not provide > BGP-Prefix support (as supported in bgp-idr-bgp-prefix. Some texts > goes on to clarify the addition of the BGP SID Prefix attribute. It > would be better to invent a new phrase or term. I’ll check this out. > In section 8.1, the authors state: > "The Prefix Segement is a lightweight extension to the BGP Labelled > Unicast". As noted in my #1 major concern, this "hand-waving" > description either needs to be refined to be accurate. If the MPLS > usage only uses the BGP-Prefix label and does not extend to the > Egress, it is simplier. the BGP Prefix SID Attribute is just an extension to a 3107 update. > However, it is not clear that is what section > 8.1 is about. If 8.1 includes the > draft-ietf-idr-bgpls-segment-routing-epe, then BGP-LS addition does > have a number of prefixes and rules. The trade-off between BGP-LS + > BGP-LS SID (draft-ietf-idr-bgp-sid) handling + BGP LS egress peer > engineering draft (draft-ietf-idr-bgp-segment-routing-epe) and a > signalling protocol is more complex than the hand-wave. Not sure I understand your point but to me the statement: “The Prefix Segement is a lightweight extension to the BGP Labelled Unicast” is correct because the prefix-sid is really just a new attribute. Here we’re just talking protocol extension. The interaction and combination between prefix-sid and epe is a deployment and operational model that (we agreed above) requires more explanation in terms of security. > It may be the > right choice based on current implementations and management issues, > but these need to be laid specifically. > > 3) Why are you defining 2-byte Private Use AS when there are plenty of > 4-Byte Private Use AS (p. 5). well, we just want to be sure we address the worse case where you only have 2 octets. > > This usage increases the confusion regarding 2-byte/4-byte ASN. IDR > specifically worked on 4 byte ASN. yes but in order to be aligned with 7938 we also take into account 2 octet ASN and the re-usability of these numbers. > Minor concerns > 1) It is not clear what happens in section 4.2.2 and figure 3-5 > > What happens if the traffic goes to node 3 instead of node 4 on the > ECMP path? > What happens if the traffic goes to node 8 instead of node 7 on the > ECMP Path? > > Is there something missing in the stroy? nope. This is plain segment routing. As explained in the draft, assuming that you use the same SRGB (as recommended) a node is known through the same sid value all along the network so ecmp becomes trivial. > 2) section 4.3 - IBGP Labeled Unicast. > > The phrase "iBGP3107 reflection with nhop-self" needs to be explicitly > spelled out as IBGP Route-Reflection with next-hop self. ok > If that is > not what the authors mean, then it needs to be further spelled out. > It is unclear where the central IBGP nodes are that share fully the > information learned from the three clusters. (nodes:5-8 cluster 1, > nodes 3-4 cluster 2, nodes 9-10 cluster 3). > > This section has hints of a solution, but it is miss a great deal. > Please upgrade to specific solution. A diagram might help. yes. I’ll check this out. In short (you certainly figured it out) it’s about using iBGP session where each node acts as a RR so to propagate to other iBGP peers (this is what "iBGP reflection” refers to). I agree, it’s probably a bit too cryptic. > 3) Load Sharing hints (Section 7.1) > > Elephant flow and mice flows are good descriptions. Flowlets and VL2 > should either warrant a 1 sentence explanation that actually describes > these features in a 22 page draft, or be removed. We have a reference for them but will add more text. > 4) The lack of a manageability or operations section (TBD in version > -02) - concerns me. The operational issues may be well known to the > data centers and devices manufacturers who have implement this > specification, but this is an interoperability specification for IETF. > Some manageabilty comments should be included or a BCP pointed to. ack. > Editorial issues: I’ll go through all below and update the draft asap. Thanks. s. > > #1 - The following 4 abbrevitions need to be initially expanded when > first used: CLOs (p.3), SRGB(p.6), flowlets (p. 14), and VL2 (p. > 14). > > #2 - page 7, section 4.2 last paragraph > Old/: assuming the IP Addresses, AS and label-index allocation > previously described, the" > New/: assuming the IP address with the AS and label-index allocation > previously described, the" > [Comma is optional] > > #3 - page 14, section 7.1 paragraph 4, /(e.g. spine switch Node1)/ - > by the diagram it should be node 5-8 or an error. Please check the > number > > #4 - page 17, section 8.2 paragraph 2. > > Old/ > This is easily accomplished by encapsulating the trafffic either > directly at the host or the source ToR node by pushing the BGP- > Prefix-SID of the destination ToR for intra-DC traffic, or border > node for inter-DC or DC-to-outside-world traffic./ > > New/ > This is easily accomplished by encapsulating the trafffic either > directly at the host or the source ToR node by pushing the BGP- > Prefix-SID of the destination ToR for intra-DC traffic, or > the BGP-Prefix-SID for the the border node for inter-DC or > DC-to-outside-world traffic./ > > If this is not the correct logic, then you can reword this further. > I read it 4 or 5 times. > > #5 - Adding a diagram to section 4.3 might help your description. > > > _______________________________________________ > spring mailing list > spring@xxxxxxxx > https://www.ietf.org/mailman/listinfo/spring