Hi Sue, Thanks for your review.
On Fri, May 24, 2024 at 12:32 AM Susan Hares via Datatracker < nore...@ietf.org> wrote: > Reviewer: Susan Hares > Review result: Ready with Issues > > GEN-ART review > > Status: Ready with Issues > Type of issues: Technical issues + Editorial issues > > General Comment: The authors should be commended for undertaking the > massive challenge of collecing the use cases for a PCE as a Central > Controller. > This work will help guide future PCE work and interactions with other > protocols. > > 🙏 > Summary of technical issues: > > 1. BGP interactions are skipped in this document except for section 3.5 > Section 3.5 does not clearly indicate the BGP Peer relationships with > the RR. > Interactions of the PCE with the BGP Flow specification should be > consider in > the PCE discussion for IP Native, SFC, SR, and Best Effor tPaths. > > Dhruv: I have added some text. Also added a disclaimer - "It is interesting to note that some of the use cases listed can also be supported via BGP instead of PCEP. However, within the scope of this document, the focus is on the use of PCEP." > 2. SRPM calculations to calculate Active Policy from a set of Candidate SR > Policy is not clearly mentioned > > Dhruv: Added "<t>Note that an SR Policy can be associated with multiple candidate paths. A candidate path is selected when it is valid and it is determined to be the best path of the SR Policy as described in <xref target="RFC9256"/>.</t>" > 3. Clear delination of which forwarding is being used IP native, IP native > over MPLS, SR-MPLS, or SRv6 > is critical at each section. See editorial issues for the issues I > found. > > Dhruv: The text has been updated, thanks for pointing out. > 4. The inclusion of individual drafts in the informative text is > beneficial, > but > care must be taken to indicate whether the PCE WG is going to adopt any > particular solution. > > Dhruv: Some of them are in the adoption queue but that does not guarantee they would gain consensus. I have made updates and made sure that those references remain informative-useful-pointers without claiming to be definitive extensions. > > Editorial > 1. Abstract, general comment > > The abstract is 3 paragraphs which is 1 paragraph longer than normal. > The authors might look at shortening this abstract. > > Dhruv: It borrows the structure from the related RFCs 8283 and 9050. I prefer keeping it in the same framing if that's okay. > 2. Abstract, paragraph 2, > > Issue: English Grammar, clarity > Old text: / > SDN has much broader applicability than signal MPLS traffic- > engineered (TE) paths and the PCE may be used to determine paths in a > range of use cases including static Label Switched Paths (LSPs), > Segment Routing (SR), Service Function Chaining (SFC), and most forms > of a routed or switched network. / > > New text: / > SDN has much broader applicability than just signaling MPLS traffic- > engineered (TE) paths. The PCE functions may be used to determine paths > in a range of use cases including static Label Switched Paths (LSPs), > Segment Routing (SR), Service Function Chaining (SFC), and most forms > of a routed or switched network. / > > Dhruv: I aligned it to RFC8283. > 3. introduction, paragraph 1 > Issue: Grammar, clarity. > > Old text: / The role and function of PCE have grown to cover several > other uses (such as GMPLS [RFC7025], Multicast), and to allow > delegated stateful control [RFC8231] and PCE-initiated use of network > resources [RFC8281]./ > > Old text: / The role and function of PCE have grown to cover several > other uses (such as GMPLS [RFC7025] or Multicast), and to allow > delegated stateful control [RFC8231] and PCE-initiated use of network > resources [RFC8281]./ > > Ack > 4. Terminology: > > I suggest you add the following definitions to aid readability. > > Centralized Controller, BGP-LS [RFC9952], SR, Failure recovery, node-SID, > Adj-SID, LSP PST, SRGB, SRLB, failure recovery, > > Dhruv: I have added a few of these. > 5. Title in 3.1 > > old title:/ PCEEC for Label Management/ > New title:/ PCE for MPLS Label Management/ > > Ack > 6. section 3.1, paragraph 3, > Why: Need reference for BGP-LS > old text:/ > The full SRGB/SRLB of a node could be learned via existing > IGP or BGP-LS mechanisms too./ > > New text:/ > The full SRGB/SRLB of a node could also be learned via existing > IGP or BGP-LS [RFC9552] mechanisms./ > > Ack > 7. section 3.2, paragraph 1. > > Why: provide reference to Segment routing the first time used. > > old text:/Segment Routing (SR) leverages the source routing paradigm./ > New text:/Segment Routing (SR) [RFC8402] leverages the source routing > paradigm. > / > > Ack > 8. section 3.2 paragraph 2, > > issue: Missing reference to inter-domain. SR and the rest of the document > mentions inter-domain. It should be added here for clarity. > > Old text:/ > A database of segments can be distributed > through the network using a routing protocol (such as IS-IS or OSPF) > or by any other means. PCEP (and PCECC) could also be one of them./ > > new text: / A database of segments can be distributed > through the network using a intra-domain routing protocol (such as > IS-IS or > OSPF) or an inter-domain protocol (BGP), or by any other means. PCEP > (and > PCECC) c ould also be one of other protocols. / > > Ack, with slight modification. > 8. section 3.2 paragraph 4, > issue: no references for SR-TE. Which specificaion do you mean? > > old text:/[RFC8355] identifies various protection and resiliency usecases > for > SR. Path protection lets the ingress node be in charge of the > failure recovery (used for SR-TE)./ > Used RFC 8664. > > 9. section 3.2.1, title and text > > issue: The text taughts about label allocation and adds at the end > > -15-text: /The forwarding behaviour and the end result are similar to > IGP-based > Adj- > SID allocation and usage in SR./ > > If you are allocating SID for SR-MPLS and SRv6, this section has confusing > text. I suggest you rework to either be SR-MPLS and SRv6. > > If this is not your intent, rename the title to PCEE SID Allocation for > SR-MPLS. > > Ack, updated the title. > 10. section 3.2.2 - description of packets, title. > > Issue: What do you mean by "top of the packet"? Does this fit SR-MPLS and > SRv6? > > Old text-1:/ The ingress router > of the forwarding path needs to encapsulate the destination Node-SID > on top of the packet. / > > old-text-2:/ > R1 may send a packet to R8 simply by pushing an SR label with segment > {1008} (Node-SID for R8). / > > If you are just doing SR-MPLS, then change both the text and the title. > > Ack, updated text/title. > 11. section 3.2.3, paragraph 1, sentence 3 - run-on sentence > > Grammar: Run on sentence. A semi-colon mans the two half of the sentence > are > equal. > "hence" gives a conclusion which means the two halfs of the sentence are > not > equal. If __-, hence ___. > > Old text:/ The header has all necessary information so that, the > packets can be guided from the ingress node to the egress node of the > path; hence, there is no need for any signalling protocol. / > > New text:/The header has all necessary information so that the > packets can be guided from the ingress node to the egress node of the > path. Hence, there is no need for any signalling protocol. / > > [professor hat on]: (explanation) Grammar rules suggest the > removal of the semi-colon and addition of a period. This creates two clear > sentences linked together with hence. [professor hat off] > > Ack. > 12. section 3.2.2, paragraph 3 > > issue: Support in 3.2.1 for the allocations for SRv6 is weak > > Current text:/ > To achieve this, the PCECC first allocates and distributes SIDs as > described in Section 3.2.1. [RFC8664] describes the mechanism for a > PCE to compute, update, or initiate SR-TE paths./ > > What: SRv6 is included in [RFC8664]? if you decide to make 3.2.1 only > SR-MPLS, you will need to change this section. > > Ack, made the text about SR-MPLS > 13. section 3.2.3.1, paragraph 6 > > Issue: reference clarity and sentence clarity. > current text:/ The procedure: > > PCECC allocates Node-SIDs and Adj-SIDs as described in Section 3.1 > for all nodes and links./ > > New text:/ > The procedure: > > PCECC allocates Node-SIDs and Adj-SIDs using the mechanism described > in Section 3.2.1 for all nodes and links./ > > Problem: Allocations of Node-SIDs and Adj-SIDs are described in section > 3.2.1. > If this is the correction section then the new text above fits. > If it is not, please rework the sentence. > > Ack > 14. Section 3.2.3.1, list of policies: > > Issue: The calculation of the active SR Policy from a set of > SR Candidate policies is missing. (technical or editorial) > > Current Text:/ > PCECC will form both SR Policies POL1 and POL2. > > PCECC will send both POL1 and POl2 to R1 via PCEP. > > PCECC optionally can allocate BSIDs for the SR Policies. > / > > Dhruv: I have added this text at the end "<t>Note that an SR Policy can be associated with multiple candidate paths. A candidate path is selected when it is valid and it is determined to be the best path of the SR Policy as described in <xref target="RFC9256"/>.</t>" > 15. Section 3.2.4 > issue: State "will be" for an individual drafts > > Current text: /PCECC PCEP extensions > for SRv6 [I-D.dhody-pce-pcep-extension-pce-controller-srv6] will be > used for that./ > > Possible text: / An example of how PCECC PCEP extensions could be > extended for SRv6 is in: > [I-D.dhody-pce-pcep-extension-pce-controller-srv6]./ > > Ack > 16. section 3.4, paragraph 3 > > issue: draft-ietf-idr-mpls-seamless-mpls is a "dead" document. > What to do: Check with MPLS WG to see what is replacing this document. > > It is still the best informative reference for seamless MPLS > 17. section 3.5, BGP reference > > old text:/ * Since PCECC needs to know the topology of both domains AS1 > and > AS2, PCECC can utilize the BGP-LS peering with routers (or RRs) in > both domains./ > > New text:/ * Since PCECC needs to know the topology of both domains > AS1 and > AS2, PCECC can utilize the BGP-LS peering with BGP routers (or RRs) > in > both domains./ > > Ack > 18. section 3.5 > issue: Clarity of what is done for SRv 6, paragraph 8, bullet 4 > > old text:/ > * PCECC will send PCInitiate message [RFC8281] towards the ingress > router R1 (PCC) in AS1 and receive the PCRpt message [RFC8231] > back from it. If the PST is PCECC-SR, the PCECC will include the > SID stack as per [RFC8664]. Optionally, a binding SID or BGP > Peering-SID [RFC9087] can also be included on the AS boundary. > The backup SID stack can be installed at ingress R1 but more > importantly, each node along the SR path could also do the local > protection just based on the top segment. If the PST is PCECC > (basic), when the PCECC will assign labels along the calculated > paths (R1-ASBR1-ASBR3-R3, R1-ASBR5-ASBR6-R3); and set up the path > by sending central controller instructions in PCEP message to each > node along the path of the LSPs as per [RFC9050]. Then PCECC will > send a PCUpd message to the ingress R1 router with information > about new LSPs and R1 will respond by PCRpt with LSP(s) status./ > > Suggested change:/ > * PCECC will send PCInitiate message [RFC8281] towards the ingress > router R1 (PCC) in AS1 and receive the PCRpt message [RFC8231] > back from it. > * If the PST is PCECC-SR, the PCECC will include the > SID stack as per [RFC8664]. Optionally, a binding SID or BGP > Peering-SID [RFC9087] can also be included on the AS boundary. > The backup SID stack can be installed at ingress R1 but more > importantly, each node along the SR path could also do the > local protection just based on the top segment. > * If the PST is PCECC (basic), the PCECC assigns labels > along the calculated paths (R1-ASBR1-ASBR3-R3, > R1-ASBR5-ASBR6-R3) and sets up the path by sending > central > controller instructions in PCEP message to each node > along > the path of the LSPs as per [RFC9050]. After these > steps, > the PCECC will send > a PCUpd message to the ingress R1 router with information > about new LSPs and R1 will respond by PCRpt with LSP(s) status./ > > Ack > 19. Section 3.6.1, step 2 sentence. > Issue: Grammar - spacing and period in the sentence. > > old text:/ Step 2: When R2 receives the packet with label 9000, it will > forward it to R4 by swapping label 9000 to 9001 and at the same > time, it will replicate the packet and swap the label 9000 to > 9002 and forward it to R5/ > > New text:/Step 2: When R2 receives the packet with label 9000, it will > forward it to R4 by swapping label 9000 to 9001 and at the same > time, it will replicate the packet and swap the label 9000 to > 9002 and forward it to R5./ > > Ack > 20. Section 3.7, paragraph 1, sentence 2 > > Old text: It applies in many scenarios including MPLS > traffic engineering (where it determines what traffic is forwarded > into which LSPs); segment routing (where it is used to select which > set of forwarding instructions (SIDs) to add to a packet); SFC (where > it indicates how a packet should be forwarded across which service > function path )./ > > New text: It applies in many scenarios including the following: > a) MPLS traffic engineering (where it determines what traffic > is forwarded into which LSPs), > b) segment routing (where it is used to select which > set of forwarding instructions (SIDs) to add to a packet), > c) SFC (where it indicates how a packet should be forwarded > across which service function path ). / > > Ack > 21. Section 3.8 + 3.9 - missing content for Flow filters - BGP Flow > Specification + configured > > The technical issue about SFC flow classification via > configured filters or BGP Flow Specification filters could be resolved > in this section. > > Similarly, the Flow specification filters might be mentioned in section > 3.9. > > It would be good to know how to combine PCECC and these other common > functions. > > Dhruv: Added this text in 3.7 - "<t>The description of traffic flows by the combination of multiple Flow Specification components and their dissemination as traffic flow specifications (Flow Specifications) is described for BGP in <xref target="RFC8955"/>". When a PCECC is used to initiate tunnels (such as TE-LSPs or SR paths) using PCEP, it is important that the head end of the tunnels understands what traffic to place on each tunnel. <xref target="RFC9168"/> specifies a set of extensions to PCEP to support the dissemination of Flow Specification components where the instructions are passed from the PCECC to the routers using PCEP.</t>" Added a reference to Section 3.7 in Section 3.8. I did not make a change in Section 3.9 as the text in Section 3.7 applies to all path setup types including Native-IP. > 22. informative references > > The AD and Shephed should evaluate the use of: > > 1) individual drafts - as indicating a direction for PCE or just an > example, > 2) draft-ietf-mpls-seamless-mpls - as it is a IESG dead draft > > Dhruv: Not directed towards the authors but I made text changes for 1) and 2) has been informatively referenced before and is still the best reference for seamless MPLS. > 23. Appendix A. > Do the proposed phases for MapReduce-Hadoop align with PCE milestones and > plans? > > > Dhruv: There are no changes in the protocol required it is an applicability statement or use case in a new environment where PCEP is not typically used. Diff: https://author-tools.ietf.org/iddiff?url1=draft-ietf-teas-pcecc-use-cases-15&url2=draft-ietf-teas-pcecc-use-cases-16&difftype=--html Thanks, Dhruv
_______________________________________________ Gen-art mailing list -- gen-art@ietf.org To unsubscribe send an email to gen-art-le...@ietf.org