Hi Samuel, Is it possible for the authors to share the diff of the proposed changes or a pointer to github (if used)? You could also just post the update, as well. The changes you proposed are split around and interrelated so looking at them as a set will likely help us progress this discussion faster.
Also, available for a call, if that helps. Thanks, Ketan On Thu, Apr 3, 2025 at 11:30 AM Samuel Sidor (ssidor) <ssi...@cisco.com> wrote: > 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> 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> 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 > > To unsubscribe send an email to pce-le...@ietf.org > >
_______________________________________________ Pce mailing list -- pce@ietf.org To unsubscribe send an email to pce-le...@ietf.org
