Subject: [Pce] Barry Leiba's No Objection on draft-ietf-pce-stateful-path-protection-10: (with COMMENT)

From: Barry Leiba via Datatracker <[email-masked]>
Date: Sat, 14 Sep 2019 14:53:13 -0700

Barry Leiba has entered the following ballot position for
draft-ietf-pce-stateful-path-protection-10: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-pce-stateful-path-protection/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks for this document.  I have only editorial suggestions.  There's no need
to reply in any detail; just please consider adopting these suggestions. 
Thanks.

— Abstract —

   Multiprotocol Label Switching Traffic
   Engineering Label Switched Paths (MPLS LSP)

Shouldn’t that be “(MPLS-TE LSPs)”?

— Section 1 —

   [RFC5440] describes PCEP for communication between a Path Computation
   Client (PCC) and a PCE or between a pair of PCEs as per [RFC4655].  A
   PCE computes paths for MPLS-TE LSPs based on various constraints and
   optimization criteria.

Even though you expanded some of these acronyms in the Abstract, they have to
be expanded again in the Introduction, because the Abstract and the document
itself each has to stand separately.

That said, “MPLS-TE” and “PCE” are in the RFC Editor’s list of common acronyms
that don’t need expansion, so you can expand them or not, as you please.  But
“PCEP” and “LSP” do need expansion here.

You should also be consistent in using “MPLS-TE” (with the hyphen), so please
check the instances of “MPLS TE” without the hyphen, and change them.  The RFC
Editor will flag this anyway, and it saves time during final editing and AUTH48
if you fix it now.

   It includes
   mechanisms to effect LSP state synchronization between PCCs and PCEs,
   delegation of control of LSPs to PCEs, and PCE control of timing and
   sequence of path computations within and across PCEP sessions and
   focuses on a model where LSPs are configured on the PCC and control
   over them is delegated to the PCE.

This is a really long sentence, and can do with being split in two.  I suggest
changing “sessions and” to “sessions.  Stateful PCE”.

   Furthermore, a mechanism to
   dynamically instantiate LSPs on a PCC based on the requests from a
   stateful PCE or a controller using stateful PCE, is specified in
   [RFC8281].

This reads oddly in passive voice, and you have a clear subject to use.  So I
suggest:

NEW
   Furthermore, [RFC8281] specifies a mechanism to
   dynamically instantiate LSPs on a PCC based on the requests from a
   stateful PCE or a controller using stateful PCE.
END

      computes the path for the protection LSP and update the PCC with

“updates”

   Note that protection LSP can be established (signaled) prior to the
   failure (in which case the LSP is said to be in standby mode
   [RFC4427] or a Primary LSP [RFC4872]) or post failure of the
   corresponding working LSP according to the operator choice/policy
   (known as secondary LSP [RFC4872]).

“a protection LSP”

I suggest changing “post failure” to “after failure”, as it reads better.

I’m not sure about the antecedent to “according to the operator choice/policy”.
 I think you mean that the establishment can be prior to failure or after
failure, according to operator choice or policy, is that right?  In that case,
the sentence isn’t worded well.  May I suggest this?:

NEW
   Note that a protection LSP can be established (signaled) before
   the failure (in which case the LSP is said to be in standby mode
   [RFC4427] or a Primary LSP [RFC4872]) or after failure of the
   corresponding working LSP (known as secondary LSP [RFC4872]).
   Whether to establish it before or after failure is according
   to operator choice or policy.
END

   [I-D.ietf-pce-association-group] introduces a generic mechanism to
   create a grouping of LSPs which can then be used to define
   associations between a set of LSPs that is equally applicable to
   stateful PCE (active and passive modes) and stateless PCE.

When I first read this I thought “that is equally applicable” applied to the
set of LSPs.  I think you mean it to apply to the generic mechanism — that is,
the generic mechanism is equally applicable.  Assuming that’s right (note
inserted comma and split sentences):

NEW
   [I-D.ietf-pce-association-group] introduces a generic mechanism to
   create a grouping of LSPs, which can then be used to define
   associations between a set of LSPs.  The mechanism is equally
   applicable to stateful PCE (active and passive modes) and stateless
   PCE.
END

— Section 3.2 —

      Protecting (P): 1 bit - This bit is as defined in Section 14.1 of
      [RFC4872] to indicate if the LSP is working or protection LSP.

At a minimum, make it “a working or protection LSP” (add the article).
Better still, “a working (0) or protection (1) LSP.”  I know it says that in
RFC 4872, but I think it makes sense to include that here.

      Secondary (S): 1 bit - This bit is as defined in Section 14.1 of
      [RFC4872] to indicate if the LSP is primary or secondary LSP.  The
      S flag is ignored if the P flag is not set.

Similarly, add the article “a”, and also consider “a primary (0) or secondary
(1) LSP.”

   If the TLV is missing, it is considered that the LSP is the working
   LSP (i.e. as if P bit is unset).

Is this really “the working LSP”, or should it be “a working LSP”?

— Section 4 —

   An LSP is associated with other LSPs with which they interact by
   adding them to a common association group via the ASSOCIATION object.

The number disagreement here is confusing, so I’m not sure what you mean to
say.  I think you mean that the other LSPs are added to the group, in which
case change “they interact” to “it interacts”.

— Section 4.2 —

   A PCC can associate a set of LSPs under its control for path
   protection purpose.

“purposes”

   PCC reports the change in association to PCE(s) via Path Computation
   Report (PCRpt) message.

Either “a Path Computation Report (PCRpt) message” or “Path Computation Report
(PCRpt) messages”.

   It is expected that both working and protection LSP are delegated

“LSPs”

— Section 4.5 —

   When the protection type is set to 1+1 or 1:N with N=1, there MUST be
…
   When the protection type is set to 1:N with N>1, there MUST be

This is a style thing, so take it or leave it as you please — it’s not wrong
the way it’s written.  It’s just that the “1:N with N=1” and “1:N with N>1”
distinction isn’t necessary, and I think it’s distracting and invites
uncertainty.  If you just made these like this:

NEW
   When the protection type is set to 1+1, there MUST be
…
   When the protection type is set to 1:N, there MUST be
END

…it would be equally correct, but also simpler and, I think, less likely to be
confusing.

— Section 5 —

   association of one LSP to another LSP across different RSVP - Traffic
   Engineering (RSVP-TE) sessions

Is it typical to have that hyphen there in the first line?  Isn’t it more
typical to write “RSVP Traffic Engineering (RSVP-TE)” without the extra hyphen?

   The information in the PPAG TLV in PCEP as received from the
   PCE, is used to trigger

Remove the comma.

---

From: Mahend Negi <[email-masked]>
Date: Tue, 17 Sep 2019 23:18:22 +0530

Hi Barry,

Many thanks for your review. Comments are incorporated in the working copy
(diff attached).

For this one comment ->
===
— Section 4.5 —

NEW

…it would be equally correct, but also simpler and, I think, less likely to
be
confusing.
===

The first sentence is for the case 1+1 and 1:1. Since RFC 4872 does
not define an explicit state 1:1, it define 1:N only this wording was
chosen. I have made this change "When the protection type is set to
1+1 or 1:1 (1:N with N=1)...".

Thanks,
Mahendra

---

From: Barry Leiba <[email-masked]>
Date: Tue, 17 Sep 2019 10:53:00 -0700

Hi, Mahend, and thanks for the response and for addressing my comments.

> 1+1 or 1:1 (1:N with N=1)...".

I understand what the text says, but my comment is about why you are
calling out "1:N with N=1" separately.  What benefit does that have
for the text?  The regular "1:N" text works perfectly whether N=1 or
N>1, so why not just let that text serve for all cases of N?  Is there
a benefit that I'm not seeing to having N=1 as a separate case?

Barry

---

From: Mahend Negi <[email-masked]>
Date: Wed, 18 Sep 2019 07:17:05 +0530

Hi Barry,

We mis-understood the last comment (section 4.5) and will updated as
suggested in the new version.

Thanks,
Mahendra

---

From: Barry Leiba <[email-masked]>
Date: Tue, 17 Sep 2019 20:47:44 -0700

Thanks!

b

On Tue, Sep 17, 2019 at 6:47 PM Mahend Negi <[email-masked]> wrote:
>
> Hi Barry,
>
> We mis-understood the last comment (section 4.5) and will updated as suggested in the new version.
>
> Thanks,
> Mahendra
>
>
>> Many thanks for your review. Comments are incorporated in the working copy (diff attached).
>>
>> For this one comment ->
>> ===
>> — Section 4.5 —
>>
>> On Sun, Sep 15, 2019 at 3:23 AM Barry Leiba via Datatracker <[email-masked]> wrote:
>>>