Looks like you need to update Chao Zhou's email address in the draft. Adrian
-----Original Message----- From: Pce <pce-boun...@ietf.org> On Behalf Of Adrian Farrel Sent: 16 August 2020 17:15 To: julien.meu...@orange.com; pce@ietf.org Cc: draft-ietf-pce-pcep-extension-for-pce-control...@ietf.org Subject: Re: [Pce] PCE WG LC for draft-ietf-pce-pcep-extension-for-pce-controller Hi Julien, WG, authors, I'm listed as one of the eight Contributors to this document, although my affiliation should read "Old Dog Consulting". I was a co-author of RFC 8283, so I am generally glad to see protocol work addressing this space. This document is almost ready to progress, but there are quite a few nits that we should take care of before asking to advance the document. After that's done, I support this document being published as an RFC. Thanks, Adrian === Figures If you make the table in Section 5.2 into "Figure 1" then you will not need to renumber Figure 2. All figures should have a caption such as you have for Figure 2. All figures should be mentioned explicitly in the text. Thus, instead of saying "as shown below" you should say "as shown in Figure 2". This is important because: - it helps the RFC Editor check that you are using all of the figures - it protects the text from edits that might move it around in relation to the figure. --- Abstract s/(G)MPLS/MPLS and GMPLS/ s/PCEP protocol extensions/PCEP extensions/ --- Introduction s/draft specify/document specifies/ --- Introduction The extension for PCECC in Segment Routing (SR) is specified in a separate draft [I-D.zhao-pce-pcep-extension-pce-controller-sr]. This paragraph is, of course, completely true. But it is absolutely unclear what it is doing here. There has been no previous discussion of SR in the document, and the only other mention of SR is in 5.5.9. I think we can probably do without this paragraph. --- 2. OLD Terminologies used in this document is same as described in the draft [RFC8283]. NEW Terminology used in this document is that same as that described in [RFC8283]. END --- 3. OLD it is assumed that label range to be used NEW it is assumed that the label range to be used END OLD A future extension could add this capability NEW A future extension could add the capability END OLD This document also allow a case where the label space is maintained by PCC itself NEW This document also allows a case where the label space is maintained by the PCC itself END --- 4. OLD Following key requirements associated PCECC should be considered when designing the PCECC based solution: NEW The following key requirements should be considered when designing the PCECC based solution: END --- 5.3 s/This document specify/This document specifies/ s//new CCI object-type/new CCI object-types/ --- 5.4 s/During PCEP Initialization Phase/During the PCEP Initialization Phase/ s/Path is setup/Path is set up/ s/sub-TLV in PCC's/sub-TLV in a PCC's/ s/PCE's OPEN message/a PCE's OPEN message/ --- 5.4 If the PCEP Speakers support the extensions of this draft but did not advertise this capability attempts a PCECC operation then a PCErr message with Error-Type=19(Invalid Operation) and Error-Value=TBD3 (Attempted PCECC operations when PCECC capability was not advertised) will be generated and the PCEP session will be terminated. This is good. But you also need to include what happens if an implementation that does not understand these extensions receives one. I suspect you can do this with a reference to another document. --- There are a couple of cases of s/a LSP/an LSP/ --- 5.5.1 s/based on PCECC mechanism/based on the PCECC mechanism/ s/LSP-IDENTIFIER TLV MUST/An LSP-IDENTIFIER TLV MUST/ s/with D flags/with D flag/ s/and set up the/and sets up the/ s/include the central/includes the central/ s/The CC-ID is uniquely identify/The CC-ID uniquely identifies/ s/two CCI object/two CCI objects/ s/PCECC LSP is same as/PCECC LSP is the same as/ s/receives PCRpt message/receives a PCRpt message/ (twice) s/The PCECC LSP are/The PCECC LSPs are/ s/In case where/In the case where/ s/label allocation are/label allocations are/ s/Similarly C bit/Similarly the C bit/ --- 5.5.1 You have: This PCUpd message is as per [RFC8231] SHOULD include the path information as calculated by the PCE. I think that you are not defining anything new here. You are just describing what 8231 already says. So how about... Per [RFC8231], this PCUpd message should include the path information calculated by the PCE. --- 5.5.1 The Ingress MAY further choose to deploy a data plane check mechanism and report the status back to the PCE via PCRpt message. This is fine, but you should probably add some explanation of why an Ingress might choose to do this. --- 5.5.1 It should be noted that in this example, the request is made to the egress node with C bit set in the CCI object to indicate that the label allocation needs to be done by the egress and it responds with the allocated label to the PCE. This is a little ambiguous. "...it responds" - what is it? Perhaps you should have: "...done by the egress, and the egress responds..." --- 5.5.2.1 s/to setup an LSP based/to set up an LSP based/ s/included in LSP object/included in the LSP object/ (twice) s/New PCEP object/A new PCEP object/ --- 5.5.2.2 If the PCC receives a PCInitiate message but does not recognize the label in the CCI, the PCC MUST generate a PCErr message with Error- Type 19(Invalid operation) and Error-Value=TBD8, "Unknown Label" and MUST include the SRP object to specify the error is for the corresponding label cleanup (via PCInitiate message). This is OK as a choice, but I wonder why you need to report this as an error. The intention of the request is to make sure that the label in the CCI is removed from the PCC. If the label is not recognised, isn't this the right result meaning that the answer should be a positive response? I'm not saying you must change this, but please consider it. --- 5.5.3 s/In order to setup/In order to set up/ OLD The PCC responds with first PCRpt message with the status as "GOING-UP" and assigned PLSP-ID. NEW The PCC responds with a PCRpt message with the status set to "GOING-UP" and carrying the assigned PLSP-ID. END s/from PCECC are send/from PCECC are sent/ s/setup operations are/setup operations are the/ s/LSP is same as/LSP is the same as/ s/the PCE SHOULD send the/the PCE SHOULD send a/ s/In case where/In the case where/ s/label allocation are made/label allocations are made/ --- 5.5.3 In case where the label allocation are made by the PCC itself (see Section 5.5.8), the procedure remains similar. Do you mean "remains similar" or "is the same"? If it is only similar then you need to describe how it is different. --- 5.5.4 ` s/modification of PCECC/modification of a PCECC/ s/In case where/In the case where/ s/label allocation are made/label allocations are made/ --- 5.5.4 In case where the label allocation are made by the PCC itself (see Section 5.5.8), the procedure remains similar. Same question as 5.5.3 --- 5.5.5 s/control over the/control over an/ s/In case of/In the case of a/ --- 5.5.9 s/the PCE via PCRpt message as per/the PCE via a PCRpt message as per/ --- 5.5.9 The PCECC capability MUST be exchanged on the PCEP session, before PCE could allocate binding label. Maybe turn this around to read... Before a PCE can allocate a binding label the PCECC capability MUST be exchanged on the PCEP session. --- 6.1 OLD the message has been extended as shown below NEW this document extends the message as shown below END --- Either in 6.1 or in section 2, you should include something like The message formats in this document are specified using Routing Backus-Naur Form (RBNF) encoding as specified in [RFC5511]. And obviously add a reference to 5511. --- 6.1 c/At max two instances/At most two instances/ --- 7.1 s/defines a new optional TLVs/defines new optional TLVs/ --- 7.1.1 OLD No flags are assigned right now. NEW No flags are defined in this document. END --- 7.3 OLD Currently, the following flag bit is defined: NEW Currently, the following flag bits are defined: END s/A PCE set this bit/A PCE sets this bit/ --- 11.3 Can you add to the definition of this registry what the size is? I think there are 32 bits available. --- 11.4 I think the name of this section is wrong. you are not creating a new registry in this section. --- 11.6 You should say how many bits this registry can track. I think it is 16. --- 11.7 It's not too important, but it might be nice to order this list by increasing error type. -----Original Message----- From: Pce <pce-boun...@ietf.org> On Behalf Of julien.meu...@orange.com Sent: 05 August 2020 17:19 To: pce@ietf.org Subject: [Pce] PCE WG LC for draft-ietf-pce-pcep-extension-for-pce-controller Hi all, This message initiates a 3-week WG Last Call on draft-ietf-pce-pcep-extension-for-pce-controller-06. Please review and share your feedback, whatever it is, using the PCE mailing list. This LC will end on Wednesday August 26, 11:59pm (any timezone). Please note that this I-D is related to draft-zhao-pce-pcep-extension-pce-controller-sr which is already in our WG adoption queue. Thanks, Dhruv & Julien ____________________________________________________________________________ _____________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce _______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce _______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce