Re: [Pce] Gunter Van de Velde's No Objection on draft-ietf-pce-segment-routing-ipv6-22: (with COMMENT)

[Dhruv Dhody <[e-mail]>]

Hi Gunter,

Thanks for a detailed review.

On Tue, Apr 2, 2024 at 10:49 PM Gunter Van de Velde via Datatracker <
[e-mail]> wrote:

> Gunter Van de Velde has entered the following ballot position for
> draft-ietf-pce-segment-routing-ipv6-22: No Objection
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to
> https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
> for more information about how to handle DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-pce-segment-routing-ipv6/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Please find this review using a fresh pair of eyes upon the draft. feel
> free to
> use or ignore these comments. Comments are ordered with some first set of
> idnit
> typo's, followed with observations when reading the document.
>
> **Idnits:**
>
> 349        Endpoint node as well as the tailend node also need to be
> considered
>
> I believe that the grammatically correct form is "tail-end," which refers
> to
> the final part of something, such as a process, activity, or physical
> object.
> Using a hyphen clarifies that the two words function together as a single
> concept. Not sure if there is earlier art that uses the term with the
> proposed
> spelling in the document?
>
>
Dhruv: I checked; authors - please change to tail-end!



> 659        PCE.  As such,the flags MUST be set to zero and a (MSD-Type,MSD-
>
> s/such,the/such, the/
>
> 635        mechanisms, e.g routing protocols [RFC9352], then it ignores the
>
> s/e.g/e.g./
>
> 653        SRv6 MSD capabilties, the PCC MUST send a PCErr message with
> Error-
>
> s/capabilties/capabilities/
>
>
Dhruv: Ack for above!



> **Observations when reading through the document:**
>
> 20         Segment Routing (SR) can be used to steer packets through an
> IPv6 or
> 21         MPLS network using the source routing paradigm.  SR enables any
> head-
> 22         end node to select any path without relying on a hop-by-hop
> signaling
> 23         technique (e.g., LDP or RSVP-TE).
>
> Proposed rewrite
> Segment Routing (SR) can be used to steer packets through a network using
> the
> IPv6 or MPLS data plane, employing the source routing paradigm. SR enables
> any
> head-end node to select any path without relying on hop-by-hop signaling
> techniques (e.g., LDP or RSVP-TE).
>
>
Dhruv: Thanks for the rewrite, it reads better!



> 29         Since SR can be applied to both MPLS and IPv6 forwarding
> planes, a
> 30         PCE should be able to compute an SR Path for both MPLS and IPv6
> 31         forwarding planes.
>
> I suspect we are talking dataplane instead of forwarding plane? I see the
> terms
> "forwarding plane" and "data plane" often used interchangeably, but they do
> seem to have nuanced differences. The forwarding plane deals with the
> logical
> decision-making process of packet forwarding, the data plane deals with the
> physical implementation of forwarding those packets based on those
> decisions.
> In addition the term dataplane is used later in this same abstract. Maybe
> best
> to use single terminology (maybe dataplane) through the document?
>
>
Dhruv: Looking at spring RFCs I see a mix of terms but when talking about
SR instantiation as SR-MPLS and SRv6, the term data-plane is used and thus
we should also use the same.




> 31         forwarding planes.  The PCEP extension and mechanisms to
> support SR-
> 32         MPLS have been defined.
>
> What about adding the reference to RFC5440?
>

Dhruv: I would avoid adding references in the abstract.



>
> 32         MPLS have been defined.  This document describes the extensions
> 33         required for SR support for the IPv6 data plane in the Path
> 34         Computation Element communication Protocol (PCEP).
>
> This text reads a bit odd. What about a readability rewrite:
> “This document outlines the necessary extensions to support Segment Routing
> (SR) for the IPv6 data plane within the Path Computation Element
> Communication
> Protocol (PCEP).”
>
>
Dhruv: Ack, with slight modification as the whole para can be read as -

"Since SR can be applied to both MPLS and IPv6 data-planes, a PCE should be
able to compute an SR Path for both MPLS and IPv6 data-planes. The Path
Computation Element communication Protocol (PCEP) extension and mechanisms
to support SR-MPLS have been defined. This document outlines the necessary
extensions to support SR for the IPv6 data-plane within PCEP."



> 126        When the SR architecture is applied to the MPLS forwarding
> plane, it
> 127        is called SR-MPLS.  When the SR architecture is applied to the
> IPv6
> 128        data plane, is is called SRv6 (Segment Routing over IPv6 data
> plane)
> 129        [RFC8754].
>
> See earlier comments. Data plane vs forwarding plane.
>
> 133        IGP SPT.  Such paths may be chosen by a suitable network
> planning
> 134        tool, or a PCE and provisioned on the ingress node.
>
> The correlation between PCE and suitable network planning tool is unclear.
> Can
> the following text be used to close down on the ambiguity: “Such paths can
> be
> selected either by an appropriate network planning tool or by a Path
> Computation Element (PCE) and then provisioned on the ingress node.”
>
>
Dhruv: The text is borrowed for RFC8664 but I agree that using Network
Controller is a better term.



> 143        [RFC8231] specifies extensions to PCEP that allow a stateful
> PCE to
> 144        compute and recommend network paths in compliance with
> [RFC4657] and
> 145        defines objects and TLVs for MPLS-TE LSPs.  Stateful PCEP
> extensions
>
> I am unclear what 'recommend' means in this context? Can this be better
> explained and clarified? In RFC8231 there is no mentioning of recommended
> paths.
>

Dhruv: In RFC 8231 you will note "LSP Update Request" i.e. PCE is
requesting the PCC to update and PCC can report that the update is
rejected. Also RFC 8664 uses similar framing. This could be updated to
"compute and update" but it then loses the point that the PCC is free to
reject. I will let this be...



>
> 157        account various constraints and objective functions.  Once a
> path is
> 158        chosen, the stateful PCE can initiate an SR-TE path on a PCC
> using
> 159        PCEP extensions specified in [RFC8281] and the SR-specific PCEP
>
> “Once a path is chosen” seems to imply that there are multiple paths
> calculated
> and the best one is selected or chosen. Is this what is implied with this?
>
>
Dhruv: Yes, 'computed' is better than 'chosen'!



> 161        extensions for supporting a SR-TE LSP for the MPLS data plane.
> This
> 162        document extends [RFC8664] to support SR for the IPv6 data
> plane.
> 163        Additionally, using procedures described in this document, a
> PCC can
> 164        request an SRv6 path from either a stateful or stateless PCE.
> This
> 165        specification relies on the PATH-SETUP-TYPE TLV and procedures
> 166        specified in [RFC8408].
>
> This section is explaining what this draft is standardizing. It is a bit
> hidden
> and tucked all the way in the back of the introduction, a bit less trivial
> for
> the reader to discover.
>
>
Dhruv: This can be put in its own paragraph.



> 168        This specification provides a mechanism for a network controller
> 169        (acting as a PCE) to instantiate candidate paths for an SR
> Policy
> 170        onto a head-end node (acting as a PCC) using PCEP.  For more
>
> Before there was mentioning of a “network planning tool”. Maybe instead the
> term network controller can be used?
>
>
Dhruv: agree



> 212        Basic operations for PCEP speakers are as per [RFC8664].  SRv6
> Paths
> 213        computed by a PCE can be represented as an ordered list of SRv6
> 214        segments
>
> Reading this gives wrong indication that RFC8664 computes SRv6 paths. In
> the
> RFC8664 is explicitly written that “This document is relevant to the MPLS
> forwarding plane only.”
>
>
Dhruv: maybe "built on" instead of "as per".



> 250        In SR networks, an SR source node encodes all packets being
> steered
> 251        onto an SR path with a list of segments.
>
> “SR source node”. I am unsure what this refers towards. Would this be the
> segment routing ingress node? In Segment Routing (SR), the ingress node is
> known by the fact that it is the node where the packet enters the Segment
> Routing domain. When a packet enters a network that employs Segment
> Routing, it
> is typically tagged with a Segment List at the ingress node.
>
>
Dhruv: RFC 8754 and RFC 8986 use the term. Maybe we can reframe this as -

"In SR networks, an SR source node [RFC8754] steers a packet into an SR
Policy resulting in a segment list."



>  363       order to indicate that the path is for SRv6, any RP or SRP
> object
>
> These acronyms are not specified in the terminology section: Request
> Parameters
> (RP) [RFC5440] and the Stateful PCE Request Parameters (SRP)
>
>
Dhruv: They are expanded on first use in section 3.2



> 398        The 'L' Flag: Indicates whether the subobject represents a
> loose-hop
> 399        (see [RFC3209]).  If this flag is set to zero, a PCC MUST NOT
> 400        overwrite the SID value present in the SRv6-ERO subobject.
> 401        Otherwise, a PCC MAY expand or replace one or more SID values
> in the
> 402        received SRv6-ERO based on its local policy.
>
> The exact meaning of L-flag is confusing for SRv6. When looking at RFC3209
> it
> reflects upon nodes, however with SRv6 this may be an adj-SID or some other
> instruction. Maybe the L-flag can be enhanced to described what this means
> in
> the context of SRv6 SID.
>
>
Dhruv: The text is the same as RFC 8664 (SR-MPLS) and within the context of
PCEP it means that PCC should use the SID as it is v/s PCC being able to
apply suitable changes. If the intention is that the path as computed by
PCE should be used as it is, it is expected that the L flag would be set
accordingly.



> From RFC3209:
>    The path between a strict node and its preceding node MUST include
>    only network nodes from the strict node and its preceding abstract
>    node.
>
> 438        Flags: Used to carry additional information pertaining to the
> 439        SRv6-SID.  This document defines the following flag bits.  The
> other
> 440        bits MUST be set to zero by the sender and MUST be ignored by
> the
> 441        receiver.
>
> There is mentioning of S/F/T/V. is there a reason they are called like
> that? I
> suspect I am missing the history of naming of these flags and it just looks
> mostly random at this stage
>
>
Dhruv: The trend that started from RFC 8664 :)



> 475        SRv6 SID: SRv6 Identifier is an 128-bit value representing the
> SRv6
> 476        segment
>
> Any special considerations for csid?
>

Dhruv: As stated in the compression draft, no change is needed in PCEP.



> 481        At least one SRv6-SID or the NAI MUST be included in the
> SRv6-ERO
> 482        subobject, and both MAY be included.
>
> Is there any checking or processing to check if the NAI and SRV6-SID
> belong to
> the same node? Can they belong to different nodes?
>
>
Dhruv: Note that such a such is not done in SR-MPLS either, I would assume
that the SID value trumps NAI.




> 731        If a PCC receives an SRv6 path that exceeds the SRv6 MSD
> 732        capabilities, it MUST send a PCErr message with Error-Type = 10
> 733        ("Reception of an invalid object") and Error-Value = 43
> ("Unsupported
> 734        number of SRv6-ERO subobjects") as per [RFC8664].
>
> I assume this is about exceeding the local PCC capabilities? A local PCC
> router
> may have enough intelligence to understand the capability of all nodes
> through
> which the datapacket will be steered.  In theory the encoded payload may
> traverse a node that is not capable to process the SRH pushed by the SR PCC
> ingress router.
>
>
Dhruv: Yes, this is about local.



> 738        The SRv6-ERO contains a sequence of subobjects.  According to
> 739        [RFC9256], each SRv6-ERO subobject in the sequence identifies a
> 740        segment that the traffic will be directed to, in the order
> given.
> 741        That is, the first subobject identifies the first segment the
> traffic
> 742        will be directed to, the second SRv6-ERO subobject represents
> the
> 743        second segment, and so on
>
> Is there expectation that the node of a NAI corresponds with the node
> owning a
> SRv6-SID
>

Dhruv: Yes



>
> 771        Note that this specification enables a network controller to
> 772        instantiate an SRv6 path in the network.  This creates an
> additional
>
> Would it be more correct to indicate that it enables both to initiate and
> to
> monitor an SRv6 path?
>
>
>
Dhruv: The security vulnerability is more about instantiation.

Authors, please feel free to add your comments especially if you disagree
with my responses here.

Thanks!
Dhruv