From: Pce <pce-boun...@ietf.org> on behalf of Adrian Farrel
<adr...@olddog.co.uk>
Sent: 23 March 2021 10:17
To: julien.meu...@orange.com; pce@ietf.org
<one comment inline>
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.
<tp>
Adrian
I share your comments about the terminology in this I-D which is not precise,
failing to specify 'MPLS label' and failing to use 'MPLS label stack entry' but
I think that the worst part is this 'binding/Label SID' which to me is a
classic example of how not to choose an identifier. The I-D does use 'binding
value' in one place as what appears to be one of the many different terms for
this concept and I think that term much better.
Since this term, whatever it is, gets embedded in IANA, I said that the IANA
early allocation should not proceed until this is resolved, but you may not
take it that far.
Tom Petch
---
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.
_______________________________________________
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce
_______________________________________________
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce
