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

[Mahend Negi <[e-mail]>]

Hi Barry,

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

For this one comment ->
===
— 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.
===

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

On Sun, Sep 15, 2019 at 3:23 AM Barry Leiba via Datatracker <
[e-mail]> wrote:

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