Subject: [Pce] Benjamin Kaduk's No Objection on draft-ietf-pce-association-bidir-12: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Wed, 17 Feb 2021 22:58:31 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-pce-association-bidir-12: 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-association-bidir/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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

---

From: "Rakesh Gandhi (rgandhi)" <[email-masked]>
Date: Fri, 19 Feb 2021 21:51:08 +0000

Thank you Benjamin for the review comments.

We have posted a new revision that address the comments:
URL:            https://www.ietf.org/archive/id/draft-ietf-pce-association-bidir-13.txt
Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-association-bidir-13

Please see inline with <RG>…

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Thursday, February 18, 2021 at 1:58 AM
To: The IESG <[email-masked]>
Cc: [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>
Subject: Benjamin Kaduk's No Objection on draft-ietf-pce-association-bidir-12: (with COMMENT)
Benjamin Kaduk has entered the following ballot position for
draft-ietf-pce-association-bidir-12: No Objection

<RG> We have added Section 3.5 - Operational Considerations

<RG> Please advise your feedbacks.

Section 4.1

<RG> Fixed - "using the generic ASSOCIATION Object"

<RG> Fixed.

<RG> Added TLV.

Section 4.2

<RG> The draft does not have specific check except the new error code should help with the error.

<RG> Good idea, merged into one flag to simplify.

Section 5.1

<RG> Fixed.

Section 5.2

<RG> Fixed.

Section 5.4

<RG> Yes.

Section 5.5

<RG> I changed the previous sentence before this snippet to cover this case.

Section 5.7

<RG> Right.

Section 9.2.1

<RG> Thanks.

Section 10.2

<RG> Ok, Changed RFC 8253 to normative.
Thanks,
Rakesh