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:spring-boun...@ietf.org] On Behalf Of Alvaro Retana 
(aretana)
Sent: Wednesday, August 23, 2017 7:39 PM
To: 
draft-ietf-spring-segment-routing-m...@ietf.org<mailto:draft-ietf-spring-segment-routing-m...@ietf.org>
Cc: spring-cha...@ietf.org<mailto:spring-cha...@ietf.org>; 
spring@ietf.org<mailto:spring@ietf.org>; Martin Vigoureux 
<martin.vigour...@nokia.com<mailto:martin.vigour...@nokia.com>>
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
_______________________________________________
spring mailing list
spring@ietf.org
https://www.ietf.org/mailman/listinfo/spring

Reply via email to