Hello, 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 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: draft-ietf-mpls-tp-shared-ring-protection-04.txt Reviewer: Mach Chen Review Date: 2017/2/23 IETF LC End Date: Intended Status: Standards Track Summary: I have some minor concerns about this document that I think should be resolved before publication. Comments: This document may need some paragraph restructuration or adjustment to make it more readable, for example , before jumping into the detail of some processes, it's better to give some introduction to the scenarios and relevant conditions, and the used terms need to be defined before being used. Major Issues: No major issues found. Minor Issues: 1. The implicit fundamental pre-condition of the proposed MSRP is that, for each protected LSP, the egress and the ingress LSR have to negotiate (or through NMS, or static configurations) the LSP label that will be pushed by the ingress. The label has to be a label that the egress must know how to handle it, otherwise, when the packet exits the ring tunnel, the packet will be mis-forwarded to non-intended node or dropped. This condition and probably the relevant mechanisms should be explicitly pointed out and described in the document. 2. Section 4.1 The Figure of "the logical layers of the ring" shows the innermost service is PW, is this document only applies to PW or other applications will be included. If it's later, some modifications are needed, and you need to check the whole document to make the relevant descriptions and usages consistent. Figure 2 has the same problem. 3. Section 4.1.3 s/The clockwise working ring tunnel label RcW_D/ The clockwise working ring tunnel label of RcW_D And since RcW_D identifies a ring tunnel, not a label, and the author seems intend to use RcW_D(X) to identify a ring tunnel label, I'd suggest to add some clarification text to this notation. 4. Section 4.4.1 Suggest to move Figure 11 to under "Bullet 1. Single-node interconnected rings." 5. Section 4.4.4, the penultimate paragraph "...LSP1->R1cW_F&A(D)->R1aP_F&A(D->C->B->A)->R2cW_I(A->F->G->H->I)->LSP1.", what's reason to add "R1cW_F&A(D)" into the switching path? Seems it's redundant. 6. Section 4.4.3, In my understanding, the ring tunnels to virtual interconnected group are shared by all LSPs that needed to be forwarded to other rings, right? If so, it's better to add some text to explicitly state this. 7. Section 5.1 " When the failure has cleared and the Wait-to-Restore (WTR) timer has expired, the nodes sourcing RPS requests MUST drop their respective switches (tail end) and MUST source an RPS request carrying the NR code. The node receiving from both directions such an RPS request (head end) MUST drop its protection switches." Above text introduces the "tail end" and "Head end" tems, but the document does not have detail definition and introduction to these terms. Suggest to give the definition of these term in the terminology and notation section. 8. Section 5.1 "A destination node is a node that is adjacent to a node that identified a failed link." Based on the above text, the "destination node" cannot be uniquely determined, since a node in a ring has two adjacent nodes. Some clarification or more accurate description is needed. 9. Section 5.1.2 s/sane ring/same ring 10. Section 5.1.3. Ring Node RPS States Are those RPS states per node or per ring tunnel or per egress? Whatever for either case, the document should state it explicitly. And according to the explanation of each state, seems that the states are mutex each other. Considering the steering mode, seems that a node may be at both switching and pass-through state, right? BTW, for those RPS codes, it's better to add a reference to the section (Section 5.2.1.1) to give a hint to the reader that there will be detailed explanation in the following sections. 11. Section 5.1.3.2 "A node in the switching state MUST terminate RPS requests flow in both directions." If a node in the switching state terminates RPS requests, how does the steering switching mode work? 12. Section 5.1. RPS Protocol "The described RPS protocol uses the short- wrapping mechanism described in section 4.3.2 as an example." Are there any differences when the RPS protocol used for wrapping and steering, if no, it should be stated clearly. Otherwise, it needs to specify the differences, or for each switching mode, define its procedures and state machine. 13. Section 5.1.3.2. Switching State, The third paragraph describes the rules in case of unidirectional failure, but reader can not recognized this only when read the next paragraph. This section needs some restructure to make it more readable. BTW, since there are differences between unidirectional failure and bidirectional failure, there need some text to describe the rules and procedures for the bidirectional case IMHO 14. Section 5.2.2. Initial States a) It just lists a table and let the reader to guess the meaning, it's better to add some clarification text to help the reader understand the meaning and intention. b) In addition: | B | Pass-through | N/A | | | Working: no switch | | | | Protection: pass through | | For pass-through state, why the working and protection tunnel states are different? c) | D | Idle - LW | NR | What is the LW? I guess it means "Lock of Working", but it needs to be expanded it when first use. Best regards, Mach