Hi Med, Thanks a lot for your review and comments. I'll need some time to go over comments in attached document, but please see inline responses <S> at least.
Regards, Samuel -----Original Message----- From: Mohamed Boucadair via Datatracker <nore...@ietf.org> Sent: Monday, March 24, 2025 12:35 PM To: The IESG <i...@ietf.org> Cc: draft-ietf-pce-segment-routing-policy...@ietf.org; pce-cha...@ietf.org; pce@ietf.org; d...@dhruvdhody.com; d...@dhruvdhody.com Subject: Mohamed Boucadair's Discuss on draft-ietf-pce-segment-routing-policy-cp-22: (with DISCUSS and COMMENT) Mohamed Boucadair has entered the following ballot position for draft-ietf-pce-segment-routing-policy-cp-22: 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: ---------------------------------------------------------------------- Hi Mike, Siva, Samuel, Colby, Shuping, and Hooman, Many thanks for the effort put into this specification. Thanks also to Xiao for the opsdir review and to the authors for promptly addressing his comments. Please find below some few DISCUSS points, with a focus to ensure that this spec is consistent with the other various pieces of work that covers this same topic. ## Section 3 CURRENT This document does not propose any extension for the use of BSID with SR Policy; the existing behavior is documented in [RFC9604]. Are there additional considerations to take into account when used with the new objects on this document? For example, the base spec says "Candidate paths of different SR Policies MUST NOT have the same BSID", while such checks are not (at least I failed to find where) defined in 9604. <S> Specific case, which you described should be already covered with following statement from Section 5 of RFC 9604: If the binding value is valid but the PCC is unable to allocate the binding value, it MUST send a PCErr message with Error-Type = 32 ("Binding label/SID failure") and Error-value = 2 ("Unable to allocate the specified binding value"). ## Section 4.2.1 The SR policy BGP extension includes: "It is RECOMMENDED that the size of the symbolic name for the SR Policy is limited to 255 bytes. Implementations MAY choose to truncate long names to 255 bytes when signaling via BGP" I guess we should be consistent and align the various pieces, unless there are specifics to a given signaling mechanism. This is even important given that some computed paths will be passed between the two signaling channels: "Typically, but not limited to, an SR Policy is computed by a controller or a path computation engine (PCE) and originated by a BGP speaker on its behalf." <S> In BGP, recommendation to limit length to 255 was introduced, because of PDU limit, in PCEP we don't have any such limitation. We can still add statement to indicate that if PCE-Initiated LSPs are reported to BGP-LS, then limit/recommendation of 255 as specified in draft-ietf-idr-sr-policy-safi may be applied by implementations. ## Section 4.2.2: SR Policy Candidate Path Name The BGP extension has the following "It is RECOMMENDED that the size of the symbolic name for the candidate path is limited to 255 bytes. Implementations MAY choose to truncate long names to 255 bytes when signaling via BGP" I guess we should be consistent. <S> Same as above. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- My full review can be found at: * pdf: https://github.com/boucadair/IETF-Drafts-Reviews/blob/master/2025/draft-ietf-pce-segment-routing-policy-cp-22-rev%20Med.pdf * doc: https://github.com/boucadair/IETF-Drafts-Reviews/blob/master/2025/draft-ietf-pce-segment-routing-policy-cp-22-rev%20Med.doc Only the main comments are echoed here. Please refer to the review for the full set of comment, suggested edits, nits. <S> Thanks a lot, I'll go over them. ## Section 3.1 Who assigns the id? Who ensures unicity? <S> For ensuring unicity - we can update existing statement to indicate that receiving PCEP peer MUST check it - "... the receiving PCEP speaker MUST send a PCErr message ...". For assignment - it is implicitly indicated already on a few places that PCEP peer originating that LSP in PCEP (e.g. PCE for PCE-Initiated) is assigning it, but we can add statemen to specify it explicitly as well. ## Section 4.1 (1) CURRENT: * Extended Association ID TLV: Mandatory TLV of the ASSOCIATION object. As that TLV is optional in rfc8697, I think the wording should be clear this is for SR policy context. <S> Ack. (2) CURRENT: "Any others MUST be ignored" Should we log this as well? <S> I would say that behavior should be consistent with existing behavior for ignored (including unrecongnized) TLVs by PCEP speaker (https://datatracker.ietf.org/doc/html/rfc5440#section-7.1). Is there any reason define different behavior for this specific case or re-state existing behavior from RFC5440? ## Section 4.2.1 (1) Should we remind that padding is not included in the Length field? CURRENT: The TLV MUST be zero-padded so that the TLV is 4-octet aligned. <S> Ack, I'll mention it explicitly. (2) "a string of printable ASCII characters": We may cite STD80 here. <S> At least RFC9256 is already referring to [RFC0020] as reference for ASCII characters: https://www.rfc-editor.org/rfc/rfc9256.html#name-identification-of-an-sr-pol And we are already specifying that definition for the name is in that RFC, but we can add it explicitly again. ## Section 5 A more explicit title would be helpful. BTW, the flow of the document would be better if we first introduce the capabilities/session establishment and then dive into SRPA signaling. <S> It is combination of various extensions, which are applicable to SR policy, so maybe something like "SR Policy Signaling Extensions", but I'll think about alternative names. ## Section 5.1 (1) Shouldn't the following be conditioned by the activation to use the feature? CURRENT: Implementations that support SR Policy MUST include SRPOLICY-CAPABILITY TLV in the OPEN object. <S> Ack. (2) I guess that we should log the error as well. Please update. CURRENT: If a PCEP speaker receives SRPA but the SRPOLICY-CAPABILITY TLV is not exchanged, then the PCEP speaker MUST send a PCErr message with Error- Type = 10 ("Reception of an invalid object") and Error-Value = TBD ("Missing SRPOLICY-CAPABILITY TLV") and MUST then close the PCEP session. Also, consider updating similar constructs in the spec. <S> PCEP implementation should already log error events as specified in: https://datatracker.ietf.org/doc/html/rfc5440#section-8.4 Is there any reason to explicitly repeat it for this specific error? (3) Figure 6: Any reason non contiguous bits were used for these flags? <S> Backward compatibility with existing implementations as S flag (which was originally used for Specified-BSID-only) was removed in late phase of the draft. (4) The use of "PCEP speaker" is confusing in some part of the text. Please make it clear whether this is about receiving or sending. <S> Sure, I'll try to update it. ## Section 5.3 CURRENT: Note that Explicit Null is currently only defined for SR MPLS and not for SRv6. Does it make sense for SRv6 at the first place? Anyway, I would align the description with draft-ietf-idr-segment-routing-te-policy. <S> I don't see any explicit statement in draft-ietf-idr-segment-routing-te-policy indicating what implementation should do if it is received for SRv6 policy. Isn't it actually better to specify what MUST be done instead just assuming that nobody will do it? I'll check whether we can align some parts of the description at least. ## Section 5.4 CURRENT: Oper: encodes the current state of the LSP, i.e., whether it is actively dropping traffic right now. A better description is needed to reflect the use of the oper status. You may consider deleting text till "i.e.,". ## Section 6.1 The registry url should be provided instead of RFC8697. <S> Ack, I'll add it. ## Section 6.5 Given that draft-ietf-idr-bgp-ls-sr-policy is in the RFC editor queue, may be it is time to delete this request. <S> Already deleted in version 23. ## Section 9.6 First, thanks to you and the PCE WG to always supply a manageability section. That's helpful. There are bunch of extensions that are defined out there for SR policy purposes (BGP, for example). Should we consider including considerations about co-existence and interactions between those? It is tempting to have a mapping between the various SR policy objects to make sure that feature parity (or no) is supported. I'm not asking to make any change to cover this mapping, but I would be happy If you consider so. <S> I personally consider this to be out of scope of this draft and I'm thinking if such mapping will be done in any specific document, whether it is not better to have it in spring WG, so it can map interactions between extensions which are not even available in PCEP. ## References I don't think that I-D.ietf-idr-sr-policy-safi is normative. Please consider moving the entry to be listed as Informative given that the only citation is: The values of this field are specified in IANA registry "SR Policy ENLP Values" under "Segment Routing" registry group, which was introduced in Section 6.10 of [I-D.ietf-idr-sr-policy-safi]. You may replace delete "which was introduced in Section 6.10 of [I-D.ietf-idr-sr-policy-safi]." and replace it with a pointer to https://www.iana.org/assignments/segment-routing/segment-routing.xhtml#sr-policy-enlp-values. <S> Ack, I'll update it. _______________________________________________ Pce mailing list -- pce@ietf.org To unsubscribe send an email to pce-le...@ietf.org
