Re: [Pce] Review of draft-ietf-pce-segment-routing-policy-cp-15 from SR Policy Arch POV

[Dhruv Dhody <[e-mail]>]

Hi Ketan, Authors,

I am just responding to a few comments as a Shepherd. Please work with each
other and let's resolve these comments....

On Mon, Apr 8, 2024 at 5:15 PM Ketan Talaulikar <[e-mail]>
wrote:

> Hello Authors/All,
>
> I've reviewed the draft from the perspective of consistency with the SR
> Policy Arch RFC9256 as well as the BGP SR Policy SAFI IDR draft. As such, I
> find some issues that I think need to be addressed before publication.
>
> Note that these may seem editorial (no change needed in PCEP signaling),
> but they are semantically important for this document to convey the intent
> and operations for these extensions.
>
> My comments are embedded in the idnits format below.
>
> Thanks,
> Ketan
>
>
> 13 Path Computation Element Communication Protocol (PCEP) Extensions for
> 14              Segment Routing (SR) Policy Candidate Paths
>
> *[major] A more appropriate title would be "PCEP Extensions for SR
> Policy" *
> *since this is not just about CPs but support for SR Policy construct.*
>
>
Dhruv: In PCEP the key construct that we signal is a CP and thus I consider
the title to be apt.



> 15              draft-ietf-pce-segment-routing-policy-cp-15
>
> 17 Abstract
>
> 19   Segment Routing (SR) allows a node to steer a packet flow along any
> 20   path.  SR Policy is an ordered list of segments (i.e., instructions)
> 21   that represent a source-routed policy.  Packet flows are steered into
> 22   an SR Policy on a node where it is instantiated called a headend
> 23   node.  An SR Policy is made of one or more candidate paths.
>
> 25   This document specifies Path Computation Element Communication
> 26   Protocol (PCEP) extension to associate candidate paths of the SR
> 27   Policy.  Additionally, this document updates [RFC8231] to allow
> 28   stateful bringup of an SR LSP, without using PCReq/PCRep messages.
> 29   This document is applicable to both Segment Routing over MPLS and to
> 30   Segment Routing over IPv6 (SRv6).
>
>
>
> *[major] This document has the specifications that actually align the
> PCEPextensions (e.g. RFC8664) for SR defined prior to this document to the *
> *SR Policy architecture RFC9256. This should be the most important thing
> to call out and that this spec formally updates RFC8664 and *
> *RFC-draft-ietf-pce-segment-routing-ipv6.*
>
>
Dhruv: Happy for the authors to make it clearer but I don't think there is
a need for a formal update. "Update" makes sense when there is some text in
those RFCs that require changes. It is just a normal extension. From PCEP
POV, SR-MPLS and SRv6 extensions for those path setup types exist on their
own. If SR Policy is in use, then of course this document should be used.

<snip>



>
> 126   Segment Routing Policy for Traffic Engineering [RFC9256] details the
> 127   concepts of SR Policy and approaches to steering traffic into an SR
> 128   Policy.
>
> 130   PCEP Extensions for Segment Routing [RFC8664] specifies extensions to
> 131   the Path Computation Element Protocol (PCEP) that allow a stateful
> 132   PCE to compute and initiate Traffic Engineering (TE) paths, as well
> 133   as a PCC to request a path subject to certain constraint(s) and
> 134   optimization criteria in SR networks.
>
> 136   PCEP Extensions for Establishing Relationships Between Sets of LSPs
> 137   [RFC8697] introduces a generic mechanism to create a grouping of LSPs
> 138   which can then be used to define associations between a set of LSPs
> 139   and a set of attributes (such as configuration parameters or
> 140   behaviors) and is equally applicable to stateful PCE (active and
> 141   passive modes) and stateless PCE.
>
>
>
> *[major] It may be somewhat misleading to give an impression that
> thepurpose for introducing the SRPA is "to create a grouping of LSPs". *
>
>
> *For sure, the SRPA enables the  grouping of CPs associated with a single
> SR Policy. However, SRPA is  absolutely necessary even when signaling a
> single SR Policy CP as it is  indicating the Color of the SR Policy and
> the identifiers of the SR Policy CP  **as per RFC9256.*
>
>
Dhruv: In PCEP, association can exist with a single LSP in the association
group. If authors feel they need to be explicit about it, it is fine with
me.

<snip>



> 156   Endpoint:  The IPv4 or IPv6 endpoint address of the SR Policy in
> 157      question, as described in [RFC9256].
>
> 159   SRPA:  SR Policy Association.  A new association type 'SR Policy
> 160      Association' is used to group candidate paths belonging to the SR
> 161      Policy.  Depending on discussion context, it can refer to the PCEP
> 162      ASSOCIATION object of SR Policy type or to a group of LSPs that
> 163      belong to the association.
>
> 165   Association Parameters:  As described in [RFC8697], refers to the key
> 166      data, that uniquely identifies the Association.
>
> 168   Association Information:  As described in [RFC8697], refers to the
> 169      non-key information about the Association.
>
> 171 3.  Overview
>
>
>
>
>
>
>
>
>
>
>
>
> *[major] What we are missing here is some text about the motivation for
> whythese extensions are a MUST have for SR Policy signaling via PCEP. I
> will try tosummarize:a) To align with SR Policy construct per RFC9256b) To
> ensure that the headend is able to "associated" a SR Policy CP signaledvia
> PCEP with other CPs of the same SR Policy from other sources - e.g.,locally
> configured or via BGP.c) To indicate the intent of the SR Policy via
> "color" that can then be usedon the headend for steering BGP service routes
> over the SR Policy provisionedvia PCEP (PCE initiated).None of these were
> possible without this extension for certain deploymentdesigns - especially
> PCE initiated.*
>
>
Dhruv: I don't disagree but this section is an overview of the extensions
and not motivation. If authors wish to add a motivation section, sure they
can do that.

<snip>


>
> 283   The Association Source MUST be set to the headend value of the SR
> 284   Policy, as defined in [RFC9256] Section 2.1.
>
> 286   The 16-bit Association ID field in the ASSOCIATION object MUST be set
> 287   to the value of "1".
>
> 289   The Extended Association ID TLV MUST be included and it MUST be in
> 290   the following format:
>
> 292       0                   1                   2                   3
> 293       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 294      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 295      |           Type = 31           |       Length = 8 or 20        |
> 296      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 297      |                             Color                             |
> 298      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 299      ~                           Endpoint                            ~
> 300      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> 302                Figure 1: Extended Association ID TLV format
>
> 304   Type: Extended Association ID TLV, type = 31 [RFC8697].
>
> 306   Length: Either 8 or 20, depending on whether IPv4 or IPv6 address is
> 307   encoded in the Endpoint field.
>
> 309   Color: SR Policy color value, non-zero as per [RFC9256] Section 2.1.
>
> 311   Endpoint: can be either IPv4 or IPv6.  This value MAY be different
> 312   from the one contained in the Destination address field in the END-
> 313   POINTS object, or in the Tunnel Endpoint Address field in the LSP-
> 314   IDENTIFIERS TLV.
>
> 316   If the PCEP speaker receives an SRPA object whose Association
> 317   Parameters do not follow the above specification, then the PCEP
> 318   speaker MUST send PCErr message with Error-Type = 26 "Association
> 319   Error", Error-Value = 20 "SR Policy Identifier Mismatch".
>
> 321   The purpose of choosing the Association Parameters in this way is to
> 322   guarantee that there is no possibility of a race condition when
> 323   multiple PCEP speakers want to associate the same SR Policy at the
> 324   same time.  By adhering to this format, all PCEP speakers come up
> 325   with the same Association Parameters independently of each other
> 326   based on the SR Policy [RFC9256] parameters.  Thus, there is no
> 327   chance that different PCEP speakers will come up with different
> 328   Association Parameters for the same SR Policy.
>
> 330   The last hop of the computed SR Policy Candidate Path MAY differ from
> 331   the Endpoint contained in the <headend, color, endpoint> tuple.  An
> 332   example use case is to terminate the SR Policy before reaching the
> 333   Endpoint and have decapsulated traffic go the rest of the way to the
> 334   Endpoint node using the native IGP path(s).  In this example, the
> 335   destination of the SR Policy Candidate Paths will be some node before
> 336   the Endpoint, but the Endpoint value is still used at the head-end to
> 337   steer traffic with that Endpoint IP into the SR Policy.  Destination
> 338   of the SR Policy Candidate Path is signaled using the END-POINTS
> 339   object and/or LSP-IDENTIFIERS TLV, as per the usual PCEP procedures.
> 340   When neither END-POINTS object nor LSP-IDENTIFIERS TLV is present,
> 341   the PCEP speaker MUST extract the destination from the Endpoint field
> 342   in the SRPA Extended Association ID TLV.
>
> *[major] What about the null endpoint support?*
>
>
Dhruv: One can still use the values 0.0.0.0 and ::



>
> 394 4.2.2.  SR Policy Candidate Path Identifier TLV
>
> 396   The SRPOLICY-CPATH-ID TLV is a mandatory TLV for the SRPA object.
>
> 398       0                   1                   2                   3
> 399       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 400      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 401      |             Type              |             Length            |
> 402      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 403      | Proto. Origin |                      MBZ                      |
> 404      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 405      |                         Originator ASN                        |
> 406      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 407      |                                                               |
> 408      |                       Originator Address                      |
> 409      |                                                               |
> 410      |                                                               |
> 411      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 412      |                         Discriminator                         |
> 413      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> 415                 Figure 3: The SRPOLICY-CPATH-ID TLV format
>
> 417   Type: 57 for "SRPOLICY-CPATH-ID" TLV.
>
> 419   Length: 28.
>
> 421   Protocol Origin: 8-bit value that encodes the protocol origin, as
> 422   specified in Section 6.5.  Note that in PCInitiate message [RFC8281],
> 423   the Protocol Origin is always set to 10 (PCEP).
>
> 425   MBZ: Must be zero.
>
> 427   Originator ASN: Represented as 4-byte number, part of the originator
> 428   identifier, as specified in [RFC9256] Section 2.4.  If 2-byte ASNs
> 429   are in use, the low-order 16 bits is used, and the high-order bits
> 430   are set to 0.  When sending PCInitiate message [RFC8281], the PCE is
> 431   acting as the originator and therefore MUST set this to an ASN that
> 432   it belongs to.
>
>
>
> *[major] This is an unnecessary restriction on PCEP which does not use
> ASN.RFC9256 section 2.4 allows the value of 0 to be used by PCEP when ASN
> is notavailable/known.*
>
>
Dhruv: Good catch!

<snip>



> 622 5.4.  Invalidation TLV
>
> 624   The INVALIDATION TLV is an optional TLV for the LSP object.  It is
> 625   used to control traffic steering into the LSP during the time when
> 626   the LSP is operationally down/invalid.  In the context of SR Policy,
> 627   this TLV facilitates the Drop-upon-invalid behavior, specified in
> 628   Section 8.2 of [RFC9256].  Normally, if the LSP is down/invalid then
> 629   it stops attracting traffic and traffic that would have been destined
> 630   to that LSP is redirected somewhere else, such as via IGP or via
> 631   another LSP.  The Drop-upon-invalid behavior specifies that the LSP
> 632   keeps attracting traffic and the traffic has to be dropped at the
> 633   head-end.  Such an LSP is said to be "in drop state".  While in the
> 634   drop state, the LSP operational state is "UP", as indicated by the
> 635   O-flag in the LSP object.  However the ERO object MAY be empty, if no
> 636   valid path has been computed.
>
> 638   The INVALIDATION TLV is used in both directions between PCEP peers:
>
> 640   *  PCE -> PCC: PCE specifies to the PCC whether to enable or disable
> 641      Drop-upon-invalid (Config).
>
> 643   *  PCC -> PCE: PCC reports the current setting of the Drop-upon-
> 644      invalid (Config) and also whether the LSP is currently in the drop
> 645      state (Oper).
>
> 647       0                   1                   2                   3
> 648       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 649      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 650      |             Type              |             Length            |
> 651      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 652      |   Oper        |   Config      |              MBZ              |
> 653      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 654                   Figure 9: The INVALIDATION TLV format
>
> 656   Type: 70 for "INVALIDATION" TLV.
>
> 658   Length: 4.
>
> 660   Oper: encodes the current state of the LSP, i.e., whether it is
> 661   actively dropping traffic right now.  This field can be set to non-
> 662   zero values only by the PCC, it MUST be set to 0 by the PCE and
> 663   SHOULD be ignored by the PCC.
>
> 665                               0 1 2 3 4 5 6 7
> 666                              +-+-+-+-+-+-+-+-+
> 667                              |             |D|
> 668                              +-+-+-+-+-+-+-+-+
>
> 670             Figure 10: Oper state of Drop-upon-invalid feature
>
> 672   *  D: dropping - the LSP is currently attracting traffic and actively
> 673      dropping it.
>
> 675   *  The unassigned bits in the Flag octet MUST be set to zero upon
> 676      transmission and MUST be ignored upon receipt.
>
> 678   Config: encodes the current setting of the Drop-upon-invalid feature.
>
> 680                               0 1 2 3 4 5 6 7
> 681                              +-+-+-+-+-+-+-+-+
> 682                              |             |D|
> 683                              +-+-+-+-+-+-+-+-+
>
> 685            Figure 11: Config state of Drop-upon-invalid feature
>
> 687   *  D: drop enabled - the Candidate Path has Drop-upon-invalid feature
> 688      enabled.
>
> 690   *  The unassigned bits in the Flag octet MUST be set to zero upon
> 691      transmission and MUST be ignored upon receipt.
>
>
>
> *[major] Was there a specific reason to specify this as a boolean instead
> ofbits/flags?*
>

Dhruv: There were more flags before but were removed, the aim is to allow
extensibility. I suggested authors turn this into a registry!

<snip>

Thanks!
Dhruv