Early Review of draft-ietf-mpls-spring-lsp-ping-06
|Requested rev.||no specific revision (document currently at 13)|
|Team||Routing Area Directorate (rtgdir)|
|Requested by||Loa Andersson|
|Draft last updated||2017-08-24|
Rtgdir Early review of -06 by Tony Przygienda
Rtgdir Last Call review of -06 by Sasha Vainshtein (diff)
Secdir Last Call review of -11 by Stephen Farrell (diff)
Genart Last Call review of -11 by Wassim Haddad (diff)
The working group call ends on August 1. Would be good to have a reviewer that has experience from "mpls", "spring" and "lsp-ping", Carlos and Mach comes to mind, but they are both co-authors. Martin V maybe, or Matthew B.
|Reviewed rev.||06 (document currently at 13)|
|Review result||Has Issues|
I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir<http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir> Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft. Document: https://www.ietf.org/id/draft-ietf-mpls-spring-lsp-ping-06.txt Reviewer: Tony Przygienda Review Date: 08/17 Intended Status: Standards Track Summary: I have significant concerns about this document and recommend that the Routing ADs discuss these issues further with the authors. Overall, I think that both the quality as well as scope of the draft seems rushed to LC and at a deeper level, SR IMO maybe not in a state yet to deliver a complete, standards-track RFC for the scope the draft claims to cover unless its breadth gets heavily curtailed. As note of caution: I’m not traditionally a MPLS/SR dataplane deep subject matter expert so I basically looked over the architectural integrity first and then read up enough detail to be fairly sure I make sense. Nevertheless, some of my comments may be off mark obviously. Comments: * The name of the draft implies a scope which it covers to a small degree only * The draft feels somewhat rushed, contains numerous spelling mistakes, partial and mis-numbered references. It omits good amount of text that is necessary for a standards track spec in terms of being normative (missing capitalization, error conditions/mis-formatted input etc). * I recommend to send the draft back to the WG until the according SIDs are mentioned/treated/stabilized to the point where it truly is a “SPRING LSP Ping” or otherwise rename the draft/RFC as “LSP Ping for IGP Prefix/adjacency SIDs” and address the comments pertaining to this scope Major Issues: · While claiming that it’s “LSP Ping/Trace for SPRING” the document does not mention many types of SIDs already defined today and/or does not differentiate enough between them o LAN adjacency SIDs (either that needs to be differentiated from adjacency SIDs or explained why it’s not necessary) o Binding SIDs o anycast SIDs o EPE SIDs o Mirroring SIDs? o possibly topology SIDs o in section 5.3 we have an assumption that GMPLS is running to support unnumbered links (4302)? How does that play with rfc3630 and incoming draft-ietf-ospf-lls-interface-id-00 identifiers. o we have Prefix Range in IGPs and the question is, should that be considered a special sub-type on LSP Ping Downstream mapping TLV? o How is the draft dealing with “controller installed SIDs”, how is that treated? · Generally, I think it may be premature to publish the draft before the “SID types” have settled to a certain degree or otherwise it has to be renamed and address a much reduced scope. Minor Issues: · “It may simplify implementations to reuse Implicit Null for Node Segment ID PHP and Adjacency Segment ID cases.” What is this saying? This is not normative, does not explain how it can be done and whether a node encountering Implicit NULL for the SIDs does consider it an error, a correct “smart” implementation or anything else? Am I missing something? · Section 5.1 (and many other sections defining formats): No error condition treatements are specified anywhere. What happens if e.g. another value than 0,1,2 is received in the protocol field? Nits: · Misspelled “supports hierarchal” · “is not hop-by-hop always” is awkward English ;-) · correction “the assignment can be unique to the node or within _a_ domain.”. We have multiple possible domains in SR (AD, area, mapping server tie-breaks) · “illustrates the problem and describe_s_ a mechanism” · “that is used to distribute a specific a label.” does not parse · “regardless of if PHP” would be better as e.g. “regardless whether” · “Adjacency Segment ID represents parallel adjacencies (Section 3.5.1 of [I-D.ietf-spring-segment-routing])”. In fact, section 3.5.1 deals with mapping server? 3.4.1 maybe? · “of &pop& operation” ? · “it can be thought _of_ as _a_ next hop destined _to a_ ? locally allocated segment that has PHP enabled.” · “When Protocol is OSPF, NP-flag defined in Section 5 of [I-D.ietf-ospf-segment-routing-extensions] should be set to 0.”. Probably MUST and in good couple of other places where normative language is not capitalized and an error on violation is implied by the section above.