Hi Ketan, Mike, Please see inline <S>.
(Thanks a lot for review, Ketan) Regards, Samuel From: Ketan Talaulikar <ketant.i...@gmail.com> Sent: Wednesday, April 2, 2025 8:03 AM To: Mike Koldychev <mkold...@proton.me> Cc: The IESG <i...@ietf.org>; draft-ietf-pce-segment-routing-policy...@ietf.org; pce-cha...@ietf.org; pce@ietf.org Subject: Re: [Pce] Ketan Talaulikar's Discuss on draft-ietf-pce-segment-routing-policy-cp-25: (with DISCUSS and COMMENT) Hi Mike, Please check inline below for my responses with KT. On Tue, Apr 1, 2025 at 7:28 PM Mike Koldychev <mkold...@proton.me<mailto:mkold...@proton.me>> wrote: Hi Ketan, Appreciate the useful feedback, please find my comments inline with <MK></MK>. Thanks, Mike. On Tuesday, April 1st, 2025 at 7:51 AM, Ketan Talaulikar via Datatracker <nore...@ietf.org<mailto:nore...@ietf.org>> wrote: > > > Ketan Talaulikar has entered the following ballot position for > draft-ietf-pce-segment-routing-policy-cp-25: Discuss > > 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-policy-cp/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > Here are a couple of topics that need a discussion with the authors/WG: > > <discuss-1> Sections 3.1 and 3.2 > > > I would like to discuss some of the normative text related to the handling > of identifiers in this section. I am also sharing some suggestions based on my > understanding - please correct me, if I am wrong. > > CURRENT > SR Policy Identifier MUST be the same for all SR Policy Candidate Paths in the > same SRPA. > > SUGGEST > LSPs that represent Candidate Paths within an SR Policy MUST carry the > same SR Policy Identifiers (carried in Extended Association ID TLV) in their > SRPA. <MK> "LSPs that represent Candidate Paths within an SR Policy" is a lot of words, the document already states that LSP represents the SR Candidate Path, I don't see a need to repeat that on every usage. </MK> KT> I beg to differ. Few extra words don't cost anything. Clarity in normative language is important. LSP is the "base" object in PCEP per my understanding and so the specification is clear when using that object as a reference. LSP represents a SR Policy CP, only when there is a valid SRPA associated with it. <S> Wouldn’t it be better to than rather add clarification in terminology section to clarify that LSPs represent Candidate Paths within an SR Policy and also that "LSP" used in this draft equivalent to an SRv6 path (represented as a list of SRv6 segments) if Path Setup Type is SRv6 (based on statement from RFC9603)? Then we don’t need to repeat that definition in multiple statements. For this one, we can then simplify to: Candidate Paths within an SR Policy MUST carry the same SR Policy Identifiers in their SRPA. (I removed part of Extended Association ID TLV as headend address is not encoded in that TLV) > > CURRENT > SR Policy Identifier MUST be constant for a given SR Policy Candidate Path for > the lifetime of the PCEP session. > > SUGGEST > LSPs that represent Candidate Paths within an SR Policy MUST NOT change their > SR Policy Identifiers for the lifetime of the LSP and the PCEP session. > > Reason: While the PCEP session is still open, LSPs cannot "move" from one SR > Policy to another. Right? However, is it OK to transition from not having an > SRPA to having one or vice versa? <MK> "MUST be constant" and "MUST NOT change" sounds the same to me. KT> Sure. However, the change is more than that - it uses the LSP object as the base/reference. <S> Can we apply same as described above – clarify in terminology and then use: Candidate Paths within an SR Policy MUST NOT change their SR Policy Identifiers. Or Candidate Paths within an SR Policy MUST NOT change their SR Policy Identifiers for the lifetime of the PCEP session. One of previous versions of the draft was using “lifetime of the LSP” and there were comments received that “lifetime” is not defined. Should be add "MUST be present" or something as well? KT> That was my question. It is up to the authors/WG to decide. If we say something along the lines of "SRPA MUST be present for all LSPs related to SR Policies", does that mean that error would get logged if any SR related encodings (e.g., SR EROs) are used with RFC8664 style when the SR Capability is supported by both PCEP speakers? <S> Yes, we have to add it, but “related to SR Policies” seems to be ambiguous. What about: “SRPA MUST be present for all SR Policy LSPs.” as “SR Policy LSP” is already defined in terminology as LSP with PST 1 or PST 3. I would further add here that this ID MUST be consistent among all the PCEP sessions. I.e., you cannot report different IDs in different PCEP sessions. KT> Sure. I agree. <S> Agreed, but it should apply only if ID is really reported - if no ID is reported to one session (e.g. because extension is not supported), but is reported to other one, then that should be considered as inconsistency. </MK> > CURRENT > SR Policy Identifier MUST be different for different SRPAs. > > What does this mean exactly? The SR Policy Indentifier is carried in the > Extended Association ID TLV and so it is part of the key of the Association > object. Isn't this obvious, can it be otherwise? <MK> It's basically saying "the SR Policy ID MUST be unique", we can remove if confusing. </MK> KT> Yes, please consider removing since it seemed superfluous to me - unless I am missing something. <S> Yes, we can remove. > CURRENT > If the identifier is inconsistent among Candidate Paths, changes during the > lifetime of the PCEP session, or is not unique across different SRPAs, the > receiving PCEP speaker MUST send a PCEP Error (PCErr) message ... > > I understand the part of changes (see my > suggestion above). But what is meant by "inconsistent" here? Also, I don't > understand the "not unique" part. <MK> I just meant this as a catch-all term for various corner cases. We can probably just say "If the above conditions are not satisfied, the receiving PCEP speaker MUST send a PCEP Error..."? KT> That would be ok by me. It made me wonder if "inconsistent" meant something else. <S> Ack, we can change it to statement suggested by Mike. </MK> > > Similar questions for the blob below in 3.2 > > CURRENT > SR Policy Candidate Path Identifier MUST be constant for the lifetime of the > PCEP session. SR Policy Candidate Path Identifier MUST be different for > distinct Candidate Paths within the same SRPA. If SR Policy Candidate Path > Identifier changes during the lifetime of the PCEP session or is not unique > among distinct Candidate Paths, the PCEP speaker MUST send a PCErr message > with > Error-Type = 26 "Association Error" and Error Value = 21 "SR Policy Candidate > Path Identifier Mismatch". <MK> This section is about a different field in the protocol, it's about the "SR Policy Candidate Path Identifier" instead of "SR Policy Identifier". We are just stating some basic rules about this key field. I don't find anything confusing/wrong here. Can you explain what your specific concern is about this paragraph? </MK> KT> Pretty much the same/similar things like we are discussing above for section 3.1 - whatever is the conclusion, similar updates may need to be applied to this section as well. <S> Ack, same can be applied to this section as well. > > <discuss-2> Section 4.2.2 > > > CURRENT > Originator Address: Represented as a 128-bit value as specified in Section 2.4 > of [RFC9256]. When sending a PCInitiate message, the PCE is acting as the > originator and therefore MUST set this to an address that it owns. > > Assume a dual/redundant PCE deployment. The use of MUST above will result in > double the number of LSPs at the PCC - one from each PCE. Is it possible to > use a common (anycast) IP address as the originator from each PCE while they > have their own/individual IP addresses for the session establishment? Perhaps > something to allow/enable as a configuration option? Is this something that > needs to be considered? <MK> We can think/discuss a bit more on this. Yes it can result in double the number of Candidate Paths in an SR Policy, but isn't that the point of redundancy? KT> PCEP has the redundancy model for RSVP-TE LSPs without requiring double the LSPs. So, why isn't the same possible for SR Policies? These are just entries in the PCC database, and PCC would choose one of these entries to actually carry the traffic. KT> If we follow the SR Policy Architecture (RFC9256), these duplicate CPs will find their way to the SRPM module (outside of the conceptual PCEP module). This may require the validation and path computation for double the LSPs. If one of the PCEs disconnects, the Candidate Paths that it owns can be wiped, but the redundant PCE's Candidate Paths would remain at the PCC, thus providing redundancy. But I do agree that MUST may be too limiting and can block future development of the protocol, we can change it to MAY to allow different redundancy models or perhaps a usecase that we didn't think of today. KT> Sounds good to me. I will leave the recommended way to the authors/WG - the point of this discussion was to check if the other option is not desired to be left open/available. <S> Ack, changing to MAY seems fine to me as well. </MK> > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > I would also appreciate responses/updates for the following comments that are > inline in the idnits format output for the v24 of the document. > > 120 Path Computation Element Communication Protocol (PCEP) Extensions for > 121 Segment Routing [RFC8664] specifies extensions to the PCEP that allow > 122 a stateful Path Computation Element (PCE) to compute and initiate > 123 Traffic Engineering (TE) paths, as well as a Path Computation Client > 124 (PCC) to request a path subject to certain constraints and > 125 optimization criteria in SR networks. Although PCEP extensions > 126 introduced in [RFC8664] are defined for creation of SR-TE tunnels, > > <minor> I didn't find RFC8664 using the term "SR-TE tunnels". Instead, it uses > > the term "SR-TE paths". > > 127 these are not SR Policies and lack many important features described > 128 in [RFC9256]. <S> We can change to SR-TE paths. > > <major> These "important features" are not listed in this document or anywhere > > else. It would be good to list at least some of them as a bullet list to guide > readers on features that would be missed by an implementation/deployment that > is only supporting RFC8664/RFC9603. <S> Ack, we can list some of them. > > <major> Are there any backwards compatibility considerations for these > > specifications as compared to RFC8664/9603? E.g., LSPs that were previously > reported without SRPA now start getting reported with the SRPA after a PCC > upgrade to a newer software version? <MK> I think we can say that it's not compatible with SR-TE Paths. I.e., you cannot have both versions running at the same time and when you upgrade you have to upgrade both PCC and PCE. </MK> KT> Please consider if there is a way to achieve the enablement of this feature without a large scale service impact - especially in a dual/redundant PCE deployment. <S> Do we really need to mention “when you upgrade you have to upgrade both PCC and PCE”? Isn’t that clear from capability handling anyway – if any of them does not support extension, then implementation cannot used SRPA (that is already clearly mentioned). So extension will be enabled only when both PCEP peers are upgraded. > 189 SR Policy LSP: An LSP set up using Path Setup Type [RFC8408] 1 > 190 (Segment Routing) or 3 (SRv6). > > <major> The use of LSP (label switched path) for SRv6 sounds odd. I am aware > > that the term LSP in integral to PCEP though. However, some clarification for > the term as done in section 2 of RFC9603 would be helpful. <MK> Yes, the Label Switched Path is not really "Label Switched" and it's not even a "Path" (it can be multiple paths or even just a Color pointer). We can add a quick note, but would have been better in the PCEP SRv6 document(s) instead. </MK> KT> It is already there in the PCEP SRv6 RFC9603 - I am simply suggesting to borrow the same text here as well. <S> Yes, we can add something similar to terminology: “Further, note that the term "LSP" used this draft would be equivalent to an SRv6 path (represented as a list of SRv6 segments) in the context of supporting SRv6 in PCEP.” > 192 SR Policy Association: A new association type used to group > 193 candidate paths belonging to same SR Policy. Depending on the > 194 discussion context, it can refer to the PCEP ASSOCIATION object of > 195 SR Policy type or to a group of LSPs that belong to the > 196 association. > > [minor] Suggest to add BGP-LS here with the reference to the base RFC9552 as > informational due to use in section 4.2.2. I also found IGP and ERO acronyms. > > 198 3. Overview > > <minor-editorial> This section talks about the use of SRPA and error handling > > while the SRPA is introduced in section 4. Could you please consider swapping > their order for better readability? OR moving the error handling portions > under the relevant sub-sections of section 4. > > 200 The SR Policy is represented by a new type of PCEP Association, > 201 called the SR Policy Association (SRPA) (see Section 4). The SR > 202 Candidate Paths of an SR Policy are the LSPs within the same SRPA. > <S> I assume that we don’t want to have “Overview” in the middle of the draft, so can we then just move 3.1/3.2/3.3 into section 4 then? > <major> The correct term is 'SR Policy Candidate Path' and not 'SR Candidate > > Path' per RFC9256. Please fix/correct in a few other places as well. > > 203 Encoding multiple Segment Lists within an SR Policy Candidate Path is > 204 described in [I-D.ietf-pce-multipath]. These considerations are not > 205 covered here. <S> Ack, we can change it. > > <minor> Perhaps ... The extensions in this document specify the encoding of a > > single segment list within an SR Policy Candidate Path. Encoding of multiple > segment lists is outside the scope of this document and specified in > [I-D.ietf-pce-multipath]. > > 217 3.1. SR Policy Identifier > > 219 SR Policy Identifier uniquely identifies an SR Policy [RFC9256] > 220 within the network. SR Policy identifier is assigned by PCEP peer > > <major> s/network/SR domain <S> Ack. > > > 492 Discriminator: 32-bit value that encodes the Discriminator of the > 493 Candidate Path, as specified in Section 2.5 of [RFC9256]. This is > 494 the field that mainly distinguishes different SR Candidate Paths, > 495 coming from the same originator. It is allowed to be any number in > 496 the 32-bit range. > > <major> Perhaps ... it is a 32-bit unsigned integer value. This is also a more > > general comment applicable in other similar places. <S> Ack, we can change it. > > 552 5. SR Policy Signaling Extensions > > 554 This section introduces mechanisms that are described for SR Policies > 555 in [RFC9256] to PCEP, but which do not make use of the SRPA for > 556 signaling in PCEP. Since SRPA is not used, there needs to be a > 557 separate capability negotiation. > > <major> I found the above hard to parse. Is the intention here to state that > > since PCEP extensions for SR-TE path signaling have been around and these are > new extensions for SR Policy signaling (compliant to [RFC9256]), there is a > need to introduce capability negotiations? <MK> It's saying that there are some parts of this draft that don't use SRPA and therefore cannot rely on the PCEP Association capability negotiation (ASSOC-Type-List). So an additional capability negotiation mechanism is required. </MK> KT> OK. Please consider rephrasing for clarity - you could wordsmith my comment above. <S> What about just changing to: This section introduces mechanisms described that are described for SR Policies in [RFC9256]. These extensions do not make use of the SRPA for signaling in PCEP therefore cannot rely on the Association capability negotiation (ASSOC-Type-List) and separate capability negotiation is required. > > 626 5.2. Computation Priority TLV > > <minor-editorial> Suggest 5.2 to be LSP Object TLVs and for 5.2 through 5.4 to > > be indented under it. Or something similar, for clarity on the different types > of extensions. <MK> Yes, that would be good. </MK> <S> Ack. > > 628 The COMPUTATION-PRIORITY TLV (Figure 7) is an optional TLV for the > 629 LSP object. It is used to signal the numerical computation priority, > 630 as specified in Section 2.12 of [RFC9256]. If the TLV is absent from > 631 the LSP object and the P-flag in the SRPOLICY-CAPABILITY TLV is set > 632 to 1, a default Priority value of 128 is used. > > 634 0 1 2 3 > 635 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 > 636 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > 637 | Type | Length | > 638 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > 639 | Priority | Reserved | > 640 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > 642 Figure 7: COMPUTATION-PRIORITY TLV Format > > 644 Type: 68 for "COMPUTATION-PRIORITY" TLV. > > 646 Length: 4. > > 648 Priority: Numerical priority with which this LSP is to be recomputed > 649 by the PCE upon topology change. Lowest value is the highest > 650 priority. > > <major> I assume 8-bit unsigned integer values? Is zero allowed? <MK> We should add a pointer to rfc9256 Section 2.12, if not already done. </MK> KT> The reference is there in the section but not against the field. The convention in the document seems to be putting the reference for the fields as well, which is very clear. <S> We can clarify that it is 8-bit unsigned integer. Since 0 is valid value, then I don’t see reason for explicitly mentioning it. Thanks, Ketan > > > 652 Reserved: This field MUST be set to zero on transmission and MUST be > 653 ignored on receipt. > > 907 6.4. TE-PATH-BINDING TLV Flag field > > 909 An earlier version of this document added new bit within the "TE- > 910 PATH-BINDING TLV Flag field" registry of the "Path Computation > 911 Element Protocol (PCEP) Numbers" registry group, which was also early > 912 allocated by the IANA. > > 914 IANA is requested to cancel the early allocation made which is not > 915 needed anymore. As per the instructions from the chairs, please mark > 916 it as deprecated. > > <minor> Suggest .. IANA is requested to mark the bit position as deprecated > <S> Sure, I’m fine with that. > as follows: > > That said, I will leave it to the IANA team. > > 918 +------------+------------------------------------------+-----------+ > 919 | Bit position | Description | Reference | > 920 +--------------+----------------------------------------+-----------+ > 921 | 1 | Deprecated (Specified-BSID-only) | This.I-D | > 922 +--------------+----------------------------------------+-----------+ > > 970 6.7. SR Policy Capability TLV Flag field > > 972 This document requests IANA to maintain a new registry under "Path > 973 Computation Element Protocol (PCEP) Numbers" registry group. The new > 974 registry is called "SR Policy Capability TLV Flag Field". New values > 975 are to be assigned by "IETF review" [RFC8126]. Each bit should be > 976 tracked with the following qualities: > > 978 * Bit (counting from bit 0 as the most significant bit). > > 980 * Description. > > 982 * Reference. > > <major> The section 5.1 has alphabetical identifiers for these bits. Strongly > > suggest to retain the same in the IANA table below in addition to the > description. > > 984 +--------+-----------------------------------------------+-----------+ > 985 | Bit | Description | Reference | > 986 +--------+-----------------------------------------------+-----------+ > 987 | 0 - 26 | Unassigned | This.I-D | > 988 +--------+-----------------------------------------------+-----------+ > 989 | 27 | Stateless Operation | This.I-D | > 990 +--------+-----------------------------------------------+-----------+ > 991 | 28 | Unassigned | This.I-D | > 992 +--------+-----------------------------------------------+-----------+ > 993 | 29 | Invalidation | This.I-D | > 994 +--------+-----------------------------------------------+-----------+ > 995 | 30 | Explicit NULL Label Policy | This.I-D | > 996 +--------+-----------------------------------------------+-----------+ > 997 | 31 | Computation Priority | This.I-D | > 998 +--------+-----------------------------------------------+-----------+ > <S> Sure, we can change it. > < EoR v24 > > > > > > _______________________________________________ > Pce mailing list -- pce@ietf.org<mailto:pce@ietf.org> > To unsubscribe send an email to pce-le...@ietf.org<mailto:pce-le...@ietf.org>
_______________________________________________ Pce mailing list -- pce@ietf.org To unsubscribe send an email to pce-le...@ietf.org
