Re: [spring] AD Review of draft-ietf-spring-segment-routing-mpls-10

["Ahmed Bashandy (bashandy)" <[e-mail]>]

I just checked the IPR section in ietf.org. The IPR disclosure for this 
draft can be found in
https://datatracker.ietf.org/ipr/search/?draft=draft-ietf-spring-segment-routing-mpls&submit=draft&rfc=&doctitle=&group=&holder=&iprtitle=&patent=

Thanks

Ahmed

On 10/30/2017 11:12 AM, Ahmed Bashandy (bashandy) wrote:
>
> Sorry for the late reply and thanks a lot for the thorough review.
>
> We have just uploaded version 11 of the draft.
>
> See replies inline “#Ahmed” to your comments
>
> Thanks
>
> Ahmed
>
> *From:*spring [mailto:[e-mail]] *On Behalf Of *Alvaro 
> Retana (aretana)
> *Sent:* Wednesday, August 23, 2017 7:39 PM
> *To:* [e-mail] 
> <mailto:[e-mail]>
> *Cc:* [e-mail] <mailto:[e-mail]>; 
> [e-mail] <mailto:[e-mail]>; Martin Vigoureux 
> <[e-mail] <mailto:[e-mail]>>
> *Subject:* [spring] AD Review of draft-ietf-spring-segment-routing-mpls-10
>
> Dear authors:
>
> I just finished reading this document.  Please take a look at my 
> comments below.
>
> The main questions/concerns that I have related to this document is 
> not just for the authors, but for the Shepherd and the Chairs too.
>
> Q1. Why is this document on the Standards Track? From the 
> Introduction: “This drafts describes how Segment Routing operates on 
> top of the MPLS data plane.”  Describes, yes.  On the other hand, the 
> Shepherd’s write-up says that it “specifies the generic functions of 
> the architecture” – I don’t see a specification, just a description.  
> As such, I think this document should be Informational.
>
> #Ahmed: The new version of the draft specifies many things that are 
> applicable to instantiation of SR over MPLS
>
> Q2. Section 2. (MPLS Instantiation of Segment Routing) is the only one 
> with any real content…but there are only a couple of things in it that 
> are not in the Architecture document: the introduction of the SRLB, 
> and some words about the index – both of which should be really 
> explained in the Architecture document, and not here.  I wonder what 
> the value of publishing this document really is.  What long-term 
> archival value does it provide?
>
> #Ahmed: The long term plan is to move details of MPLS-specific 
> specifications to this document and keep the architecture document 
> more general
>
> Q3. I also have to wonder about the IPR declared for this document.  
> If most of the information here is already defined, described or 
> specified in draft-ietf-spring-segment-routing, should the IPR 
> declaration apply to that document as well (or maybe instead of this 
> one)?  It is not the IETF’s role (including the WG) to discuss whether 
> a piece of IPR is valid or not – I just want to make sure the 
> disclosures apply to the right document.
>
> #Ahmed: We will make sure that all IPR is declared
>
> I didn’t find a discussion on the list about any of these points.
>
> Thanks!
>
> Alvaro.
>
> Major
>
> M1. Section 2. (MPLS Instantiation of Segment Routing) explains how 
> “in the MPLS instantiation, the SID values are allocated within a 
> reduced 20-bit space out of the 32-bit SID space.” However, I couldn’t 
> find where draft-ietf-spring-segment-routing defines the SID space as 
> using 32 bits (or any other length).  In fact, the closest that 
> document comes is when it defines an SID and mentions “Examples of 
> SIDs are: an MPLS label, an index value in an MPLS label space, an 
> IPv6 address.”   I’m assuming the “32-bit SID space” comes from the 
> fact that the extensions define an SID of that length.  All this is to 
> say that as you describe how SR operates in the MPLS dataplane, do so 
> not explicitly depending on the implementation of the extensions 
> (which in fact seem to already account for different lengths).
>
> #Ahmed: The part that talks about the reduced 20 bit spaced has been 
> removed. Instead the beginning of section 2.2 clearly specifies that a 
> SID is an MPLS label and if it is an index it should be inside the 
> SRGB. Section 2.4 specifies how to translate the Index to a label 
> within the SRGB
>
> M2. [I mentioned these issues in the review of 
> draft-ietf-spring-segment-routing as well.]
>
> M2.1. “The notion of indexed global segment, defined in 
> [I-D.ietf-spring-segment-routing]…”  That document doesn’t properly 
> define the concept/use of the index.  There are several mentions in 
> this document that I think rely on a proper definition/discussion of 
> the concept.
>
> #Ahmed: Section 2.4 specify how to translate the Index to a label 
> within the SRGB. The architecture draft defines the concept of an index
>
> M2.2. The concept of an SRLB is not defined in the Architecture 
> document either.
>
> #Ahmed: The latest version of the architecture document defines the SRLB
>
> M3. Still in Section 2, from this text:
>
>       * When different SRGBs are used, the outgoing label value is set
>
>          as: [SRGB(next_hop)+index].  If the index can't be applied to
>
>          the SRGB (i.e.: if the index points outside the SRGB of the
>
> next-hop or the next-hop has not advertised a valid SRGB), then
>
>          no outgoing label value can be computed and the next-hop MUST
>
>          be considered as not supporting the MPLS operations for that
>
> particular SID.
>
> …several questions/comments…
>
> M3.1. [minor] I hope to see an explanation of the 
> “[SRGB(next_hop)+index]” notation.
>
> #Ahmed: Section 2.4 clearly specifies how to map an index to a label 
> within the SRGB
>
> M3.2. What is a “valid SRGB”?  I don’t think the validity of the SRGB 
> is described in the Architecture document.
>
> #Ahmed: The term “valid SRGB” has been removed. Instead Section 2.3 
> specifies the conditions an SRGB must satisfy
>
> M3.3. I’m assuming that once the “next-hop MUST be considered as not 
> supporting” then the packets are dropped, right?
>
> #Ahmed: Sections 2.8 and 2.9 specify the forwarding behavior. I hope 
> they are sufficiently clear
>
> M3.4. [Maybe I’m missing something obvious here.]  Going back to the 
> validity of the SRGB advertised by a specific node, shouldn’t the 
> ingress node verify that before imposing a path that will fail?  But I 
> couldn’t find anything in the Architecture document that talks about 
> the ingress node verifying that the path is valid (including the 
> validity of the SRGB).
>
> #Ahmed: Section 2.8 and 2.9 clearly specify the forwarding behavior 
> for PUSH. I hope they are sufficiently clear. This document also have 
> a reference to [I.D. filsfils-spring-segment-routing-policy] where the 
> discussion about calculating and verifying the values of labels in a 
> stack of labels
>
> M4. Still in Section 2: “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.”  As I mentioned in my review 
> of the Architecture document, that document doesn’t contain deployment 
> guidelines related to the SRGB, and it doesn’t describe how “using the 
> same SRGB…eases operations and troubleshooting”.
>
> #Ahmed: The latest version of the architecture document addresses this 
> comment
>
> Minor
>
> P1. The term “service chain” is used in the Abstract.  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.
>
> #Ahmed: The term “service chain” has been removed from this draft
>
> P2. Informative References to VPN, VPLS, VPWS, LDP, RSVP-TE…would be nice.
>
> #Ahmed: References to all of this has been removed from this document
>
> P3. Section 2. (MPLS Instantiation of Segment Routing) says that “a 
> controller-driven network…MAY use the control plane to discover the 
> available set of local SIDs”.  The “MAY” implies that there is a 
> choice (i.e. it is optional) and that other discovery mechanisms 
> exist.  What are those other choices?  Note that earlier in this 
> section you already wrote that IGPs are used for flooding the 
> information. s/MAY/may
>
> #Ahmed: This document gives an example of a use of the SRLB. Listing 
> all possible uses of the SRLB is certainly not within the scope of 
> this document.
>
> P4. Section 2: “…the use of the binding segment as specified in 
> [I-D.ietf-spring-segment-routing], also allows to substantially reduce 
> the length of the segment list…”  Nice, but there is no description of 
> the binding segment in draft-ietf-spring-segment-routing.
>
> #Ahmed: The latest version of draft-ietf-spring-segment-routing takes 
> care of defining a binding SID
>
> P5. References.  Please take a look at rfc8174 and update the 
> “Requirements Language” and associated references.
>
> #Ahmed: I agree. I modified the paragraph about requirements language 
> to conform to rfc8174
>
> Nits
>
> N1. I think that the references to *-segment-routing-extensions are 
> superfluous.  BTW, the fourth paragraph of the Introduction uses a 
> reference to *-segment-routing-extensions to point at ISIS/OSPF (the 
> protocols!).
>
> #Ahmed: I agree. Now we have clear references to each IGP extensions 
> separately
>
> N2. It would be very nice if the examples used IPv6 addresses.
>
> #Ahmed: It is just that everyone is used to IPv4 more than IPv6. So we 
> believe using IPv4 is easier for people to understand
>