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

[Adrian Farrel <[e-mail]>]

Hi Cheng!

This is good progress, thanks.

I have cut down to the points that are still open.

Nothing we need to fight about 😊

Best,
Adrian

>> == Questions / Issues ==
>>
>> 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.
>
> [Cheng] How about this - 
>
>   o  BT = 0: The binding value is a 20-bit MPLS label value.  The TLV
>      is padded to 4-bytes alignment.  The Length MUST be set to 7 and
>      first 20 bits are used to encode the MPLS label value.
>
>   o  BT = 1: The binding value is a 32-bit MPLS label stack entry as
>      per [RFC3032] with Label, TC [RFC5462], S, and TTL values encoded.
>      Note that the receiver MAY choose to override TC, S, and TTL
>      values according to its local policy.  The Length MUST be set to 8.

OK. This is clear. I think you just need to look through the other mentions of BT=0 and BT=1 to make sure they match with this.

>> 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.)
>
> [Cheng] The idea was to make it consistent with IDR WG I-D [draft-ietf-idr-segment-routing-
> te-policy]. 
> One option could be to move this to the SR-policy-related PCE WG I-D [draft-ietf-pce-segment-
> routing-policy-cp]. 
> I have kept this open for now for further discussion.

To be clear, I was suggesting:
- this document creates and defines the registry
- the documents that need codepoints are responsible for requesting allocation of codepoints

>> 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?
>
> [Cheng] They should not be impacted. I have added this - 
>
>   In case of multiple TE-PATH-BINDING TLVs, all
>   instances of TE-PATH-BINDING TLVs MUST always be included in the LSP
>   object.  In case of an error, a PCErr message MAY include the
>   offending TE-PATH-BINDING TLV in the PCEP-ERROR object.

I think maybe I wasn't clear.
Suppose the LSP object contains three TE-PATH-BINDING TLVs (TLV1, TLV2, and TLV3)
Suppose that TLV3 contains a bad value.
That means a PCErr will be sent with PCEP-ERROR and that MAY contain the offending TLV3.
OK so far.
Now, what happens to TLV1 and TLV2?
    Are they processed and used by the receiver?
      Or
    Are they all rejected because the received message has been rejected?
Personally, I think you should reject the whole message because this makes the case of modifying previous values a lot easier (see a later point).

>> 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?
>
> [Cheng]
> The idea was to say that while BT=2 (just IPv6 address) for BSID is allowed, there
> are older implementations, but BT=3 SHOULD be proffered which requires explicit
> SID structure and behavior to be explicitly encoded. 
> It is not about when no Path Binding TLV is present.
> Do you have a better way to put this?

Ah, and possibly "ouch".
You are actually saying that BT=2 is only present to support pre-standard implementations. That's actually not a good reason to put something in a standard.

How about...
   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.  This enables the sender to have
   control of the SRv6 Endpoint Behavior and SID Structure.  A sender MAY
   choose to set the BT to 2, in which case the receiving implementation
   chooses how to interpret the SRv6 Endpoint Behavior and SID Structure
   according to local configuration.

>> 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?
>
> [Cheng] It has been specified that all instances of TE-PATH-BINDING TLVs
> need to be included and the offending TE-PATH-BINDING TLV can be
> included in the PCEP-ERROR object. 
>
> Any suggestion to make this clearer?

There are two separate issues here. Sorry for including them in one comment.

First. When you send a new PCRpt it completely overwrites any previous PCRpt and so you must include all of the TE-PATH-BINDING TLVs you want to apply form now on. So...
OLD
   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.
NEW
   If a PCC wishes to withdraw all previously reported binding values, it
   MUST send a PCRpt message without any TE-PATH-BINDING TLV.  If a
   PCC wishes to modify any or all previously reported binding values, it
   MUST send a PCRpt message containing TE-PATH-BINDING TLVs 
   containing all the binding values that apply from that point on.

   If a PCE wishes to modify a previously requested binding value, it
   MUST send a PCUpd message with TE-PATH-BINDING TLVs containing
   all the binding values that apply from that point on.  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, and the PCC MAY continue to
   use the previously allocated values.
END

Second. As discussed in a previous point, you have to describe what happens when a message with multiple TE-PATH-BINDING TLVs contains one that is in error. Is the whole message rejected, or do the good TLVs still get applied?  If the message is rejected, presumably the previously allocated values still apply. If the good TLVs still get actioned, presumably any that are not continued in the new message are removed. You can handle this by:
- addressing the issue as in the previous point
- putting a note here something like...

   As previously noted, if a message that is intended to modify a previously
   allocated binding value contains a TE-PATH-BINDING TLV with a bad 
   value, .... <fill in the details>

>> Expand PCE on first use
>
> [Cheng]Done. BTW PCE has a * in https://www.rfc-editor.org/materials/abbrev.expansion.txt

Oh, that's new. My apologies.

>> 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.
>
> [Cheng] No following text :)
> I suggest changing it to - "The TE-PATH-BINDING TLV in the LSP object identifies
> that the allocation is for Binding label/SID."

Sorry, I meant "with the text that follows in the next sentences of the document."

The document had...

     The TLV in LSP object identifies what should be
      allocated, such as Binding label/SID.  A PCC would set this bit to
      1 and include a TE-PATH-BINDING TLV in the LSP object to request
      for allocation of Binding label/SID by the PCE in the PCEP
      message.  A PCE would also set this bit to 1 and include a TE-
      PATH-BINDING TLV to indicate that the Binding label/SID is
      allocated by PCE and encoded in the PCEP message towards PCC.
      Further, a PCE would set this bit to 0 and include a TE-PATH-
      BINDING TLV in the LSP object to indicate that the Binding label/
      SID should be allocated by the PCC as described in Section 4.

It's not important, but my point was that the first sentence is redundant.

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

[snip]

> [Cheng] IANA has made the allocation as the I-D is in the RFC-Editor queue.
> Perhaps we can use the value directly as per https://www.iana.org/
> assignments/pcep/pcep.xhtml#pcep-error-object?

It is slightly premature to use that value, but I think it is safe to do. So, yes, put the actual value in the text.