Re: [Pce] Benjamin Kaduk's Discuss on draft-ietf-pce-stateful-path-protection-10: (with DISCUSS and COMMENT)

[Dhruv Dhody <[e-mail]>]

Hi Ben,

Thanks for your review, let me start the discussion and the authors
can interject as needed.

On Wed, Sep 18, 2019 at 5:14 AM Benjamin Kaduk via Datatracker
<[e-mail]> wrote:
>
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-pce-stateful-path-protection-10: Discuss
>
> 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/
>
>
>
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
> (1) draft-ietf-pce-association-group notes that "PCEP extensions that define
> a new association type should clarify the relationship between the SVEC
> object and the association type, if any".  Where do we do so for the
> path protection association type?
>

This is discussed in
https://tools.ietf.org/html/draft-ietf-pce-association-diversity-10#section-5.3,
where it is more relevant. This draft includes the relationship with
diversity association in Section 5.

> (2) Section 3.2 says:
>
>       Protection Type (PT): 6 bits - This field is as defined in
>       Section 14.1 of [RFC4872] to indicate the LSP protection type in
>       use.
>
> There doesn't seem to be a registry created by RFC 4872 to track these
> PT values, so I assume that the way to allocate a new value is
> "standards-track RFC that Updates: RFC 4872".  Is that also the way to
> allocate new PT values for PPAG usage?  How would someone updating RFC
> 4872 to allocate a new type know to consider/document whether it applies
> for PPAG usage?
>

That's true. Maybe we can add this text - "Any type already defined or
that could be defined in the future for use in the RSVP-TE PROTECTION
object is acceptable in this TLV unless explicitly stated otherwise."

> (3) In Section 4.3:
>
>    A PCE can create/update working and protection LSPs independently.
>    As specified in [I-D.ietf-pce-association-group], Association Groups
>    can be created by both the PCE and the PCC.  Further, a PCE can
>
> The requirement that source, destination, and tunnel ID of all LSPs
> within a PPAG MUST be the same is new to this document, so I think we
> need to specify error handling for when attempts to update LSPs
> independently would violate that invariant (presumably in Section 4.5).
>

In the existing error case in Section 4.5 we could make it "add or update".

> (4) In Section 6.3:
>
>                                       IANA is requested to allocate new
>    error values within the "PCEP-ERROR Object Error Types and Values"
>    sub-registry of the PCEP Numbers registry, as follows:
>
> The following table only lists Error-values.  What Error-type(s) should
> they be associated with?
>

It is mentioned in the text, but worth making it explicit in the table as well.

> (5) We don't say which objects the PPAG TLV can appear in.  (Section 3.2
> says "[t]he Path Protection Association TLV is an optional TLV for use
> with the Path Protection Association Type", but it's hard to interpret
> that as meaning "for use [only] with the ASSOCIATION object defined in
> draft-ietf-pce-association-group", especially since there is a "path
> protection association type" already (and it's a codepoint in the
> "ASSOCIATION Type Field" registry).
>

We can update to - "The Path Protection Association TLV is an optional
TLV for use in the ASSOCIATION Object with the Path Protection
Association Type."

> (6) I'm not sure if a change to the document is needed here, but perhaps
> some discussion is in order: we say that a given LSP can belong to more
> than one PPAG, but only allow one PPAG TLV per [some context that
> remains unclear; see (5)].  I don't have a good handle for whether these
> two requirements are potentially in conflict: that is, whether a single
> PPAG TLV would have to specify the flags that apply for both PPAGs that
> a given LSP is a member of, or if the containing objects serve to scope
> the PPAG TLV flags' interpretation.
>

When a given LSP belong to more than one PPAG, we expect to have
multiple ASSOCIATION Object for each PPAG with each of them having a
single PPAG TLV.

> (7) Do the Protection Type fields of the PPAG TLV in the various LSPs
> that are members of the same PPAG need to match, in the same way that
> the source/destination/tunnel-ID do?
>

Yes. Authors should make that explicit.

>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Section 1
>
>    This document specifies a stateful PCEP extension to associate two or
>    more LSPs for the purpose of setting up path protection.  The
>    proposed extension covers the following scenarios:
>
> nit: after publication, it doesn't really seem like a *proposed*
> extension anymore.
>
> Section 3.1
>
>    LSPs are not associated by listing the other LSPs with which they
>    interact, but rather by making them belong to an association group
>    referred to as "Path Protection Association Group" (PPAG) in this
>    document.  [...]
>
> The first 2/3 of this sentence is a (true) generic statement about all
> association groups, which exist outside of this document in a generic
> fashion.  I strongly suggest rewording this to make clear that the PPAG
> is the specific association (group) type used for this document's
> functionality but that other association groups are possible.  The
> current wording implies that the PPAG is the only association possible,
> and it is just given a special name for this document's purposes.
>
>    [I-D.ietf-pce-association-group] specifies the mechanism for the
>    capability advertisement of the Association types supported by a PCEP
>    speaker by defining a ASSOC-Type-List TLV to be carried within an
>    OPEN object.  This capability exchange for the Association type
>    described in this document (i.e.  Path Protection Association Type)
>    MAY be done before using the policy association, i.e., the PCEP
>    speaker MAY include the Path Protection Association Type (TBD1) in
>    the ASSOC-Type-List TLV before using the PPAG in the PCEP messages.
>
> Why is this only MAY and not MUST?
>

In [I-D.ietf-pce-association-group] we explicitly did not mandate the
capability exchange for all association types and let each type decide
for itself. This association type fits into where the capability
exchange is optional. When the capability exchange was added in
[I-D.ietf-pce-association-group], there was a desire from the WG to
keep it optional for already defined/implemented protection
association type. Also this is a basic feature in path computation and
it is expected to be supported universally. In a corner case there is
an error handling for unknown/unsupported association-type to take
care of it.

>    This Association type is dynamic in nature and created by the PCC or
>
> nit: is it the type that is dynamic or the associations of that type?
>

It is the type. [I-D.ietf-pce-association-group] has dynamic or
operator-configured association, this one is dynamic.

>       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.
>
>       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.
>
> Just to check my understanding (no change expected to result): if P is
> zero, the LSP is working, and thus we know that it's primary since it's
> actively carrying traffic?
>

Yes, Working LSPs are always primary.


> Section 3.2
>
> nit(?) The section heading does not match the name of the TLV requested
> in the IANA considerations, nor do we mention here that "PPAG TLV" is
> synonymous with it.
>
>    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 conditional on the LSP being part of a PPAG, or does it hold
> generically as well?
>

IMHO when part of a PPAG i.e. association object but no TLV.

> Section 4.5
>
>    As per the processing rules specified in section 5.4 of
>    [I-D.ietf-pce-association-group], if a PCEP speaker does not support
>
> There is no Section 5.4 in draft-ietf-pce-association-group-10;
> presumably this was supposed to be Section 6.4?
>
>    A given LSP MAY belong to more than one PPAG.  If there is a conflict
>
> While I'm not arguing to remove this option, I'm also a bit confused at
> how useful having a single LSP be in multiple PPAGs woud be, given the
> need for source/destination/tunnel-ID to match.
>

One working LSP could have multiple protection LSPs with each
belonging to a different association.

>    When the protection type is set to 1+1 or 1:N with N=1, there MUST be
>    only one working LSP and one protection LSP within a PPAG.  If a PCEP
>    speaker attempts to add another working/protection LSP, the PCEP peer
>    MUST send PCErr with Error-Type 26 (Association Error)
>    [I-D.ietf-pce-association-group] and Error-Value TBD4 (Attempt to add
>    another working/protection LSP for Path Protection Association).
>
> This seems to prevent a scenario where there's a need to change the
> protection LSP and a desire to do so without removing the protected
> nature of the working LSP.  Unless it's presumed that this could always
> be done by updating the protection LSP and it would never be necessary
> to create a new one?  (Similarly for 1:N, though maybe less severe.)
>

One can remove the protection LSP and keep the association intact. The
error will not be triggered.
Also, PCEP messages are per LSP, when the first LSP is reported with
association, it is bound to be the only LSP in the association.
So the error in this section are focused only when one adds "another".

Maybe we can add "..at maximum.." to make it clear?

> Section 5
>
>    [RFC4872] introduces the concept and mechanisms to support the
>    association of one LSP to another LSP across different RSVP - Traffic
>    Engineering (RSVP-TE) sessions using ASSOCIATION and PROTECTION
>    object.  The information in the PPAG TLV in PCEP as received from the
>    PCE, is used to trigger the signalling of working LSP and protection
>    LSP, with the Path Protection Association Flags mapped to the
>    corresponding fields in the PROTECTION Object in RSVP-TE.
>
> Just to check my understanding: this paragraph is saying that the
> contents of the PPAG TLV received by the PCC is used as input to
> populating the PROTECTION object that corresponds to the protection
> group?
>

Yes.

> Section 10.2
>
> I think RFCs 7525 and 8253 need to be normative (and please cite RFC
> 7525 as BCP 195).
>
>

Thanks!
Dhruv