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

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 17 Sep 2019 16:44:42 -0700

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?

(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?

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

(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?

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

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

(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?

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

   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?

      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?

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?

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.

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

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?

Section 10.2

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

---

From: Dhruv Dhody <[email-masked]>
Date: Wed, 18 Sep 2019 11:45:23 +0530

Hi Ben,

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

> https://datatracker.ietf.org/doc/draft-ietf-pce-stateful-path-protection/

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.

> 4872 to allocate a new type know to consider/document whether it applies

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

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

> The following table only lists Error-values.  What Error-type(s) should

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

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

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

> the source/destination/tunnel-ID do?

Yes. Authors should make that explicit.

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.

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

> actively carrying traffic?

Yes, Working LSPs are always primary.

> Is this conditional on the LSP being part of a PPAG, or does it hold

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

> need for source/destination/tunnel-ID to match.

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

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

> populating the PROTECTION object that corresponds to the protection

Yes.

> I think RFCs 7525 and 8253 need to be normative (and please cite RFC

Thanks!
Dhruv

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 19 Sep 2019 16:02:43 -0500

Hi Dhruv,

> Thanks for your review, let me start the discussion and the authors

Thanks; it's a great start.

> diversity association in Section 5.

A naive reading of the linked section would be that it applies only to the
"Disjointness Association Type" defined in that document.  I'm not in a
position to claim that the reasoning in that document does not apply to
this one, but I think we should make a more clear link between the Path
Protection ASsociation Type and the discussion in that document.

> object is acceptable in this TLV unless explicitly stated otherwise."

To be honest, that's probably good enough in practice, since it seems
somewhat unlikely that the last bit is ever going to get allocated (though,
I guess technically the field could be converted to an integer instead of
flags).  I do still have to wonder how the reader would know *where* it
would be stated otherwise, and how the author of a document defining a new
type would know to consider our use case as well as the RFC 4872 use case.

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

Wording it right might be a little finicky, but that's fine in general.

Thanks!

> multiple ASSOCIATION Object for each PPAG with each of them having a

Sounds good, and quite natural given (5).

> Yes. Authors should make that explicit.

Thanks

> an error handling for unknown/unsupported association-type to take

Okay, thanks for giving me the backstory.

> operator-configured association, this one is dynamic.

Ah, of course, I had forgotten about that division of configuration.

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

It may be worth making the text explicit about it.

> So the error in this section are focused only when one adds "another".

I don't think that the points you cover above are what I was concerned
about -- I was thinking more along the lines of a "make-before-break"
scheme for changing what LSP is the protection LSP for a given working LSP.
Maybe given the timescales involved it's not an issue in practice, though.

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

(I don't think that's needed.)

Thanks!

-Ben

---

From: Mahend Negi <[email-masked]>
Date: Sun, 22 Sep 2019 21:06:24 +0530

Hi Ben/Dhruv,

Thanks for the review and clarification. Comments are incorporated in the
working copy (links below).

Working copy:
https://github.com/mahendnegi/ietf/blob/master/pce/path-protection/iesg/draft-ietf-pce-stateful-path-protection-11.txt
Diff:
https://github.com/mahendnegi/ietf/commit/c51efb2b025d545e066b9b13de6d84de48d1a03c

Please do share your opinion.

Thanks,
Mahendra

On Fri, Sep 20, 2019 at 2:33 AM Benjamin Kaduk <[email-masked]> wrote:

> > > Please refer to
> https://www.ietf.org/iesg/statement/discuss-criteria.html

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 23 Sep 2019 17:48:05 -0700

Hi Mahendra,

Thanks for the links.  What's in the diff generally looks good; I think we
may still need to have a bit more discussion here about items (1) and (4)
(for the latter, I'm not sure if the plan is still to update the table to
list everything or just leave it to the body text).

Also, with respect to (7), I think there was at least one place in the text
where we mention that the source, destination, and tunnel ID must match
where we don't also mention the protection type matching.  Maybe it's
sufficiently less important that we don't need to mention it everywhere,
but I'll mention it just in case.

Thanks,

Ben

---

From: Dhruv Dhody <[email-masked]>
Date: Tue, 24 Sep 2019 11:38:13 +0530

Hi Ben,

Apologies for the delay. I am replying this email where we tackle
points that needed some more discussion.

> Protection ASsociation Type and the discussion in that document.

The direct relationship is between the 'Disjointness Association Type'
and SVEC. But it is natural to include both Protection association and
disjoint association together. If SVEC is also present the
relationship is as per the Section 5.3 of
[I-D.ietf-pce-association-diversity]. Suppose if only protection
association is present and SVEC object, there is no direct
relationship and both object would be processed.

I suggest we add this text to make sure reader is aware of SVEC - "The
relationship between the Synchronization VECtor (SVEC) object and the
Disjointness Association is described in section 5.3 of
[I-D.ietf-pce-association-diversity]."

> type would know to consider our use case as well as the RFC 4872 use case.

I guess such an update would happen in TEAS; and historically PCE and
TEAS work very closely, as well as share the same AD. I think we are
quite safe here.

In case you were wondering why flags - we chose PT as flags as per RFC
4872 where this is called "LSP (Protection Type) Flags".

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

Lets update the table as well. Request authors to update.

> > Yes. Authors should make that explicit.

Sure. Request authors to add.

> Maybe given the timescales involved it's not an issue in practice, though.

Maybe worth adding some text here to be safe - "During the
make-before-break (MBB) procedure, two paths will briefly coexist. The
error handling related to number of LSPs allowed in a PPAG MUST NOT be
applied during MBB."

I hope this message clears all the remaining points.

Thanks!
Dhruv

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 25 Sep 2019 14:17:45 -0700

Thanks!

Thanks!

Thanks!

> I hope this message clears all the remaining points.

I think it does; thanks again.

-Ben