Hi All, I've come across another "shared" registry between this and another IDR document that also needs some update to this document. Refer https://www.ietf.org/archive/id/draft-ietf-idr-sr-policy-safi-03.html#section-6.10
Request authors to please align the IANA section in this document also with the above IDR document. Thanks, Ketan On Mon, Apr 8, 2024 at 5:33 PM Ketan Talaulikar <ketant.i...@gmail.com> wrote: > Hi Dhruv, > > I've also posted my review of the document from the POV of consistency > with SR Policy Arch and BGP SR Policy SAFI documents > > https://mailarchive.ietf.org/arch/msg/pce/xExbrHXRsC66fGUdy5wtvQlbXc4/ > > I hope these help improve the document by providing a better overall > context of these very important PCEP extensions for SR. > > Thanks, > Ketan > > > On Sat, Apr 6, 2024 at 7:46 PM Dhruv Dhody <d...@dhruvdhody.com> wrote: > >> Hi Mike, Authors, >> >> Please make a new version of the I-D where you handle the following >> items. We can then send the I-D to the IESG. >> >> (1) Please handle Ketan's concern and add the IANA note as he suggested - >> https://mailarchive.ietf.org/arch/msg/pce/ZaYof63GNYdplFUOLo6G6hJlx3c/ >> >> (2) A few comments/query got missed, please update or respond if no >> changes are needed... >> >> - Section 5.2, I am unsure about the interaction between the unsetting of >> P-flag (PCEP speaker does not support the TLV) and the default value (128 >> when the TLV is not present). Isn't it a bit weird? >> - Section 5.4, Should Oper/Config get a registry for ease of adding new >> flags in future? >> >> (3) Some new comments on checking the diff >> >> - abstract, s/[RFC8231]/RFC 8231/ (no references in abstract) >> - s/ANY/any/ >> - RFC 7525 is obsolete by RFC 9325, please update! >> >> (4) I am working on the shepherd writeup - >> https://notes.ietf.org/HziLkaoxS6iYoQ3sOcwk-A?view ; will update in the >> datatracker once you post a new version handling these. >> >> Thanks! >> Dhruv >> >> On Mon, Mar 18, 2024 at 7:40 AM Mike Koldychev <mkold...@proton.me> >> wrote: >> >>> Hi Dhruv, >>> >>> I've incorporated your changes and all the other comments that I have >>> received so far. Hopefully I didn't miss anything. Version 15 is uploaded. >>> Thanks a lot for your comments and updates! >>> >>> Thanks, >>> Mike. >>> >>> On Saturday, March 9th, 2024 at 8:23 AM, Dhruv Dhody <d...@dhruvdhody.com> >>> wrote: >>> >>> Hi Authors, >>> >>> I have finished the shepherd review of >>> draft-ietf-pce-segment-routing-policy-cp-14. Please handle these comments >>> before we ship this I-D to IESG. >>> >>> ## Major >>> - Section 5.6, you need to add update: RFC 8231 in the draft metadata. >>> This should also be captured in the abstract. The prefered way is to >>> clearly identify the text in RFC8231 that is changing with "OLD:" and >>> "NEW:" format! >>> - Section 8, Security considerations need to also cover the non-SRPA >>> TLVs which are not considered in the current text. >>> >>> ## Query >>> - Section 4.1, >>> ```` >>> If the PCC receives a >>> PCInit message with the Association Source set not to the headend IP >>> but to some globally unique IP address that the headend owns, then >>> the PCC SHOULD accept the PCInit message and create the SRPA with the >>> Association Source that was sent in the PCInit message. >>> ```` >>> What is the purpose of this text? PCC should use the source as set by >>> the PCE - isn't it given? Am I missing something? Boris also pointed this >>> out in his review. >>> >>> >>> ## Minor >>> - Abstract is not very useful for a non-expert. Maybe change something >>> like - >>> ```` >>> OLD: >>> A Segment Routing (SR) Policy is a non-empty set of SR Candidate >>> Paths, which share the same <headend, color, endpoint> tuple. SR >>> Policy is modeled in PCEP as an Association of one or more SR >>> Candidate Paths. PCEP extensions are defined to signal additional >>> attributes of an SR Policy. The mechanism is applicable to all SR >>> forwarding planes (MPLS, SRv6, etc.). >>> NEW: >>> Segment Routing (SR) allows a node to steer a packet flow along any >>> path. SR Policy is an ordered list of segments (i.e., >>> instructions) that represent a source-routed policy. Packet flows >>> are steered into an SR Policy on a node where it is instantiated >>> called a headend node. An SR Policy is made of one or more candidate >>> paths. >>> >>> This document specifies Path Computation Element Communication >>> Protocol (PCEP) extension to associate candidate paths of the SR >>> Policy. It applies equally to the SR-MPLS and Segment Routing over >>> IPv6 (SRv6) instantiations of segment routing. >>> END >>> ```` >>> - Similarly I find Introduction to be very light on details. Consider >>> adding text by looking through recently published RFCs for instance. >>> - Terminology: >>> ``` >>> OLD: >>> SRPA: SR Policy Association. PCEP ASSOCATION that describes the SR >>> Policy. Depending on discussion context, it refers to a PCEP >>> object or to a group of LSPs that belong to the Association. >>> NEW: >>> SRPA: SR Policy Association. A new association type 'SR Policy >>> Association' is used to group candidate paths belonging to the SR >>> Policy. Depending on discussion context, it can refer to the PCEP >>> ASSOCIATION object of SR Policy type or to a group of LSPs that >>> belong to the association. >>> END >>> ``` >>> - Section 4, please add this text at the start - >>> ```` >>> As per [RFC8697], LSPs are associated with other LSPs with which they >>> interact by adding them to a common association group. As described >>> in [RFC8697], the association group is uniquely identified by the >>> combination of the following fields in the ASSOCIATION object: >>> Association Type, Association ID, Association Source, and (if >>> present) Global Association Source or Extended Association ID, >>> referred to as Association Parameters. >>> ```` >>> - Section 4.2, since none of the TLV are multi-instance. Can we simplify >>> this text - >>> ```` >>> OLD: >>> Unless specifically stated otherwise, the TLVs listed in the >>> following sub-sections are assumed to be single instance. Meaning, >>> only one instance of the TLV SHOULD be present in the object and only >>> the first instance of the TLV SHOULD be interpreted and subsequent >>> instances SHOULD be ignored. >>> NEW: >>> This document specifies four new TLVs to be carried in the SRPA object. >>> Only one TLV instance of each type can be carried, and only the first >>> occurrence is processed. Any others MUST be ignored. >>> ```` >>> Also applicable to section 5! >>> - Section 4.2.2, consider changing the SHOULD to MUST in this section. I >>> could not think of a justification for SHOULD here! >>> - Section 5.1, >>> - please also state what happens if the TLVs are used without the >>> exchange of SRPOLICY-CAPABILITY TLV or the corresponding bit is unset. >>> Without it, what is the use of adding this TLV? >>> - Consider updating the description such as "P-flag: If set to '1' by a >>> PCEP speaker, the P flag indicates that the PCEP speaker supports the >>> handling of COMPUTATION-PRIORITY TLV for the SR Policy." >>> - please add "Unassigned bits MUST be set to '0' on transmission and >>> MUST be ignored on receipt." >>> - Section 5.2, I am unsure about the interaction between the unsetting >>> of P-flag (PCEP speaker does not support the TLV) and the default value >>> (128 when the TLV is not present). Isn't it a bit weird? >>> - Section 5.3, should the use of this TLV be limited to SR-MPLS? Also >>> can ENLP value be converted into a registery maintained at >>> https://www.iana.org/assignments/segment-routing/segment-routing.xhtml >>> which can be referred by both PCE and BGP? >>> - Section 5.4, please add "The unassigned bits in the Flag octet MUST be >>> set to zero upon transmission and MUST be ignored upon receipt." Should >>> "Invalidation Reasons Flags" get a registry for ease of adding new flags in >>> future? In general, can the text in this section be tightened a little bit? >>> Examples - be explicit on who is sending and who is receiving for instance. >>> Also, consider adding a more detailed example to show the usage of the >>> flags better alongside PCEP message exchange. >>> - Section 5.5, please add normative reference to >>> draft-ietf-pce-binding-label-sid >>> - Section 6.5, are you refereing to the registry at >>> https://www.iana.org/assignments/segment-routing/segment-routing.xhtml, >>> in which case it is called "Segment Routing" and not "Segment Routing >>> Parameters". Also better to call the new registry being added as >>> subregistry. >>> - Section 10.2, please make RFC 8253 and RFC 7525 as normative >>> references. >>> >>> ## Nits >>> - Expand PCEP and SR in the title >>> - Expand PCEP, SRv6 in the abstract >>> - Expand MBZ on first use. It is also better to state that the field is >>> ignored on receipt >>> - Section 4.2.2, add reference to RFC 9256 for Discriminator, as you >>> have done for other fields >>> - Section 4.2.4, add reference to RFC 9356 for Preference >>> - s/there needs to be a separate capability negotiation/a separate >>> capability negotiation is useful/ >>> - Expand on first use OAM/PM/BFD >>> - Section 6, please update the text in subsections where the number of >>> assignments in tables do not match the introductory text. >>> >>> I am also attaching the updated xml that could be a starting point for >>> you to work on -15 version. >>> >>> Thanks! >>> Dhruv >>> >>> >>>
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce
