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. > > > > > 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. > 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? > > 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. > </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. > > > 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. > </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. > > > > > <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. > </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]. > > > > <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. > > > > <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. > > > 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. > > > 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. > > > > <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. > > > > <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 > > > > > > 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. > > > > 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. > > > > > 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> > > > > > 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. 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 > > > > 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 > +--------+-----------------------------------------------+-----------+ > > > > < 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
