Hi,
Here is my WG last call review of this document. Thanks to the authors
for all of the work that has gone in.
[A note for the chairs: Was this last call shared with SPRING?]
Cheers,
Adrian
===
Abstract
The Source Packet Routing in Networking (SPRING) architecture
In fact, although RFCs 7855, 8354, and 8355 talk about "Source Packet
Routing in Networking (SPRING)", the architecture document (RFC 8402)
is the "Segment Routing Architecture."
---
Abstract
It depends only on "segments"
As it is a new paragraph, you need to clarify what "it" is. Or maybe
move this sentence to the previous paragraph so that para 2 begins "A
Segment Routed Path..."
---
Abstract
s/forwarding plane/forwarding planes/ (twice)
s/support for IPv6 data plane in/support for the IPv6 data plane in the/
---
Abstract
OLD
The PCEP extension and mechanism to support SR-MPLS
is described in RFC 8664.
NEW
The PCEP extensions and mechanisms to support SR-MPLS
are described in RFC 8664.
END
---
Abstract
I think the final sentence is superfluous (or confusing).
Probably this is a repeat of "This document describes the extensions
required for SR support for IPv6 data plane in Path Computation Element
communication Protocol (PCEP)."
---
1.
s/Locater/Locator/
s/The list of segment/The list of segments/
---
1.
Upon completion of a segment, a pointer in the new
routing header is incremented and indicates the next segment.
Not so new anymore!
Try...
Upon completion of a segment, a pointer in the SRH
is incremented and indicates the next segment.
---
1.
Segment Routing use cases are described in [RFC7855] and [RFC8354].
Segment Routing protocol extensions are defined in [RFC8667], and
[RFC8666].
There is nothing untrue in this paragraph. But what does it add? The use
cases aren't mentioned anywhere in the document, and the protocol
elements aren't used.
I'd just remove the paragraph.
---
1.
As per [RFC8754], an SRv6 Segment identifier is an 128-bit value.
"SRv6 SID" or simply "SID" are often used as a shorter reference for
"SRv6 Segment". Further details are in an illustration provided in
[RFC8986].
This duplicates and somewhat modifies where the first paragraph says...
An ordered list of segments
is encoded as an ordered list of IPv6 addresses in the routing
header.
I suggest you try to fold this short paragraph into the first paragraph
and clarify the difference between "an 128-bit value" and "IPv6 address"
---
1.
The SR is applied to IPV6 forwarding
plane using Source Routing Header (SRH) [RFC8754].
You've already said this in the first paragraph
---
1.
A SR path can be
derived from an IGP Shortest Path Tree (SPT), but SR-TE paths may not
follow IGP SPT.
a. You have just introduced "SR-TE". You need to expand it as "Segment
Routing Traffic-Engineering".
b. Is there actually any difference between an SR path and an SR-TE
path? If so, you might add it to Section 2 after the definition of
"SR Path".
c. Notwithstanding RFC 8664, s/may/might/ to avoid confusion with the
normal English "you may not do it!"
---
1.
s/SR-TE LSP for MPLS data plane/SR-TE LSP for the MPLS data plane/
s/support SR for IPv6 data plane/support SR for the IPv6 data plane/
s/either stateful or a stateless PCE/either a stateful or stateless PCE/
---
2.
s/equivalent to a SRv6 Path/equivalent to an SRv6 Path/
---
3.
s/operations for PCEP speakers is/operations for PCEP speakers are/
---
3.
SR-capable PCEP speakers should be able to generate and/or process
such an ERO subobject.
"should be able to" ??
Maybe...
SR-capable PCEP speakers can generate or process
such an ERO subobject.
---
3.
This document define new subobjects "SRv6-ERO" and "SRv6-RRO" in the
ERO and the RRO respectively to carry the SRv6 SID (IPv6 Address).
Should the previous paragraph have talked about the RRO?
---
3.1
In SR networks, an ingress node of an SR path appends all outgoing
packets with an SR header consisting of a list of SIDs (IPv6 Prefix
in case of SRv6).
a. s/SR header/SRH/
b. The SRH is only used for SRv6, so this text is a bit mixed-up
c. Isn't there technically a case where only one SID is present so the
SRH is not needed (the SID is put in the DA field of the
encapsulating IPv6 header)?
---
3.1
OLD
For the use of IPv6 in control plane only with MPLS data plane,
mechanism remains the same as specified in [RFC8664].
NEW
For the use of an IPv6 control plane but an MPLS data plane, the
mechanism remains the same as specified in [RFC8664].
END
---
3.1
This document describes an extension to the SR path for the IPv6 data
plane.
It does? I thought it was all about PCEP.
---
3.1 seems to have some duplication of content from Section 3.
---
4.1.1
To avoid any risk of stale text persisting into the RFC (and given
that you have this covered in the IANA section)...
OLD
* PST = 3 (early allocated by IANA): Path is setup using SRv6.
NEW
* PST = 3 : Path is setup using SRv6.
END
...and...
OLD
If a PCEP speaker includes PST=3 (early allocated by
IANA)
NEW
If a PCEP speaker includes PST=3
END
...and...
OLD
The code point for the TLV type is 27 (early allocated by IANA). The
TLV length is variable.
NEW
The code point for the TLV type is 27. The TLV length is variable.
END
---
4.1.1
A PCEP speaker MUST indicate its support of the function described in
this document by sending a PATH-SETUP-TYPE-CAPABILITY TLV in the OPEN
object with this new PST "3" included in the PST list.
I think that "MUST" is not needed here. So s/MUST indicate/indicates/
---
4.1.1
If a PCEP speaker includes PST=3 (early allocated by
IANA) in the PST List of the PATH-SETUP-TYPE-CAPABILITY TLV then it
MUST also include the SRv6-PCE-CAPABILITY sub-TLV inside the PATH-
SETUP-TYPE-CAPABILITY TLV.
Two questions:
a. What happens if PST=3 is present but SRv6-PCE-CAPABILITY is
missing?
b. What happens if SRv6-PCE-CAPABILITY is present, but PST=3 was not
included? (You don't say that that is not allowed.)
It's possible that a forward pointer to Section 5 is enough (I didn't
cross-check this).
---
4.1.1
It's OK to say the TLV length is variable, but you should refer back to
the RFC that defines what a TLV length is.
Ah! and then the final paragraph of 4.1.1 makes it clear. Maybe that
paragraph needed to come before the figure?
---
4.1.1
Flags: 2 octet, two bits are currently assigned in this document.
A pointer to Section 9.4 would be useful
---
4.2
In order to indicate the SRv6 path, RP or SRP object MUST include the
PATH-SETUP-TYPE TLV specified in [RFC8408]. This document defines a
new Path Setup Type (PST=3 (early allocated by IANA)) for SRv6.
s/the SRv6 path/that the path is for SRv6/
s/RP or SRP/any RP or SRP/
d/(early allocated by IANA)/
---
4.2
The LSP-IDENTIFIERS TLV MAY be present for the above PST type.
You need to give an implementer a clue about why they might include the
TLV and why they might leave it out. It is OK to refer to another
document if the decision process is identical.
It's possible that a forward pointer to Section 5 is enough (I didn't
cross-check this).
---
4.3
OLD
In order to support SRv6, new subobject "SRv6-ERO" is defined in ERO.
NEW
In order to support SRv6, a new "SRv6-ERO" subobject is defined for
inclusion in the in ERO.
END
---
4.3.1
d/(early allocated by IANA)/
s/a SRv6-ERO/an SRv6-ERO/
s/a SRv6 SID/an SRv6 SID/
---
4.3.1
This section includes a lot of encoding rules using "MUST". Good, but
you need to say what a receiver does when it discovers a rule has been
broken.
Similarly for...
SR-MPLS specific NT types are not valid in SRv6-ERO.
It's possible that a forward pointer to Section 5 is enough (I didn't
cross-check this).
---
4.3.1
OLD
This document reuses NT types defined in [RFC8664]:
NEW
This document reuses NT types defined in [RFC8664], but assigns them
new meanings appropriate to SRv6:
END
---
4.3.1 Flags
A pointer to Section 9.2 would be useful.
---
4.3.1
What is the interaction between the L flag in the subobject header and
the S bit in the Flags field?
---
4.3.1
The list of Endpoint behaviors are
defined in [RFC8986].
s/are/is/
However, I think it would be better to say:
[RFC8986] defined a registry of Endpoint Behaviors that is maintained
by IANA.
---
4.3.1 / 4.3.1.1
Figure 2 shows the "SID Structure" field as 32 bits, but Figure 3 is
pretty clear that it is 64 bits. I think Figure 2 is broken.
---
4.4
OLD
In order to support SRv6, new subobject "SRv6-RRO" is defined in RRO.
NEW
In order to support SRv6, a new "SRv6-RRO" subobject is defined for
inclusion in the in RRO.
END
---
4.4.1
Is the SRv6-SID field optional in the RRO subobject? The figure implies
it must be present, and that would seem pretty reasonable for an RRO.
But the text says all the fields are exactly as in the ERO, and the S
flag seems to still have meaning.
There is the same problem with the length of the SID structure field.
---
5.1
d/(early allocated by IANA)/ (three times)
s/Error- Type/Error-Type/ (might be an XML2RFC bug)
s/PATH-SETUP- TYPE-CAPABILITY/PATH-SETUP-TYPE-CAPABILITY/
s/plane's capability/plane capability/
---
5.1
If a PCE
receives an SRv6-PCE-CAPABILITY sub-TLV with the X flag set to 1 then
it MUST ignore any MSD-Type, MSD-Value fields and MUST assume that
the sender can impose any length of SRH.
Is that really "MUST assume" or just "may assume"? That would seem to
be consistent with...
However, if a PCE learns these via different means, e.g
routing protocols, as specified in:
[I-D.ietf-lsr-ospfv3-srv6-extensions];
[I-D.ietf-lsr-isis-srv6-extensions]; [I-D.ietf-idr-bgpls-srv6-ext],
then it ignores the values in the SRv6-PCE-CAPABILITY sub-TLV.
But there's a hole :-( If we follow the previous paragraph, the PCE
may send an ERO with more SIDs than allowed by the MSD in the Open
message (if the value learned from the IGP is a larger number. But...
If a PCEP
session is established with a non-zero SRv6 MSD value, and the PCC
receives an SRv6 path containing more SIDs than specified in the SRv6
MSD value (based on the SRv6 MSD type), the PCC MUST send a PCErr
message with Error-Type = 10 (Reception of an invalid object) and
Error-Value = TBD1 (Unsupported number of SRv6-ERO subobjects).
...says that regardless of the MSD in the IGP, the PCC must reject an
ERO that contains more SIDs that specified in the Open message.
Furthermore, you have...
If a PCC receives a list of SRv6 segments, and the number of SRv6
segments exceeds the SRv6 MSD that the PCC can impose on the packet
(SRH), it MUST send a PCErr message with Error-Type = 10 ("Reception
of an invalid object") and Error-Value = TBD5 ("Unsupported number of
SRv6-ERO subobjects") as per [RFC8664].
This last paragraph seems to cover all bases, so why not just use it?
---
5.1
There seems to be a contradiction about the MSD 0 case.
You have...
If a PCE receives an SRv6-PCE-CAPABILITY sub-TLV
with the X flag as set, and SRv6 MSD-Type or MSD-Value fields are set
to zero then it is considered as an error and the PCE MUST respond
with a PCErr message (Error-Type = 1 "PCEP session establishment
failure" and Error-Value = 1 "reception of an invalid Open message or
a non Open message.").
...and...
If a
PCEP session is established with an SRv6 MSD value of zero, then the
PCC MAY specify an SRv6 MSD for each path computation request that it
sends to the PCE, by including a "maximum SID depth" metric object on
the request similar to [RFC8664].
The first says you can't have a session with MSD 0, the second tells you
how to act if you do.
---
5.1
The N flag, X flag and (MSD-Type,MSD-Value) pair inside the SRv6-PCE-
CAPABILITY sub-TLV are meaningful only in the Open message sent from
a PCC to a PCE. As such, a PCE MUST set the flags to zero and not
include any (MSD-Type,MSD-Value) pair in the SRv6-PCE-CAPABILITY sub-
TLV in an outbound message to a PCC. Similarly, a PCC MUST ignore
N,X flag and any (MSD-Type,MSD-Value) pair received from a PCE. If a
PCE receives multiple SRv6-PCE-CAPABILITY sub-TLVs in an Open
message, it processes only the first sub-TLV received.
How does this work when the session is between two PCEs?
---
5.2.1
I find the following...
* If NT=0, the F bit MUST be 1, the S bit MUST be zero and the
Length MUST be 24.
So, NT=0, F=1, S=1 is an error.
Then...
If a PCC finds that the NT field, Length field, S bit, F bit, and T
bit are not consistent, it MUST consider the entire ERO invalid and
MUST send a PCErr message with Error-Type = 10 ("Reception of an
invalid object") and Error-Value = 11 ("Malformed object").
But...
If a PCC receives an SRv6-ERO subobject in which the S and F bits are
both set to 1 (that is, both the SID and NAI are absent), it MUST
consider the entire ERO invalid and send a PCErr message with Error-
Type = 10 ("Reception of an invalid object") and Error-value = TBD3
("Both SID and NAI are absent in the SRv6-ERO subobject").
Two error messages for the same error?
---
5.2.1
If a PCC receives an SRv6-ERO subobject in which the S bit is set to
1 and the F bit is set to zero (that is, the SID is absent and the
NAI is present), but the PCC does not support NAI resolution, it MUST
consider the entire ERO invalid and send a PCErr message with Error-
Type = 4 ("Not supported object") and Error-value = 4 ("Unsupported
parameter").
This is good. But it seems harsh for the poor PCE. How was the PCE
supposed to know that the PCC didn't support NAI resolution?
---
5.2.1
d/(early allocated by IANA)/ (twice)
---
5.2.2
There is nothing wrong with what is written here, but it may be very
helpful to PCC implementations if you make it clear that the SID list
in an SRH is arranged in the reverse order to the ERO subobject list.
---
5.3
Here, too, it would be really helpful to be absolutely clear about the
order of subobjects, and to compare this to the order of the SIDs in the
SRH.
---
5.3
d/(early allocated by IANA)/ (twice)
---
5.3
RFC 8664 makes some wise observations about how a PCC constructs an RRO
(how can it know?) and suggests that the mechanisms are out of scope of
the RFC. I suggest you look at that text and write something similar
here.
---
6.
No mention of RFC 8253?
---
7.1
s/be allowed to set/be allowed to be set/
---
7.4
I thought Endpoint Behavior and SID Structure were new to this
document and provided the potential for (future) verification?
---
9.1
Isn't the RRO in PCEP the "Reported Route Object"?
---
9.1
I'm trying to find the document that makes the variation from RFC 5440
for the definition of "PCEP only" ERO and RRO subobjects. I understand
why you want to do this, but what you are doing is contrary to RFC 5440.
I see that RFC 8664 dodged this by "forgetting" to mark the SR-ERO and
SR-RRO subobjects as "PCEP only".
I have started a separate thread on this on the mailing list and have
copied TEAS because this seems to need some small administrative work to
fix it up.
---
9.2
It might be nice if the columns in your table had the same names as you
use in the text.
---
9.2
You need to tell IANA how many bits can be allocated. Otherwise they may
try to allocate bit 12 to the next request.
---
9.4
It might be nice if the columns in your table had the same names as you
use in the text.
---
9.4
The table headings are missing "-----" separators
---
9.4
You need to tell IANA how many bits can be allocated. Otherwise they may
try to allocate bit 16 to the next request.
---
11.1
I think that 8402 is only an informative reference.
---
Appendix A.
s/Contributor/Contributors/
-----Original Message-----
From: Pce <pce-boun...@ietf.org> On Behalf Of julien.meu...@orange.com
Sent: 13 February 2023 17:39
To: pce@ietf.org
Subject: [Pce] WG Last Call for draft-ietf-pce-segment-routing-ipv6-15
Dear PCE WG,
This message starts a 2-week WG last call on
draft-ietf-pce-segment-routing-ipv6-15 [1]. Please, be express any
comments you have about this document using the PCE mailing list.
This WGLC will end on Tuesday 28th February 2023.
Thanks,
Julien
--
[1] https://datatracker.ietf.org/doc/draft-ietf-pce-segment-routing-ipv6/
_______________________________________________
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce
