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

[Ketan Talaulikar <[e-mail]>]

Hi Dhruv,

Please check inline for responses/clarifications with KT.


On Mon, Apr 8, 2024 at 7:54 PM Dhruv Dhody <[e-mail]> wrote:

> 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.
>

KT> The unit of signaling is a CP, but by virtue of the SRPA what is
introduced is the ability to signal an SR Policy. Isn't it?


>
>
>
>> 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.
>

KT> What is being signaled via RFC8664 is something that is not defined in
SR Architecture but those specs are still called SR extensions. Putting a
formal "update" points readers that this document is what extends those
previous documents for SR Policy (and thereby SR) extensions.


>
> <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.
>

KT> Agree. A clarification is important in this context since the SRPA is
necessary (I would say mandatory) even for a single CP in the context of
this document for conformance with SR Policy architecture.


>
> <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.
>

KT> The section says "Overview" and not "Overview of the extensions" - but
these are editorial things which I would leave to the authors. My comment
was on the necessity of these extensions for supporting SR Policy construct
in PCEP.


>
> <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 ::
>

KT> It would be good to explicitly call that out. There may be
misunderstandings otherwise.

>
>
>
>>
>> 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!
>

KT> I did see some flags being used in the previous versions, but those are
very generic and applicable for any CP invalidation scenario in general and
not specific to the invalidation case. So my question is what future
extensibility do we foresee here? I am also looking at this from alignment
across to BGP-LS (in case we have missed something there).

Thanks,
Ketan


>
> <snip>
>
> Thanks!
> Dhruv
>