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

"Weiqiang Cheng" <chengweiqiang@chinamobile.com> Sat, 25 March 2017 08:55 UTC

Return-Path: <chengweiqiang@chinamobile.com>
X-Original-To: mpls@ietfa.amsl.com
Delivered-To: mpls@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8A2AE124281; Sat, 25 Mar 2017 01:55:43 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.6
X-Spam-Level:
X-Spam-Status: No, score=-2.6 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, RP_MATCHES_RCVD=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Ngvu_4ad6dqs; Sat, 25 Mar 2017 01:55:40 -0700 (PDT)
Received: from cmccmta1.chinamobile.com (cmccmta1.chinamobile.com [221.176.66.79]) by ietfa.amsl.com (Postfix) with ESMTP id 96CA61293FB; Sat, 25 Mar 2017 01:55:37 -0700 (PDT)
Received: from spf.mail.chinamobile.com (unknown[172.16.121.9]) by rmmx-syy-dmz-app02-12002 (RichMail) with SMTP id 2ee258d63082635-0b645; Sat, 25 Mar 2017 16:55:30 +0800 (CST)
X-RM-TRANSID: 2ee258d63082635-0b645
X-RM-SPAM-FLAG: 00000000
Received: from cmcc (unknown[183.240.196.121]) by rmsmtp-syy-appsvr05-12005 (RichMail) with SMTP id 2ee558d630812c5-1ea35; Sat, 25 Mar 2017 16:55:30 +0800 (CST)
X-RM-TRANSID: 2ee558d630812c5-1ea35
From: "Weiqiang Cheng" <chengweiqiang@chinamobile.com>
To: "'Mach Chen'" <mach.chen@huawei.com>, <rtg-ads@ietf.org>
Cc: <rtg-dir@ietf.org>, <draft-ietf-mpls-tp-shared-ring-protection@ietf.org>, <mpls@ietf.org>
References: <F73A3CB31E8BE34FA1BBE3C8F0CB2AE28FDCEF54@SZXEMA510-MBX.china.huawei.com>
In-Reply-To: <F73A3CB31E8BE34FA1BBE3C8F0CB2AE28FDCEF54@SZXEMA510-MBX.china.huawei.com>
Date: Sat, 25 Mar 2017 16:55:29 +0800
Message-ID: <008501d2a545$8e3f3840$aabda8c0$@com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Microsoft Office Outlook 12.0
Thread-Index: AQHSjcZujP4pQeKD5EC3MI1pw61Qg6GlbAVw
Content-Language: zh-cn
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/DSnJUat9ZzIxn22en2WBFZCivG4>
Subject: Re: [mpls] RtgDir review: draft-ietf-mpls-tp-shared-ring-protection-04.txt
X-BeenThere: mpls@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Multi-Protocol Label Switching WG <mpls.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mpls>, <mailto:mpls-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mpls/>
List-Post: <mailto:mpls@ietf.org>
List-Help: <mailto:mpls-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mpls>, <mailto:mpls-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 25 Mar 2017 08:55:43 -0000

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:mpls-bounces@ietf.org] On Behalf Of Mach Chen
> Sent: Monday, February 27, 2017 6:05 PM
> To: 'rtg-ads@ietf.org'; <rtg-ads@ietf.org>;
> Cc: 'rtg-dir@ietf.org'; <rtg-dir@ietf.org>;; 
> 'draft-ietf-mpls-tp-shared-ring-protection@ietf.org';
> <draft-ietf-mpls-tp-shared-ring-protection@ietf.org>;; mpls@ietf.org
> 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