Path Computation Element Communication Protocol (PCEP) Extensions for Associated Bidirectional Label Switched Paths (LSPs)
draft-ietf-pce-association-bidir-14

I agree with Barry that the Abstract in its current form feels kind of cluttered.  Anything we could do to tidy that up would help.  I wonder how it might look with all the acronyms removed, and instead introduce them down in the Introduction or later sections.

Alvaro also raises a good point, and I'd like to see that addressed somehow.

Thank you to Chris Lonvick for the SECDIR review.

Thanks to the OpsDir reviewer (Al Morton) for reviewing and providing suggestions on this document, and the author for working though them.

Thank you for the work put into this document. I must admit that this document was too deep in PCE for a full review of mine, I am trusting my routing AD peers for this review.

Please find below some non-blocking COMMENT points (but replies would be appreciated), and some nits.

I hope that this helps to improve the document,

Regards,

-éric

== COMMENTS ==
-- Section 3 --
Figure 2, should there be a LSP2 label on the link between C and F (as well as between E and B) ? At the risk of overloading the figure though...

-- Section 3.1.1 --
Suggest to mention that the topology of figure 1 is reused.

== NITS ==

-- Section 1 --
Is "Path Control Element (PCE)" correct or is it "Path Computation Element (PCE) " (per RFC Editor abbreviation list) ?

This document defines extensions that can be used in different modes of operation (§3.4).  However, there is no operational guidance related to the advantages/disadvantages or considerations of using each of them.  Are there cases (beyond implementation support) when one mode could be preferred?  If all modes are supported, how should an operator choose?

I believe that the specification is incomplete without this type of guidance, but I am not making this point a DISCUSS hoping that it will be easy for the authors to address.

Thanks for an easy read.  I just have two very small comments:

— Abstract —

   The Path Computation Element Communication Protocol (PCEP) provides
   mechanisms for Path Computation Elements (PCEs) to perform path
   computations in response to Path Computation Clients (PCCs) requests.
   The Stateful PCE extensions allow stateful control of Multiprotocol
   Label Switching (MPLS) Traffic Engineering (TE) Label Switched Paths
   (LSPs) using PCEP.

Hm.  I’m not clear here: Does this have something to do with path computation?

He-he... seriously, I understand the repetition, given the expansion of the abbreviations.  What I wonder is whether it’s necessary to put all those terms into the Abstract, given that the expansion of "PCEP" already includes "path computation element".  What do you think about shortening the Abstract thus?:

SUGGESTION
   This document defines Path Computation Element Communication Protocol
   (PCEP) extensions for grouping two unidirectional MPLS-TE Label
   Switched Paths (LSPs), one in each direction in the network, into an
   Associated Bidirectional LSP.  The mechanisms defined in this
   document can be applied using a Stateful PCE for both PCE-Initiated
   and PCC-Initiated LSPs, as well as when using a Stateless PCE.  The
   procedures defined are applicable to the LSPs using RSVP-TE for
   signaling.
END

I note that "MPLS-TE", "PCE", and "RSVP-TE" are all in the RFC Editor’s list of abbreviations that don’t need expansion... though, of course, you can put the expansions back in if you prefer.  I also note that "PCC" is not, but I think it would be awkward to include the expansion of "PCC" here, so maybe we can manage without it in the Abstract.

— Section 3.1 —

   Both endpoint nodes act as a PCC.

Nit: "Both" is plural, so either "Both endpoint nodes act as PCCs." or "Each endpoint node acts as a PCC."

Alvaro makes a good point, and I'm interested in seeing the updates
made in response to it.

Section 4.1

   As per [RFC8697], LSPs are associated by adding them to a common
   association group.  This document defines two new Association Types,
   called "Single-sided Bidirectional LSP" (TBD1) and "Double-sided
   Bidirectional LSP" (TBD2), based on the generic ASSOCIATION Object
   ((Object-Class value 40).  [...]

(nit) pedantically I might say that these are instances of the
ASSOCIATION object, as "based on" carries some connotation that they are
distinct but similar.  So this might become "using the generic
ASSOCIATION Object" or "for the generic ASSOCIATION Object".

   o  The Tunnel (as defined in [RFC3209]) containing the forward and
      reverse LSPs of the Single-sided Bidirectional LSP Association on
      the originating node MUST have the same LSP parameters (as
      described in section 3.1.1 of [RFC3209]), albeit both LSPs have
      reversed endpoint nodes.

There is no Section 3.1.1 of RFC 3209 (and it hardly talks about
"parameters", either).

   The Association ID, Association Source, optional Global Association
   Source and optional Extended Association ID in the Bidirectional LSP
   Association Object are initialized using the procedures defined in
   [RFC8697] and [RFC7551].

(nit?) Is it better to talk about Global Association Source and Extended
Association IDs as being TLVs?

Section 4.2

The new TLV has a flag to indicate the forward LSP, but the
forward/reverse directionality is also indicated by the respective node
addresses (for double-sided bidirectional LSPs); is there any protocol
element that should check and enforce consistency of the two
representations?

[I was going to ask about some other error handling cases here, but I
see that Section 5.7 already covers many of them -- thank you!]

If I understand correctly, the F and R flags are mutually exclusive.
Can/should that also be enforced (and with what error handling)?  I
don't think I understand why they are separate bits instead of just a
single bit (e.g., 1 for forward and 0 for reverse).

Section 5.1

Some nits in the last four bullet points: IIUC all should start with the
plural "Stateful PCEs" (for consistency), and then the verbs in the last
two would have to be adjusted to match, so "Stateful PCEs establish and
remove", and "Stateful PCEs create and update".

Section 5.2

Similarly (also nit-level), the bullet points here should probably all
use the "PCCs" plural form for consistency, and some verb conjugations
would need adjustment to match.

Section 5.4

Just to check my understanding: this text is saying that whenever you
use the bidirectional LSP association group, you also set the B flag in
the request parameters?

Section 5.5

                                               There is no change in the
   PLSP-ID allocation procedure for the forward LSP of the Single-sided
   Bidirectional LSP on the originating endpoint node.  In case of
   Double-sided Bidirectional LSP Association, there is no change in the
   PLSP-ID allocation procedure.

This snippet carefully does not say anything about the PLSP-ID
allocation procedures for the reverse LSP of a single-sided
bidirectional association.  Do those procedures change?

Section 5.7

   If a PCE speaker receives an LSP with a Bidirectional LSP Association
   Type that it does not support, the PCE speaker MUST send PCErr with
   Error-Type = 26 (Association Error) and Error-Value = 1 (Association
   Type is not supported).

This reads a little bit (but not entirely) like it's binding the
behavior of implementations that don't implement this specification,
which generally doesn't make sense to attempt to do.
(But IIRC this is the behavior required by the core spec anyway, so it
wouldn't really matter.)

Section 9.2.1

   o  Bit number (count from 0 as the most significant bit)

Thank you for including this detail!

Section 10.2

The "RECOMMENDED" for RFC 8253 usage might arguably promote it to
normative (per
https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/).

The figures in Section 3 repeatedly state use the phrase "reports the reverse LSP1
   (not shown for simplicity)". Not being an expert in these technologies, I found the consistent omission of this from the figure to be confusing, not clarifying.