Re: [Pce] WG Last Call for draft-ietf-pce-binding-label-sid-07 (and Code Point Allocation)

[Adrian Farrel <[e-mail]>]

Hi Julien, WG, authors.

Code point allocation: Is the request for all of the code points in the
document? What about the not-yet-allocated code point from
[I-D.ietf-pce-pcep-extension-for-pce-controller]. This spec can't be
implemented without it.

WG last call: I have a few questions/issues/nits below.

Best,
Adrian

== Questions / Issues ==

Abstract

What is the difference between "a Binding Segment Identifier (BSID)" and
a "binding Segment-ID (SID)"?

---

Abstract

"Such a binding label/SID"
This is the first use of "label". You need some context.

---

Abstract

   This document proposes
   an approach for reporting binding label/SID to Path Computation
   Element (PCE) for supporting PCE-based Traffic Engineering policies.

Who does the reporting?
Same in Section 1 top of page 4.

---

3.

   o  BT = 0: The binding value is an MPLS label carried in the format
      specified in [RFC5462] where only the label value is valid, and
      other fields MUST be considered invalid.  The Length MUST be set
      to 7.

I don't think that RFC 5462 is the right reference. That document simply
renames a field in the MPLS label stack entry.

So, are you carrying an MPLS label or an MPLS label stack entry? Either
is possible, although since you're only interested in the label, it
might be more normal to carry just the label value in the least
significant 20 bits of the binding value field. That would be consistent
with a Length of 7.

But, if you want to carry the full label stack entry (with the other
fields  ignored) then perhaps...
   o  BT = 0: The binding value is an MPLS label carried in an MPLS 
      label stack entry format as specified in [RFC3032] where only the
      label value is valid, and other fields MUST be ignored.  The
      Length MUST be set to 8.

This would be more consistent with:
   o  BT = 1: Similar to the case where BT is 0 except that all the
      fields on the MPLS label entry are set on transmission.  However,
      the receiver MAY choose to override TC, S, and TTL values
      according its local policy.  The Length MUST be set to 8.
But here you may want a little more clarity as:
   o  BT = 1: Similar to the case where BT is 0 except that all the
      fields of the MPLS label stack entry are set on transmission of
      the PCEP message containing the TLV.  A PCC receiver of this TLV 
      in a PCEP message MAY choose to override TC, S, and TTL values 
      according its local policy.  The Length MUST be set to 8.

But, at the bottom of the section...
   For the BT as 0, the 20 bits represent the MPLS
   label.  For the BT as 1, the 32-bits represent the label stack entry
   as per [RFC5462].
Which is going back on itself (and using the wrong reference).

Just make a decision on the meaning of BT=0 and make the text clean.

---

3.

I'm a bit puzzled as to why this document defines two flags for the 
Path Binding TLV Flags field, when this document clearly doesn't use or
depend on them.

In order to not make [I-D.ietf-spring-segment-routing-policy] a 
normative reference, perhaps you should not mention the specific bits,
create the registry (in 11.1.1) as empty, and simply not that 
[I-D.ietf-spring-segment-routing-policy] defines some flags.

(Obviously, [I-D.ietf-spring-segment-routing-policy] will need to make
assignments from the registry.)

---

4.

   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.

If one of these contains a bad value, and a PCErr is sent according to 
the previous paragraph, what happens to all the other Binding Values?
Are they used or discarded?

---

4.

   For SRv6 BSIDs, it is RECOMMENDED to always explicitly specify the
   SRv6 Endpoint Behavior and SID Structure in the TE-PATH-BINDING TLV
   by setting the BT (Binding Type) to 3, instead of 2.  The choice of
   interpreting SRv6 Endpoint Behavior and SID Structure when none is
   explicitly specified is left up to the implementation.

I puzzled about the alternative to "RECOMMENDED".  I wondered if the
second sentence was meant to offer that guidance, but perhaps it is
intended to only to cover the case when no Path Binding TLV is present.
Two concepts in one paragraph?

---

4.

   If a PCC wishes to withdraw or modify a previously reported binding
   value, it MUST send a PCRpt message without any TE-PATH-BINDING TLV
   or with the TE-PATH-BINDING TLV containing the new binding value
   respectively.

   If a PCE wishes to modify a previously requested binding value, it
   MUST send a PCUpd message with TE-PATH-BINDING TLV containing the new
   binding value.  The absence of TE-PATH-BINDING TLV in PCUpd message
   means that the PCE does not specify a binding value in which case the
   binding value allocation is governed by the PCC's local policy.

How does this work if multiple binding values have been assigned by 
using multiple TE-PATH-BINDING TLVs? Suppose, having done that, it is
the intention to withdraw one of the binding values but leave the others
in place. Should the message making the change contain all the TLVs 
except the one being withdrawn?

   If a PCC receives a valid binding value from a PCE which is different
   than the current binding value, it MUST try to allocate the new
   value.  If the new binding value is successfully allocated, the PCC
   MUST report the new value to the PCE.  Otherwise, it MUST send a
   PCErr message with Error-Type = TBD2 ("Binding label/SID failure")
   and Error Value = TBD4 ("Unable to allocate the specified label/
   SID").

And in this failure case, what is the residual state? Is the old value
still in place, or is no value in place?





== A whole pile of nits and minor issues ==

Abstract

s/a BSID to RSVP-TE/a BSID to an RSVP-TE/
s/or binding Segment-/or a binding Segment-/
s/to SR Traffic/to an SR Traffic/

---

Abstract

"This document proposes". You need to word this to be appropriate for an
RFC. Thus, "This document specifies".

---

Abstract

s/for supporting/to support/

---

Requirements Language should move to Section 2.

---

1.

Expand PCE on first use

---

1.

s/through a network that are/through a network where those paths are/
s/[RFC8402], Binding/[RFC8402], a Binding/
s/an Segment Routed/a Segment Routed/
s/equal to BSID/equal to a BSID/
s/interface or tunnels/interface or tunnel/
s/as segments/as a segment/
s/extension to PCEP that allows/extensions to PCEP that allow/
s/PCEP extension to setup/PCEP extensions to set up/

---

1. Please expand "LSP" on first use

---

1.

   The PCEP extension to setup and maintain SR-TE
   paths is specified in [RFC8664].

   [RFC8664] provides a mechanism for a network controller (acting as a
   PCE) to instantiate candidate paths for an SR Policy onto a head-end
   node (acting as a PCC) using PCEP.

Do you need both of these sentences?

---

1.

s/"Binding label/SID has local"/"A binding label/SID has local"/
s/the following diagram/Figure 1/  (you can use an xref)
s/"In IP/MPLS WAN"/"In the IP/MPLS WAN"/
s/setup using/set up using/

---

1.

   Binding value means either MPLS label or SID
   throughout this document.

This might read better as:

   Throughout this document, the term "binding value" means either an
   MPLS label or SID.

---

2.

The following are not used in this document and don't need to be in the
list:
   LER
   LSR

The following are not used in later sections of this document and don't
need to be in the list:
   SRGB
   SRLB

---

3.

s/in the figure below/in Figure 3/
s/in ([RFC8231])/in [RFC8231]/
s/type of this TLV is to be allocated by IANA/type of this TLV is TBD1/

---

3.

For Binding Type (BT), a forward pointer to 11.1.1 would be nice.

---

3.1.

   Carried as the Binding Value in the TE-PATH-BINDING TLV when the BT
   is set to 3.  Applicable for SRv6 Binding SIDs
   [I-D.ietf-spring-srv6-network-programming].

These need to be sentences with a subject.
I think you can update to [RFC8986].

---

3.1

Figure 4 should be numbered Figure 3.
The text should contain an explicit mention of Figure 3.
The text should mention each of the fields in the figure.

---

3.1
   Endpoint Behavior: 2 octets.  The Endpoint Behavior code point for
   this SRv6 SID as defined in section 9.2 of
   [I-D.ietf-spring-srv6-network-programming].
There is no section 9.2 in RFC 8986. You're probably after section 10.2.
But is that the right thing to do? Don't you want to point at the IANA
registry so that you embrace any future values as well?

---

3.1
   LB Length: 1 octet.  SRv6 SID Locator Block length in bits.

   LN Length: 1 octet.  SRv6 SID Locator Node length in bits.

   Function Length: 1 octet.  SRv6 SID Function length in bits.

   Argument Length: 1 octet.  SRv6 SID Arguments length in bits.

It is unclear what these lengths refer to in the context of this binding
value. Probably you can just provide pointers to where the meaning is
found.

---

4.
s/([RFC5440])/[RFC5440]/
s/message, PCE MUST/message, the PCE MUST/
s/MUST send the PCErr/MUST send a PCErr/

---

4.
   If a PCE requires a PCC to allocate a specific binding value, it may
   do so by sending a PCUpd or PCInitiate message containing a TE-PATH-
   BINDING TLV.
In order to avoid confusion, I suggest s/may do/does/

---

4.
   If a PCE requires a PCC to allocate a specific binding value, it may
   do so by sending a PCUpd or PCInitiate message containing a TE-PATH-
   BINDING TLV.

"it may do so" is probably "it instructs the PCC"

---

4.

s/receives TE-PATH-BINDING TLV/receives a TE-PATH-BINDING TLV/
s/other than LSP object/other than an LSP object/
s/It may do so by sending a PCUpd/It does so by sending a PCUpd/

---

5.

"a PCErr message is sent"  Who does the sending? "the receiver"?

---

6.

s/for SRv6 SID./for an SRv6 SID./

---

7.

s/This section specify/This section specifies/

---

7.

"allocate the binding label" Do you mean "binding label/SID"?
The rest of the paragraph seems a little inconsistent on that.
Perhaps, also, the section title is wrong.

---

7.

s/before PCE could/before the PCE can/

---

7.

      The TLV in LSP object identifies what should be
      allocated, such as Binding label/SID.

Suggest to remove this sentence as superfluous with following text.

---

7.

OLD
   o  to let the PCC allocates the binding label/SID, a PCE could set
      P=0 and empty TE-PATH-BINDING TLV ( i.e., no binding value is
      specified) in the LSP object in PCInitiate/PCUpd message.
NEW
   o  to let the PCC allocates the binding label/SID, a PCE could set
      P=0 and include an empty TE-PATH-BINDING TLV (i.e., no binding 
      value is specified) in the LSP object in PCInitiate/PCUpd message.
END

Similarly

OLD
   o  a PCC could request that the PCE allocate the binding label/SID by
      setting P=1, D=1, and empty TE-PATH-BINDING TLV in PCRpt message.
NEW
   o  a PCC could request that the PCE allocate the binding label/SID by
      setting P=1, D=1, and including an empty TE-PATH-BINDING TLV in
      PCRpt message.
END

---

7.

Bullets following "Note that,..." should all start with capital letters
so that multi-sentence bullet text is not confused.

---

7.

   o  if both peers have not exchanged the PCECC capabilities as per
      [I-D.ietf-pce-pcep-extension-for-pce-controller] and it receives
      P=1 in the LSP object, it needs to act as per
      [I-D.ietf-pce-pcep-extension-for-pce-controller]:

What is "it"?

---

7.

      *  Send a PCErr message with Error-Type=19 (Invalid Operation) and
         Error-Value=TBD (Attempted PCECC operations when PCECC
         capability was not advertised)

I understand this to mean to use the Error-Value defined in 
[I-D.ietf-pce-pcep-extension-for-pce-controller] and marked in that 
document as TBD3.

I suggest you use

      *  Send a PCErr message with Error-Type=19 ("Invalid Operation") 
         and Error-Value=TBD7 ("Attempted PCECC operations when PCECC
         capability was not advertised"). [RFC-EDITOR: Please replace
         TBD7 with the value assigned by IANA for this Error-Value,
         indicated by "TBD3" in [I-D.ietf-pce-pcep-extension-for-pce-
         controller].]

---

7.
   PCE would directly allocate the label
   from the PCE-controlled label space using P=1 as described above,
   whereas PCE would request for the allocation of a specific BSID from
   the PCC-controlled label space with P=0 as described in Section 4.
s/PCE would/The PCE can/ (twice)

---

9.

s/rouge/rogue/
s/other LSPs that uses/other LSP that uses/

---

11.1.1

The two new registries need to have ranges specified so that IANA knows
what new values can be assigned. 

---

I think the affiliations of some of your authors and contributors may
be out of date.