Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-mpls-tp-shared-ring-protection-04.txt

From: "Weiqiang Cheng" <[email-masked]>
Date: Sat, 25 Mar 2017 16:55:29 +0800

Hi Mach,

Thanks a lot for your comments. An updated version of draft-ietf-mpls-tp-shared-ring-protection which solves your comments will be submitted when the submission is open again. 

And please see some replies inline:

> -----Original Message-----
> From: mpls [mailto:[email-masked]] On Behalf Of Mach Chen
> Sent: Monday, February 27, 2017 6:05 PM
> To: [email-masked]' <[email-masked]>
> Cc: [email-masked]' <[email-masked]>; 
> [email-masked]'
> <[email-masked]>; [email-masked]
> Subject: [mpls] RtgDir review: 
> draft-ietf-mpls-tp-shared-ring-protection-04.txt
> 
> 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.

[Authors] Some description about LSP label allocation will be added in the new version.

> 
> 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.

[Authors] Yes, other services can also be supported. In the latest version “PW” is replaced by "service".

> 
> 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.

[Authors] The sentence will be rephrased to make it clearer. 

> 4.
> Section 4.4.1
> Suggest to move Figure 11 to under "Bullet  1.  Single-node 
> interconnected rings."

[Authors]  Done. 

> 
> 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)->LS
> P1.", what's reason to add "R1cW_F&A(D)" into the switching path? 
> Seems it's redundant.

[Authors]  Agree, the redundant part will be removed. 

> 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.

[Authors] Yes, some description will be added to reflect 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.

[Authors]  The "tail end" and "head end" will be replaced with detailed description.

> 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.

[Authors] The sentence has been rephrased to make it clear.

> 9.
> Section 5.1.2
> s/sane ring/same ring

Done.

> 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.

[Authors]
As indicated by the title of 5.1.3, the RPS state is per node. 

In steering mode, a node can be in either switching or pass-through state, but cannot be both. 

> 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?

[Authors] In steering mode, upon receiving the RPS request, ring nodes which are not the destination node would enter the pass-through state, and the RPS request is relayed along the ring to the next node until it reaches the destination node of the RPS request.

> 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.

[Authors]  A sentence will be added to this section: "The RPS protocol is applicable to all the three protection modes."

> 
> 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

[Authors] This section has been restructured to describe the procedures of bidirectional failure and unidirectional failure.

> 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?
[Authors]
When a node is in pass-through state, it allows traffic to be forwarded on either the working tunnel or the protection tunnel.

As described in section 5.1.3,
For working ring tunnel, "no switch" means traffic is carried on this tunnel.
For protection ring tunnel, "no switch" means traffic is blocked on the protection tunnel, and "pass-through" means traffic can be forwarded on the protection tunnel.

> 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.
[Authors]

LW means Lockout of Working, it has been expanded when first use, and the acronym "LW" is added right after the full name.

Best regards,
Mach

---

From: Mach Chen <[email-masked]>
Date: Sat, 25 Mar 2017 09:58:57 +0000

Hi Weiqiang,

Thanks for addressing the comments, I have reviewed the update draft, I'm fine with it.

Best regards,
Mach

> From: Weiqiang Cheng [mailto:[email-masked]]
> Sent: Saturday, March 25, 2017 4:55 PM
> To: Mach Chen <[email-masked]>; [email-masked]
> Cc: [email-masked]; [email-masked];
> [email-masked]
> Subject: Re: RtgDir review: draft-ietf-mpls-tp-shared-ring-protection-04.txt
> 
> Hi Mach,
> 
> Thanks a lot for your comments. An updated version of draft-ietf-mpls-tp-
> shared-ring-protection which solves your comments will be submitted when
> the submission is open again.
> 
> > "...LSP1->R1cW_F&A(D)->R1aP_F&A(D->C->B->A)->R2cW_I(A->F->G->H-
> >I)->LS
> [Authors] In steering mode, upon receiving the RPS request, ring nodes
> which are not the destination node would enter the pass-through state, and
> the RPS request is relayed along the ring to the next node until it reaches the
> destination node of the RPS request.
> 
> > 12.
> [Authors]  A sentence will be added to this section: "The RPS protocol is
> applicable to all the three protection modes."
> 
> >
> > 13.
> [Authors] This section has been restructured to describe the procedures of
> bidirectional failure and unidirectional failure.
> 
> > 14.
> When a node is in pass-through state, it allows traffic to be forwarded on
> either the working tunnel or the protection tunnel.
> 
> For protection ring tunnel, "no switch" means traffic is blocked on the
> protection tunnel, and "pass-through" means traffic can be forwarded on the
> protection tunnel.
> 
> > c)
> > |  D  |  Idle - LW                  |  NR            |
> >
> LW means Lockout of Working, it has been expanded when first use, and the
> acronym "LW" is added right after the full name.
> 
> Best regards,
> Mach
> 
>