Carrying Binding Label/Segment Identifier (SID) in PCE-based Networks.
draft-ietf-pce-binding-label-sid-16

[S4.1; question]

* For clarity, when the locator prefix is a /48 and the nodes are each
  allocated a /64, which of these is expected:

    [a] {LB=48, LN=64, FUN=..., ARG=...}, or

    [b] {LB=48, LN=16, FUN=..., ARG=...}

    (where LB + LN = 64)?

In Section 1:

   This document specifies an extension to PCEP to manage of binding
   label/SID for both SR and non-SR paths.

I can't parse this sentence.

The SHOULD in Section 8 seems strange; it offers the implementer a choice of not allocating the binding label/SID when the PCC has met the stated conditions.  Why the choice?  If the SHOULD is meant to cover the "unable to allocate" case, I would argue that the SHOULD is not necessary because there's no interoperability choice being exercised, and would instead suggest:

   *  To request that the PCE allocate the binding label/SID, a PCC MUST
      set P=1, D=1, and include an empty TE-PATH-BINDING TLV in PCRpt
      message.  The PCE will attempt to allocate it and respond to the PCC with
      PCUpd message including the allocated binding label/SID in the TE-
      PATH-BINDING TLV and P=1, D=1 in the LSP object.  If the PCE is
      unable to allocate, it MUST send a PCErr message with Error-Type =
      TBD2 ("Binding label/SID failure") and Error-Value = TBD5 ("Unable
      to allocate a new binding label/SID").

Thank you for including Section 9.

Thank you to Dan Harkins for the SECDIR review.

I support Ben Kaduk’s DISCUSS position.

I have concerns based on the behavior intended for Binding Type (BT) = 3.  As was balloted for draft-ietf-lsr-isis-srv6-extensions, this document also describes a TLV to carry the SID sub-structure as an addressing mechanism which seems at odds with the IPv6 addressing architecture (RFC 4291). Despite this reservation, I see that there is a significant, production implementation and for this reason I am balloting no objection.

Thank you for the work put into this document. It explains a lot how RFC 8986 'network programming' can be used in a PCE/PCC environment and it is welcome. Even if parts of the document is difficult to read (e.g., the use case example).

Please find below one non-blocking COMMENT point.

Special thanks to Julien Meuric for the shepherd's write-up including the section about the WG consensus (even if I would have appreciated a justification for the PS status). 

I hope that this helps to improve the document,

Regards,

-éric

There are still many typos in the document; even if the RFC editor will fix them, let's try to avoid them.

-- Abstract --
Introducing "(SID)" after Segment Identifier would help the reader to understand the content of this document.

Thanks to Theresa Enghardt for their General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/IyiRN3TEd1p_yEs0UviVwhgH04Q).

-------------------------------------------------------------------------------
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 1.2. , paragraph 4, nit:
-    binding label/SID, and associted error handling.
+    binding label/SID, and associated error handling.
+                                 +

Section 1.2. , paragraph 2, nit:
> and PCE can use this TLV, how they can can allocate a binding label/SID, and
>                                    ^^^^^^^
Possible typo: you repeated a word.

Document references draft-ietf-pce-pcep-yang-17, but -18 is the latest
available revision.

Document references draft-ietf-spring-segment-routing-policy-14, but -16 is the
latest available revision.

FORMER DISCUSS (for the record)
Hi,

thank you for this document.
I have few points I'd like to discuss. Some might simply arise from my limited knowledge/expertise in pce. Thank you for your understanding.


Section 5 is confusing to me. In my understanding, from a PCE perspective, an LSP can be in three different situations: state synched with PCE, delegated to PCE, or initiated by PCE. However, maybe not all the text applies to the three cases. So, I wonder whether organizing this section along the lines of what can be done and not done depending on the situations would help. At least, one key aspect which is not clear to me is whether the binding value is an attribute over which control (change/remove) is given to pce in case of delegation.


Also, I have the impression that there are "error" conditions which are not described:

* how should PCC react (regardless of whether it already has a binding value) if it receives from PCE an empty TLV with R=1?

* how should PCC react if it receives from PCE a TLV with a binding value (different from the one it has) with R=1?

* how should PCC react if it receives from PCE a single TLV with a binding value different from the one it has and with R=0?


The draft seems silent about how to deal with multiple identical TE-PATH-BINDING TLVs. Use any? Throw an error?


The draft allows multiple TE-PATH-BINDING TLVs to be sent but in my view falls short to describe the associated expectations. Ultimately the binding value is something that will end-up programmed in the data-plane (or at least reserved in the control plane) but maybe not all systems have the same capabilities in these domains. Since PCE doesn't know these capabilities, I think, how should be handled the situation where PCE requests N values but the node wants to -or can- only use M (where M < N). Should an error be thrown back, or only M values chosen and reported back? Is the selection of the M values function of a local policy or should they be the M first or last, or any other specific rule?
These two paragraphs could be viewed as giving elements of response to these questions but it's not fully clear:
   If a PCE requires a PCC to allocate a (or several) specific binding
   value(s), it may do so by sending a PCUpd or PCInitiate message
   containing a TE-PATH-BINDING TLV(s).  If the value(s) can be
   successfully allocated, the PCC reports the binding value(s) to the
   PCE.  If the PCC considers the binding value specified by the PCE
   invalid, it MUST send a PCErr message with Error-Type = TBD2
   ("Binding label/SID failure") and Error Value = TBD3 ("Invalid SID").

   If the binding value is valid, but the PCC is unable to allocate the
   binding value, it MUST send a PCErr message with Error-Type = TBD2
   ("Binding label/SID failure") and Error Value = TBD4 ("Unable to
   allocate the specified binding value").  Note that, in case of an
   error, the PCC rejects the PCUpd or PCInitiate message in its
   entirety and can include the offending TE-PATH-BINDING TLV in the
   PCEP-ERROR object.



FORMER COMMENTs (FOR the record)
   To implement the needed changes to PCEP, in this document, we
   introduce a new OPTIONAL TLV that a PCC can use in order to report
   the binding label/SID associated with a TE LSP, or a PCE to request a
   PCC to allocate a specific binding label/SID value.
It can also request PCC to allocate a value of its own choosing (with an empty TLV), correct?

   As another way to use the extension specified in this document, to
   support the PCE-based central controller [RFC8283] operation where
   the PCE would take responsibility for managing some part of the MPLS
   label space for each of the routers that it controls, the PCE could
   directly make the binding label/SID allocation and inform the PCC.
   See Section 8 for details.
I am a bit sceptical about this and Section 8. This requires a mechanism which is, as the draft says, specified nowhere.

   If a PCE recognizes the TLV but does not support the TLV, it MUST
   send a PCErr with Error-Type = 2 (Capability not supported).
Could you elaborate on what you mean by "does not support the TLV"? For example the BT value?
Also, could the following situation exist: a node with some LSPs which can support a binding value and other which can't. How is PCC supposed to react to a PCE request to allocate a binding value on an LSP from the latter set?

   Multiple TE-PATH-BINDING TLVs are allowed to be present in the same
   LSP object.  This signifies the presence of multiple binding SIDs for
   the given LSP.  In the case of multiple TE-PATH-BINDING TLVs, the
   existing instances of TE-PATH-BINDING TLVs MAY be included in the LSP
   object.
Not sure what that last sentence means or adds compared to the previous ones.

s/associted/associated/

Thanks for this document.

No comments other than to say thank you for the reference of where the associated configuration YANG will be found.

Regards,
Rob