Subject: [Pals] Rtgdir last call review of draft-ietf-pals-p2mp-pw-02

From: Min Ye <[email-masked]>
Date: Wed, 10 May 2017 18:15:16 -0700

Reviewer: Patrice Brissette
Review result: Has Issues

[Resending to RTG-DIR]

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-pals-p2mp-pw-02.txt
Reviewer: Patrice Brissette
Review Date: May 10, 2017
IETF LC End Date: May 12, 2017
Intended Status: Standard Track 
Summary: 
•	I have some minor concerns about this document that I think should
be resolved before publication. 
 
Comments: 
•	Please supply an overview of the draft quality and readability. 
•	Include anything else that you think will be helpful toward
understanding your review. 
Major Issues: 
•	"No major issues found." 
Minor Issues:
•	Technically, I think the draft is completed. However, it doesn’t
flow very well. Information is all over. I suggest the authors to
review the layout/flow of the document. 
 
Here are my “detailed” comments:
 
Abstract — What is the plus value on that draft? No clear
 
Many Long sentences in the text. very hard to understand and follow.
Syntax to be improved.
 
Introduction
Typo : “A reference model or a P2MP PW is depicted in Figure 1 below”
 
“In this document, we specify a method of signaling P2MP
   PW using LDP.” —> suggest to move it from intro to abstract 
 
Also, make sure the 3rd person is used. Try to a void “we” usage
 
May I suggest to have a requirement section. Requirements are all over
the document.
 
“   In case of mLDP, a Leaf-PE can decide to join the P2MP LSP at any
   time; whereas in the case of RSVP-TE, the P2MP LSP is set up by
the
   R-PE, generally at the initial service provisioning time. It
should
   be noted that local policy can override any decision to join, add
or
   prune existing or new L-PE(s) from the tree. In any case, the PW
   setup can ignore these differences, and simply assume that the
P2MP
   PSN LSP is available when needed
“
Quite complex to follow. Missing to “why” / explanation.
 
“The LDP liberal label retention mode is used“
Another requirement… is that a MAY, SHOULD, MUST?
 
“In this case, a PW status message with status
   code of 0x00000008 (Local PSN-facing PW (ingress) Receive Fault)
MUST
   also be sent to the R-PE“ 
How? The L-PE fails to join the P2MP PSN LSP.
 
Section 2.2
“   Note that since the LDP label mapping message is only sent by the
R-
   PE to all the L-PEs, it is not possible to negotiate any interface
   parameters.“
Why is that note there? Is that already been mentioned previously.
Fig.4 must  be moved to proper in the text OR create 2 subsection in
2.2
 
“As such, PW status negotiation procedure
   described in [RFC4447bis] is not applicable to P2MP PW. A node
MUST
   NOT claim to be  P2MP PW capable by sending a LDP P2MP PW
Capability
   TLV  if it is not also capable of handling PW status“
 
Should a node send LDP P2MP PW Capability TLV or not? Not well explain

There is some reference to LSR in the text where the major part use
the wording “node”.
 
Nits: 
N/A
 
Regards,
Patrice Brissette

---

From: Sami Boutros <[email-masked]>
Date: Mon, 15 May 2017 01:52:57 +0000

Hi Patrice,

Thanks for reviewing the draft.

Please see comments inline.

On 5/10/17, 6:15 PM, "Min Ye" <[email-masked]> wrote:

>https://urldefense.proofpoint.com/v2/url?u=http-3A__trac.tools.ietf.org_area_rtg_trac_wiki_RtgDir&d=DwIDaQ&c=uilaK90D4TOVoH58JNXRgQ&r=IVzcTRLQdpta08L0b_y2zDkqvwJhRKMCAbX-2K-LV98&m=MWIXfe7E_gFwWZFrsVPpNTa6EQimCGFx-G3sIjMLbMg&s=tA3NumQERHERuYDYA0CCprq36__MHGLfZU3E8YypvVg&e=  

I went over the abstract, I didn’t see any long sentences. Not sure 
What to improve? Can you be specific?

>   PW using LDP.” —> suggest to move it from intro to abstract

Not sure if we can reference a figure in the abstract. The abstract
Already mention that the second sentence.

>Also, make sure the 3rd person is used. Try to a void “we” usage

Agreed, I will remove all usage of “we” in the document.

>May I suggest to have a requirement section. Requirements are all over

There is already an RFC for that. [RFC7338]   F. Jounay, et. al, 
"Requirements for Point to Multipoint Pseudowire", RFC7338, September 2014.

This solution document addresses the requirements.

>Quite complex to follow. Missing to “why” / explanation.

Sure I can clarify this a little more, will remove some sentences 
That make it confusing, we are simply here differentiating mLDP LSP from
p2mp LSP w/ RSVP-TE and saying that PW setup is agnostic of the transport 
p2mp LSP setup.

>Another requirement… is that a MAY, SHOULD, MUST?

I will change it to a MUST.

>How? The L-PE fails to join the P2MP PSN LSP.

Correct the L-PE have to signal this failure to the root PE.

>Why is that note there? Is that already been mentioned previously.

This is the only reference in the document.

>Fig.4 must  be moved to proper in the text OR create 2 subsection in

Sorry didn’t get what you mean here? Can you elaborate?

>Should a node send LDP P2MP PW Capability TLV or not? Not well explain

What is said here, that you can’t be P2MP PW capable without being PW status capable.
Not sure how to make it clearer.

>There is some reference to LSR in the text where the major part use

I will make all consistent, and use LSR instead of node.

Thanks,

Sami

---

From: "Patrice Brissette (pbrisset)" <[email-masked]>
Date: Mon, 15 May 2017 16:02:51 +0000

Hey Sami,

Please see below. Look for <Patrice>

Regards,
Patrice Brissette

On 2017-05-14, 9:52 PM, "Sami Boutros" <[email-masked]> wrote:

    Hi Patrice,

    
    >Many Long sentences in the text. very hard to understand and follow.

<Patrice> This comment is regarding the draft in general. It doesn’t flow well. You need to read it more than once to see the overall picture. It might just be to reshuffle some sections.

    

<Patrice> This is NOT a figure but rather the explanation. That line makes the document very clear. It just need to be spelled out. “specify a method of signaling P2MP PW using LDP”.

    

<Patrice> Your document enhances that based RFC by providing more “MUST”, “SHOULD”, etc. 
They are all over the doc. To make it clear, grouping them in a section may help. Another idea is to have Requirement sub-sections per topic.

    

    

<Patrice> question remains, how? If L-PE fails to join the LSP, PW will be down. How can it signals the failure to root PE?

<Patrice> Forgot my thoughts on that one.

<Patrice> Sorry …Let me try again. “P2P PW Downstream FEC Element”. I think you should have a section just on that topic. Actually, maybe you can create a subsection for each different FEC explained in the document. 

<Patrice> right… I must have been tired. Too many NOT 

    
    Thanks,
    
    Sami

---

From: Sami Boutros <[email-masked]>
Date: Thu, 18 May 2017 16:25:46 +0000

Hi Patrice,

><Patrice> This comment is regarding the draft in general. It doesn’t flow well. You need to read it more than once to see the overall picture. It might just be to reshuffle some sections.

Sure will look into that.

><Patrice> This is NOT a figure but rather the explanation. That line makes the document very clear. It just need to be spelled out. “specify a method of signaling P2MP PW using LDP”.

This is exactly what we have in the abstract first sentence “This document specifies a mechanism to signal Point-to-Multipoint
   (P2MP) Pseudowires (PW) tree using LDP.”
 
>They are all over the doc. To make it clear, grouping them in a section may help. Another idea is to have Requirement sub-sections per topic.

It seems that this is the same as your document flow comment above, I will look into that.

><Patrice> question remains, how? If L-PE fails to join the LSP, PW will be down. How can it signals the failure to root PE?

L-PE will use the PW status message, to signal to root PE this is what the text is saying.

><Patrice> Sorry …Let me try again. “P2P PW Downstream FEC Element”. I think you should have a section just on that topic. Actually, maybe you can create a subsection for each different FEC explained…

Ok, I will add subsections for Downstream and Upstream.

Thanks,

Sami

---

From: "Patrice Brissette (pbrisset)" <[email-masked]>
Date: Thu, 18 May 2017 19:44:22 +0000

Thank Sami

Regards,
Patrice Brissette

On 2017-05-18, 12:25 PM, "Sami Boutros" <[email-masked]> wrote:

    Hi Patrice,

    

    

    

    

    Thanks,
    
    Sami