Re: [Pce] WG Last Call for draft-ietf-pce-segment-routing-ipv6-15

[Cheng Li <[e-mail]>]

Hello Adrian,

Many thanks for your comments, brilliant!

I am preparing a update to address your comments and other guys' comments together, please wait for few days.

Cheers,
Cheng



-----Original Message-----
From: Pce [mailto:[e-mail]] On Behalf Of Adrian Farrel
Sent: Tuesday, February 21, 2023 5:14 AM
To: [e-mail]; [e-mail]
Subject: Re: [Pce] WG Last Call for draft-ietf-pce-segment-routing-ipv6-15

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 <[e-mail]> On Behalf Of [e-mail]
Sent: 13 February 2023 17:39
To: [e-mail]
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
[e-mail]
https://www.ietf.org/mailman/listinfo/pce