Hi Alvaro, Martin, Bruno, Sorry for the long delay.
The authors are working on the different reviews and will address all comments before next meeting. Thanks. s. On October 10, 2017 11:35:24 PM GMT+02:00, Alvaro Retana <[email protected]> wrote: >Dear authors: > > > >Hi! > > > >It’s been 2 months since I sent out this review and I still haven’t >heard anything back (not even an ACK!) from you. What is the plan to >address the comments and move this work forward? > > > >Thanks! > > > >Alvaro. > > > >From: spring <[email protected]> on behalf of Alvaro Retana ><[email protected]> >Date: Thursday, August 10, 2017 at 3:00 PM >To: "[email protected]" ><[email protected]> >Cc: "[email protected]" <[email protected]>, >"[email protected]" <[email protected]>, Martin Vigoureux ><[email protected]> >Subject: [spring] AD Review of draft-ietf-spring-segment-routing-12 > > > >Dear authors: > > > >I just finished reviewing this document – sorry for the delay in >processing. Thanks for all the work you’ve but into this document! > > > >I have some significant concerns (see below for details). In general, >the document presents an incomplete view of the architecture: details >about important pieces are barely mentioned or not fully described, as >is the case of the role of a central controller (PCE), and the BGP and >Binding Segments. > > > >Also, some of the architectural details are predicated on specific bits >defined in the extensions, where this document should describe the >general operation and leave the details (like bit names) to the >extensions. Note that I’m not asking that you don’t mention the >extensions – pointing to them is fine --, I’m asking you to define the >functionality in general and not as a function of the extensions. For >an example, see point M5.1. below. > > > >I will wait for at least the Major comments to be addressed before >starting the IETF Last Call. > > > >Thanks! > > > >Alvaro. > > > > > > > >Major: > > > >M1. The Introduction mentions several types of segments, and it says >that the LDP LSP, RSVP-TE LSP, and BGP LSP segments are “illustrated in >[I-D.ietf-spring-segment-routing-mpls]”. But that is only true for the >first two, for which examples are shown. Where are these segment types >defined? The definition, and not the examples, is the Major issue >here. This document being the main architecture document should >include the complete description. BTW, the list is only about the >“MPLS instantiation”, are there similar types of segments for IP? > > > > > >M2. From Section 2. (Terminology): “Using the same SRGB on all nodes >within the SR domain ease operations and troubleshooting and is >expected to be a deployment guideline.” It is “expected to be a >deployment guideline” where/when?? Given that this document is the >general architecture, I figured that maybe >draft-ietf-spring-segment-routing-mpls contained that deployment >recommendation, but all that document says is: “As described in >[I-D.ietf-spring-segment-routing], using the same SRGB on all nodes >within the SR domain eases operations and troubleshooting and is >expected to be a deployment guideline.” So…where are the >Deployment/Operational considerations related to the SRGB? I note that >neither document (draft-ietf-spring-segment-routing or >draft-ietf-spring-segment-routing-mpls) include them. I would expect >some information to be in this (general) document and other more >specific information (like the considerations about using the same >SRGB) to be in the more specific document >(draft-ietf-spring-segment-routing-mpls, in this case). > > > >M2.1. The example illustrated in Figure 2 would be a great place to >demonstrate the value of having the same SRGB. In fact, the text now >shows things not working and it even warns by saying that “using >anycast segment without configuring the same SRGB on nodes belonging to >the same device group may lead to misrouting”, but no explanation of >how it should be done the right way. > > > > > >M3. From Section 2. (Terminology): “…a global segment is represented by >a globally-unique index.” > > > >M3.1. I couldn’t find anywhere a discussion about the use of the >index. When it is discussed in 3.1.2, it seems to be an understood >concept: “A Prefix-SID is allocated in the form of an index in the >SRGB…” Even if straightforward, I think the concept of the index >should be explained (maybe with an example) and not assumed. I again >went to look at draft-ietf-spring-segment-routing-mpls, but that >document just points back to this one: “The notion of indexed global >segment, defined in [I-D.ietf-spring-segment-routing]…” > > > >M3.2. I assume that “globally-unique index” is really unique to the SR >Domain, right? I ask this question because “globally-unique IPv6 >address” has a different connotation (as in unique world-wide, not just >inside a domain). Please clarify. > > > >M3.3. Note that the latest version (-10) of >draft-ietf-spring-segment-routing-mpls introduced the SR Local Block >(SRLB) which is not defined in this document. I mention it here >because it was introduced right after the pointer about the index… > > > > > >M4. Section 3.1.1. (Prefix-SID Algorithm) > > > >M4.1. This section starts by justifying the use of an algorithm based >on the IGP protocol extensions: the extensions have an algorithm field, >so we should use it. The justification should be the other way around: >this document (the architecture) defines the concept of an algorithm >and the extensions implement it. Please re-word the first 3 paragraphs >– note that the instance/topology references seem superfluous to me. > > > >M4.2. The algorithms are not really defined – there are mentions of a >“well known ECMP-aware SPF algorithm”, but no specific reference to >what that is. The last sentence in the Section mentions that the >“details of the two defined algorithms are defined in…” pointing to the >extension drafts – but those drafts just offer an additional piece of >information in (from the OSPF document): “the standard shortest path >algorithm as computed by the OSPF protocol”. Ideally the algorithms >would be defined here (even if it is just to say: “the standard >algorithm used in the corresponding IGP”), and the extensions would >just reference it. > > > >M4.3. When should the packets be dropped?? The following text points >to at least 3 different places where it should be dropped, one marked >with a MUST. Please clarify. > > > >M4.3.1. “A router MUST drop any SR traffic associated with the SR >algorithm to the adjacent router, if the adjacent router has not >advertised support for such SR algorithm.” Ok, this sounds as if the >traffic is dropped one hop before the router not supporting the >algorithm – and it has a MUST in it. > > > >M4.3.2. “The ingress node of an SR domain validates that the path to a >prefix…includes nodes all supporting the advertised algorithm. As a >consequence, if a node on the path does not support algorithm X, the >IGP-Prefix segment will be interrupted and will drop packet on that >node.” This sounds like the traffic would be dropped on the node not >supporting the algorithm. > > > >M4.3.3. “It's the responsibility of the ingress node using a segment to >check that all downstream nodes support the algorithm of the segment.” >This last sentence hints at the fact that the ingress node should maybe >even stop the traffic there, or maybe not sending it unless all nodes >in the path support the same algorithm… > > > > > >M5. Section 3.1.2. (MPLS Dataplane). > > > >M5.1. The first bullet (again!) explains the architecture in function >of the extensions. The architecture should explain what needs to be >done and the extensions would then do it… Suggestion: > > > >NEW> > > In order to achieve a behavior equivalent to Penultimate Hop Popping > > in MPLS [Reference], a Node N advertising a Prefix-SID SID-R for its > > attached prefix R MUST be able to instruct its connected neighbors to > > perform the NEXT operation while processing SID-R. Upon receiving > > this instruction from Node N, its neighbors of N MUST perform the NEXT > > operation while processing SID-R. Alternatively, the neighbors of N > > MUST perform the CONTINUE operation while processing SID-R. > > > >M5.1.1. The penultimate bullet refers again to the extensions (but it >only mentions the P-bit this time). Please explain what the >architecture intends to do, but not from the point of view of the >extensions. This and the last bullet seem to be the result of the >first one (above). I think that the first bullet would have enough >text for the FIB entries to be obvious, but if you want to include this >corollary, please do it right after. > > > >M5.2. “A Prefix-SID is allocated in the form of an index in the SRGB >(or as a local MPLS label) according to a process similar to IP address >allocation.” The SRGB is defined (in the Terminology section) as “the >set of local labels reserved for global segments”, what is the >difference between “an index in the SRGB” and “a local MPLS label”? >I’m hoping that the explanation of the concept of index would clarify >this. > > > >M5.2.1. Three bullets down… “If a node learns a Prefix-SID having a >value that falls outside the locally configured SRGB range, then the >node MUST NOT use the Prefix-SID…” Going back to my previous question: >it looks like “a local MPLS label” would have to be included in the >SRGB – again, clarifying upfront would help a lot. Note that this >piece of text falls into the recommendation of having the same SRGB >configured in all nodes. > > > >M5.2.2. Related to the concept of index and its relationship to the >SRGB…the next bullet says: “…the segment is global (the SID is >allocated from the SRGB or as an index)”. We now have “globally-unique >index”, “an index in the SRGB”, and “the SRGB or as an index” >(separately). > > > > > >M6. Section 3.3. (IGP-Anycast Segment, Anycast SID): “…the value of the >SID following the anycast SID MUST be understood by all nodes >advertising the same anycast segment.” It seems to me that this is >really a statement of fact: all nodes, not just ones advertising an >anycast segment must understand what to do with the next SID… IOW, >s/MUST/must I would even take this sentence out because it is >redundant and obvious. > > > > > >M7. Section 3.4. (IGP-Adjacency Segment, Adj-SID) also tries to explain >functionality starting from the extensions. > > > >M7.1. “The encodings of the Adj-SID include the a set of flags among >which there is the B-flag. When set, the Adj-SID refers to an >adjacency that is eligible for protection (e.g.: using IPFRR or >MPLS-FRR).” If the Adjacency Segment is one that is locally >significant to the node advertising it, what is the purpose of >signaling that it is eligible for protection? Wouldn’t that be a local >decision as well? Maybe an example of how the architecture is expected >to work would help. > > > >M7.2. “The encodings of the Adj-SID also include the L-flag. When set, >the Adj-SID has local significance. By default, the L-flag is set.” >The definition of IGP-Adjacency Segment already says that it is “local >(unless explicitly advertised otherwise)”, which makes this statement >unnecessary. If you want to keep it, please figure out a way that >doesn’t justify it based on a bit in the extensions. > > > > > >M8. Section 3.5. (Binding Segment). A binding segment is not described >anywhere in this document – please do so! Section 3.5.1. (Mapping >Server) mentions a “Remote-Binding SID S advertised by the mapping >server”, and says that more “details are described in the SR/LDP >interworking procedures >([I-D.ietf-spring-segment-routing-ldp-interop]”, but that draft doesn’t >mention a Binding Segment. Section 5. (IGP Mirroring Context Segment) >says that “the binding segment [is] defined in SR IGP protocol >extensions ( [I-D.ietf-isis-segment-routing-extensions], >[I-D.ietf-ospf-segment-routing-extensions] and >[I-D.ietf-ospf-ospfv3-segment-routing-extensions])”; however, those >documents don’t mention a Binding Segment, they just (except for the >OSPFv2 draft) define TLVs. Note that >I-D.ietf-ospf-segment-routing-extensions points back to this document >when referring to the definition of a Binding SID, completing a >circular reference. > > > >M8.1. BTW, Section 5. (IGP Mirroring Context Segment) says that the >“Mirror SID is advertised using the binding segment”, which then looks >like the Mirror Segment is an application of the Binding Segment (+ an >explicit indication of it). I think that Section 5 should then be >moved to be a sub-section of 3.5. > > > >M8.2. Part of the Security Considerations (in Section 8.1) are related >to the Binding SID, which is another reason for clearly explaining that >part of the architecture here. > > > > > >M9. Section 3.5.1. (Mapping Server) mentions a Mapping Server, but it >punts to I-D.ietf-spring-segment-routing-ldp-interop for further >details. While the SRMS can be used for interoperability cases, I >think that it is an important part of the overall architecture and as >such it should be described and discussed in this document >instead…including any Manageability Considerations (as mentioned in >Section 9). > > > > > >M10. Section 3.6. (Inter-Area Considerations) > > > >M10.1. This section shows an example of the behavior: maintain the SID >across area boundaries. But it doesn’t actually say how the >architecture is expected to work. IOW, in the example the SID is >maintained, but should that always happen (MUST, SHOULD)? Or is it just >an example (MAY)? > > > >M10.2. Another case of explaining the architecture based on the >extension functionality: “When an ABR propagates a prefix from one area >to another it MUST set the R-Flag.” Maybe try something like this >instead: “The re-advertisement of an SID that originated on a different >IGP area MUST be indicated.” > > > >M10.3. [minor] As the advertisement moves to the left (from Area 2 to >the Backbone…), the ABRs change the Node-SID for a Prefix-SID, right? >The description in the last 3 paragraphs uses Node-SID: “node S…pushes >Node-SID(150)”. > > > >M10.4. [minor] Going back to global vs local significance, it would be >nice to remind the readers at this point that the SR domain is really >the whole IGP network, and not just one flooding domain…which is why >SID 150 can be used throughout. > > > > > >M11. Section 4. (BGP Peering Segments) is the only non-IGP focused >section in this document. The architecture of the EPE solution (as >described in draft-ietf-spring-segment-routing-central-epe) goes beyond >what has already been discussed here because it incorporates elements >such as a mandatory centralized controller, which was optional before >(a PCE server is mentioned only casually in the Terminology section). >This leaves us with an incomplete architecture for the BGP case. I >would prefer it if the whole architecture was defined in this single >document (i.e. expand this section by potentially moving parts from >draft-ietf-spring-segment-routing-central-epe – which might leave that >document without enough content to stand alone). > > > > > >M12. Section 8. (Security Considerations). The main part of this >section talks about the instructions on the packets – is adding that >meta-data a security concern? I think it could be because someone >watching could tell which “forwarding path elements (e.g.: nodes, >links, services, etc.)” are used for specific flows, etc. But I also >think that the risk is mitigated by the fact that the information MUST >NOT be exposed outside the SR domain. Mentioning that, and the fact >that the SR domain will usually be under a single administration would >be a good thing. > > > > > >M13. Section 9. (Manageability Considerations) says that “an >implementation SHOULD protect the network in case of conflict detection >by providing a deterministic resolution approach…addressed in >[I-D.ietf-spring-conflict-resolution].” That “SHOULD” is in conflict >with I-D.ietf-spring-conflict-resolution (in WGLC) where it says that >“All protocols which support SR MUST adhere to the policies defined in >this document.” It seems like the easy way forward is to >s/SHOULD/MUST. In either case, I think that >I-D.ietf-spring-conflict-resolution should be a Normative reference. > > > > > > > >Minor: > > > >P1. Introduction > > > >P1.1. The term “service chain” is used in the Abstract and in the >Introduction. Given that the concept is not vital to the architecture >and that there might be unnecessary confusion with SFC, I would suggest >taking it out. > > > >P1.2. Related… Section 1.1 says that this document “defines…the >service segments”, but there’s no specific mention of “service >segments” anywhere, except for a very quick mention in Section 5. (IGP >Mirroring Context Segment). What am I missing? Consider taking >“service segments” off the list of things this document defines. > > > >P1.3. The last paragraph says that “This document defines a set of >instructions (called segments) that are required to fulfill the >described use-cases.” The use cases are not mentioned until 1.1. >Suggestion: merge Section 1 and 1.1 to provide proper flow to the text. > > > >P1.4. Speaking of use cases, I-D.ietf-spring-oam-usecase doesn’t >actually include use cases that will affect the architecture. It >includes use case of how to use monitoring system defined in it. Same >comment for the mention in Section 9. > > > > > >P2. From Section 2. (Terminology) > > > >P2.1. SID is not defined, just extended. Besides the examples, please >provide an actual definition. > > > >P2.2. “The CONTINUE instruction is implemented as the SWAP instruction >in the MPLS dataplane.” Please include a reference to where the “SWAP >instruction” is defined. I assume you’re really referring to “label >swapping” in rfc3031, are you? If so, please be consistent as “MPLS >dataplane” and “MPLS architecture” are used in different places. > > > >P2.3. The “Local Segment” definition says that for IPv6 it “is not >advertised in any routing protocol”. But for MPLS it is advertised, >right? Please clarify. I think that some of the vocabulary is a >little confusing, for example, the definition of IGP-Adjacency Segment >says that it “is local (unless explicitly advertised otherwise) to the >node that advertises it” – this means that for IPv6 this piece of the >architecture wouldn’t apply because a Local Segment is not advertised. >To add to the confusion, draft-filsfils-spring-srv6-network-programming >talks about local SIDs… IOW, in some places it sounds as if what >should be the general architecture only really applies to MPLS – or >maybe we need a more explicit local qualifier. > > > >P2.4. “The PCEP discovery capability is described in >[I-D.ietf-pce-segment-routing].” That document doesn’t even mention >the word “discovery”. Please use the proper name for easier >cross-reference. > > > >P2.5. “unless explicitly advertised otherwise” is used to qualify the >global/local nature of a segment. How do the characteristics of the >segment change if “explicitly advertised otherwise”? For example, if >an IGP-Prefix Segment is advertised as local, how do its >characteristics change, or do they? > > > >P2.5.1. In Section 3.4. (IGP-Adjacency Segment, Adj-SID), the use of an >Adj-SID is illustrated for both the local and global cases, but both >explanations say the same thing: “forwarded along the shortest-path to >N and then be switched by N, without any IP shortest-path >consideration, towards link L” – I don’t see a difference in behavior >between the local and global nature of this SID. There is some >additional text related to global: “The use of global Adj-SID allows to >reduce the size of the segment list when expressing a path at the cost >of additional state (i.e.: the global Adj-SID will be inserted by all >routers within the area in their forwarding table).” If node N is the >one executing on the Adj-SID, how does including it in other FIBs help? > > > >P2.6. None of the segments after Section 3.4 (binding, BGP, mirroring) >are defined in the Terminology. Please do so for completeness. > > > > > >P3. Referring to Figure 1: “…each PE device is connected to two routers >in each of the groups A and B.” The routers connected to the PEs (Rx) >are not in either group. > > > > > >P4. s/Obviously// > > > > > >P5. Section 3.4. (IGP-Adjacency Segment, Adj-SID): “The remote node MAY >be an adjacent IGP neighbor or a non-adjacent neighbor…” The “MAY” is >out of place as there isn’t another option, it is one or the other. >s/MAY/may > > > > > >P6. Maybe it’s because the rest of 3.5 is incomplete, but Section >3.5.2. (Tunnel Head-end) has no context to speak of – without that, it >feels like an orphan section. > > > > > >P7. Section 8.1. (MPLS Data Plane) > > > >P7.1. You included a couple of references at the end of this section, >which is good. I would however not explicitly mention specific >sections, as the whole RFCs (rfc4381 and rfc5920) are about >MPLS-related security. > > > >P7.2. You compare the use of a single segment to RSVP-TE – if the >security characteristics are similar, please provide a reference. > > > > > >P8. References > > > >P8.1. RFC2460 was obsoleted by RFC8200 / RFC6822 was obsoleted by >RFC8202 > > > >P8.2. I think that the reference to RFC4206 should be Informative. > > > > > > > >Nits: > > > >N1. The Abstract is very long – I would even say too long. If it was >up to me, I would leave just the first paragraph. > > > >N2. The second to last paragraph in the Introduction (“Numerous >use-cases…”) references the “marketing style” content of >I-D.ietf-spring-oam-usecase. I have asked the authors of that document >to please focus their content. Please consider taking this paragraph >out. > > > >N3. s/participating into the source based/participating in the source >based > > > >N4. References. Please add Informative references to netconf (Section >2), PCEP (2), FRR (3.1.1), “parallel adjacency suppression” (3.4). > > > >N5. Section 3: “IGP segments require extensions in link-state IGP >protocols. IGP extensions are required in order to advertise the IGP >segments.” These 2 sentences say the exact same thing. > > > >N6. Please avoid using “we” in the text – except for the >Acknowledgements. > > > >N7. s/Regardless Segment Routing/Independent of Segment Routing > > > >N8. s/and not to each other individual nodes in the LAN/and not to >individual nodes in the LAN > > > >N9. >s/([I-D.ietf-spring-segment-routing-ldp-interop]/[I-D.ietf-spring-segment-routing-ldp-interop] > > > >N10. “IGP deployed using areas” An area is an OSPF-specific construct >– flooding domain is more generic. > > > >N11. It would be nice if the examples used IPv6 instead of IPv4.
_______________________________________________ spring mailing list [email protected] https://www.ietf.org/mailman/listinfo/spring
