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

[Barry Leiba <[e-mail]>]

Thanks!

b

On Tue, Sep 17, 2019 at 6:47 PM Mahend Negi <[e-mail]> wrote:
>
> Hi Barry,
>
> We mis-understood the last comment (section 4.5) and will updated as suggested in the new version.
>
> Thanks,
> Mahendra
>
>
> On Tue 17 Sep, 2019, 23:18 Mahend Negi, <[e-mail]> wrote:
>>
>> 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.
>>>
>>>