Re: [mpls] RtgDir Early review: draft-ietf-spring-segment-routing-mpls-13

[Ahmed Bashandy <[e-mail]>]

I just posted version 14 to address Adrian's comments

I will address your comments in the next version in few days

thanks

Ahmed



On 6/10/18 12:43 AM, Alexander Vainshtein wrote:
>
> Explicitly adding Shraddha  who is the shepherd of this draft.
>
> Regards,
>
> Sasha
>
> Office: [phone]
>
> Cell: [phone]
>
> Email: [e-mail]
>
> *From:* Alexander Vainshtein
> *Sent:* Friday, June 8, 2018 5:43 PM
> *To:* '[e-mail]' <[e-mail]>; 
> '[e-mail]' 
> <[e-mail]>
> *Cc:* '[e-mail]' <[e-mail]>; [e-mail]; 
> [e-mail]; '[e-mail]' <[e-mail]>
> *Subject:* RtgDir Early review: draft-ietf-spring-segment-routing-mpls-13
>
> Hello,
>
> I have been selected to do a routing directorate “early” review of 
> this draft: 
> https://datatracker.ietf.org/doc/draft-ietf-spring-segment-routing-mpls/
>
> The routing directorate will, on request from the working group chair, 
> perform an “early” review of a draft before it is submitted for 
> publication to the IESG. The early review can be performed at any time 
> during the draft’s lifetime as a working group document. The purpose 
> of the early review depends on the stage that the document has 
> reached. As this document is currently in the WG Last call, my focus 
> for the review was to determine whether the document is ready to be 
> published. Please consider my comments along with the other working 
> group last call comments.
>
> For more information about the Routing Directorate, please see 
> ​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>
> *Document*: draft-ietf-spring-segment-routing-mpls-13
>
> *Reviewer*: Alexander (“Sasha”) Vainshtein 
> ([e-mail] 
> <mailto:[e-mail]>)
>
> *Review Date*: 08-Jun-18
>
> *Intended Status*: Proposed Standard.
>
> *Summary*:
>
> I have some minor concerns about this document that I think should be 
> resolved before it is submitted to the IESG.
>
> *Comments*:
>
> I consider this draft as an important  companion document to the 
> Segment Routing Architecture 
> <https://tools.ietf.org/html/draft-ietf-spring-segment-routing-15> 
> draft that, ideally, should augment definitions of the Segment Routing 
> (SR) notions and constructs given there with details specific for the 
> SR instantiation that uses  the MPLS data plane (SR-MPLS).  Many 
> issues raised in my review reflect either gaps that should be, but 
> have not been, closed, or inconsistencies between the two drafts.
>
> Since RFC 8287 <https://tools.ietf.org/html/rfc8287> is already 
> published as a Standards Track RFC, I expect such augmentation to be 
> backward compatible with this document (or to provide clear 
> indications of required updates to this document). And I include the 
> MPLS WG into distribution list.
>
> This draft was not easy reading for me. In particular, the style of 
> Section 2.5 that discusses at length and in some detail multiple 
> “corner cases” resulting, presumably, from misconfiguration, before it 
> explains the basic (and relatively simple) “normal” behavior, looks 
> problematic to me.
>
> The WG Last Call has been extended by one week. Nevertheless, I am 
> sending out my comments
>
> *Major Issues*: None found
>
> *Minor Issues*: Quite a few but, hopefully, easy to resolve.
>
> 1.*Encapsulation of SR-MPLS packets*:
>
> a.RFC 3032 (referenced by the draft) and RFC 5332 (*/not mentioned in 
> the draft/*) depend two encapsulations of labeled packets - one for 
> Downstream-allocated labels and another for Upstream-allocated ones.
>
> b.From my POV the ST-MPLS only uses Downstream-allocated labels – but 
> I expect the draft to state that explicitly, one way or another. (If 
> Upstream-allocated labels are relevant for SR-MPLS, I would see it as 
> a major gap, so I hope that this is not the case).
>
> 2.*Label spaces in SR-MPLS*:
>
> a.RFC 3031 (referenced by the draft) defines per-platform and 
> per-interface label spaces, and RFC 5331 (*/not mentioned in the 
> draft/*) adds context-specific label spaces and context labels.
>
> b.The draft does not say which of these are or are not relevant for 
> SR-MPLS
>
> c.From my POV:
>
> i.Labels representing all kinds of SIDs mentioned in the draft MUST be 
> allocated from the per-platform label space only
>
> ii.At the same time, instantiation of Mirror Segment IDs defined in 
> Section 5.1 of the Segment Routing Architecture draft using MPLS data 
> plane clearly calls for context labels and context-specific label spaces
>
> d.I expect the draft to provide a clear-cut position on these aspects 
> of SR-MPLS.
>
> 3.*SR-MPLS and hierarchical LSPs*:
>
> a.SR LSPs that include more than one segment are hierarchical LSPs 
> from the POV of the MPLS data plane. Therefore some (possibly, all) of 
> the models for handling TTL and TC bits that have been defined in RFC 
> 3443 (*/not mentioned in the draft/*) should apply to SR-MPLS
>
> b.RFC 8287 (*/not referenced in the draft/*) specifically discussed 
> operation of the LSP Traceroute function for SR LSPs in the case when 
> Pipe/Short Pipe model for TTL handling is used
>
> c.I expect the draft to provide at least some guidelines regarding 
> applicability of each specific model defined in RFC 3443 (separately 
> for TTL and TC bits) to SR-MPLS.
>
> 4.*Inferring network layer protocol in SR-MPLS*:
>
> a.I wonder if the draft could provide any details on the situation 
> when a label that represents some kind of SID is the bottom-of-stack 
> label to be popped by the egress LER
>
> b.For the reference, RFC 3032 says that “the identity of the network 
> layer protocol  must be inferable from the value of the label which is 
> popped from  the bottom of the stack, possibly along with the 
> contents  of the network layer header itself”
>
> c.From my POV the following scenario indicates relevance of this 
> expectation for SR-MPLS:
>
> i.IS-IS is used for distributing both IPv4 and IPv6 reachability in a 
> given domain
>
> ii.An IS-IS adjacency over some dual-stack link is established, and a 
> single Adj-SID for this adjacency is advertised
>
> iii.The node that has assigned and advertised this Adj-SID receives a 
> labeled packet with the label representing this Adj-SID being both the 
> top and bottom-of-stack label
>
> iv.The implementers must be given unambiguous instructions for 
> forwarding the unlabeled packet via the dual-stack link as an Ipv4 or 
> an IPv6 packet.
>
> 5.*Resolution**of Conflicts*: Are the
>
> a.Are the conflict resolution procedures listed in section 2.5 
> mandatory to implement?
>
> b.If they are mandatory to implement, are they also mandatory to 
> deploy, or can the operators simply treat any detected conflict as 
> requiring human intervention and preventing normal operation of SR-MPLS?
>
> c.For the reference, the IETF capitalized MUST appears just in a few 
> places in Section 2.5, and each appearance has very narrow context:
>
> i.For MCCs where the "Topology" and/or "Algorithm" fields are not 
> defined, the numerical value of zero MUST be used for these two fields
>
> ii.If the same set of FECs are attached to the same label "L1", then 
> the tie-breaking rules MUST always select the same FEC irrespective of 
> the order in which the FECs and the label "L1" are received. In other 
> words, the tie-breaking rule MUST be deterministic.
>
> iii.An implementation of explicit SID assignment MUST guarantee 
> collision freeness on the same router
>
> From my POV, it is not possible to infer the answer to my question 
> from these statements. Some explicit statement is required.
>
> d.The tie-breaking rules in section 2.5.1 include some specific values 
> for encoding FEC types and address families – but these values are not 
> supposed to appear in any IANA registries (because the draft does not 
> request any IANA actions). Can you please clarify what is so special 
> about these values?
>
> e.I also doubt that comparison of FECs that represent IPv4 and IPv6 
> prefix SIDs makes much sense (for conflict resolution or else), 
> because, among other things, there are valid scenarios when an IPv4 
> /32 prefix is embedded in an IPv6 /128 one.
>
> f.Section 2.5.1 defines 3 types of SR-MPLS FECs, but I am not sure all 
> SID types defined in the Segment Routing Architecture draft can be 
> unambiguously mapped to one of these types. Problematic examples 
> include at least the following:
>
> i.Parallel Adjacency SID
>
> ii.Mirror SID
>
> Explicit mapping of SID types to SR-MPLS FEC types would be most 
> useful IMO. If some SID types cannot be mapped to SR-MPLS FECs, this 
> must be explicitly stated in the draft.
>
> 6.*Node SIDs in SR-MPLS*:
>
> a.Node SIDs are explicitly defined and discussed in the Segment 
> Routing Architecture draft but are not mentioned even once in this draft
>
> b.AFAIK, the common implementation practice today includes assignment 
> of at least one Node SID to every node in the SR-MPLS domain
>
> c.Is there a requirement to assign at least one Node SID per {routing 
> instance, topology, algorithm} in SR-MPLS? If not, can the authors 
> explain expected behavior of such a node? (See also my comment about 
> routing instances below).
>
> 7.*SRGB Size in SR-MPLS*:
>
> a.The draft correctly treats the situation when an index assigned to 
> some global SID cannot be mapped to a label using the procedure in 
> Section 2.4 as a conflict.
>
> b.At the same time the draft does not define any minimum size of SRGB 
> (be it defined as a single contiguous block or as a sequence of such 
> blocks) that MUST be implemented by all SR-capable nodes
>
> c.I suspect that lack of such a definition could be detrimental to 
> interoperability of SR-MPLS solutions. AFAIK, the IETF has been 
> following, for quite some time, a policy that some reasonable 
> MUST-to-implement defaults should be assigned for all configurable 
> parameters exactly in order to prevent this.
>
> 8.*Algorithms and Prefix SIDs*:
>
> a.The draft mentions Algorithms (as part of SR-MPLS Prefix FEC) in, 
> but it does not explicitly link them with the Prefix-SID algorithms 
> defined in section 3.1.1 of the Segment Routing Architecture draft
>
> b.From my POV, the draft should explicitly state that the default 
> Prefix-SID algorithm MUST be implemented in all SR-MPLS-compliant routers.
>
> c.The Segment Routing Architecture draft states (in section 3.1.3) 
> that “Support of multiple algorithms applies to SRv6”. But neither 
> draft states whether multiple algorithms apply to SR-MPLS. Can you 
> please clarify this point?
>
> 9.*Routing instances and the context for Prefix-SIDs*:
>
> a.The Segment Routing Architecture draft states in Section 3.1 that 
> the “context for an IGP-Prefix segment includes the prefix, topology, 
> and algorithm”
>
> b.This draft seems to define (in section 2.5) the context for the 
> Prefix SID as “Prefix, Routing Instance, Topology, Algorithm” where ”a 
> routing instance is identified by a single incoming label downloader 
> to FIB” (but the notion of the label downloader to FIB is not defined).
>
> c.These two definitions look different to me.
>
> d.At the very least I would expect alignment between the definitions 
> of context for the Prefix-SID between the two drafts. Preferably, the 
> definition given in the Segment Routing Architecture draft should be 
> used in both drafts.
>
> 10.*Example of PUSH operation in Section 2.10.1*:
>
> a.The first para of this section begins with the sentence “Suppose an 
> MCC on a router "R0" determines that PUSH or CONTINUE operation is to 
> be applied to an incoming packet whose active SID is the global SID 
> "Si"”. In the context of SR-MPLS this means (to me) that the incoming 
> packet is a labeled packet and its top label matches the global SID “Si”.
>
> b.However, the example for PUSH operation in the next para of this 
> section is the case of an (unlabeled) IP packet with the destination 
> address covered by the IP prefix for which “Si” has been assigned.
>
> c.From my POV:
>
> i.Mapping unlabeled packets to SIDs is indeed out of scope of the 
> draft. Therefore an example of a PUSH operation that is applied to a 
> labeled packet (with the active SID inferred from the top label in the 
> stack) is preferable.
>
> ii.Valid examples of  PUSH operation applied to a labeled incoming 
> packet can be found in Sections 4.2 or 4.3 of the TI-LFA 
> <https://tools.ietf.org/html/draft-bashandy-rtgwg-segment-routing-ti-lfa-04> 
> draft
>
> *Nits*:
>
> 1.I concur with Adrian regarding numerous nits he has reported in his 
> WG LC Comment 
> <https://mailarchive.ietf.org/arch/msg/spring/FRhO2lgR8r4VlKP2ZN2dZwHU5BY>. 
> I would like to thank Adrian for an excellent review that have saved 
> me lots of hard work.
>
> 2.In addition, I’d like to report the following nits:
>
> a.Ti-LFA in Section 2.11.1 should be TI-LFA (as in the TI-LFA 
> <https://tools.ietf.org/html/draft-bashandy-rtgwg-segment-routing-ti-lfa-04> 
> draft)
>
> b.TI-LFA draft is referenced in the text of Section 2.11.1, but there 
> is no matching reference in the “References” section (probably, 
> Informational)
>
> c.“zero Algorithm” in Section 2.5 (immediately above Section 2.5.1) 
> must be replaced with “default algorithm”. Similarly, “non-zero 
> Algorithm” should be replaced with “non-default algorithm”
>
> 3.I think that RFC 3443 and RFC 5332 should be listed as Normative 
> references in this draft while RFC 5331 and RFC 8277 should be listed 
> as Informative references. This would improve the readability of the 
> draft without any impact on its advancement.
>
> Hopefully, these comments will be useful.
>
> Regards,
>
> Sasha
>
> Office: [phone]
>
> Cell:      [phone]
>
> Email: [e-mail] 
> <mailto:[e-mail]>
>
>
> ___________________________________________________________________________
>
> This e-mail message is intended for the recipient only and contains 
> information which is
> CONFIDENTIAL and which may be proprietary to ECI Telecom. If you have 
> received this
> transmission in error, please inform us by e-mail, phone or fax, and 
> then delete the original
> and all copies thereof.
> ___________________________________________________________________________