Hi Julien, Thank you for your comments. We have handled the shepherd review and an update is posted. Please find the diff and responses inline. Thank you!
https://datatracker.ietf.org/doc/draft-ietf-pce-pcep-extension-for-pce-controller/ and https://www.ietf.org/rfcdiff?url1=draft-ietf-pce-pcep-extension-for-pce-controller-07&url2=draft-ietf-pce-pcep-extension-for-pce-controller-08 Best regards, Shuping > -----Original Message----- > From: julien.meu...@orange.com [mailto:julien.meu...@orange.com] > Sent: Wednesday, October 21, 2020 10:51 PM > To: draft-ietf-pce-pcep-extension-for-pce-control...@ietf.org > Cc: pce@ietf.org > Subject: Shepherd's Review of > draft-ietf-pce-pcep-extension-for-pce-controller-07 > > Hi authors of draft-ietf-pce-pcep-extension-for-pce-controller, > > Please find below my review of the aforementioned I-D. > > Regards, > > Julien > > --- > > *Main Concerns* > > - From a PCECC operation stand point, it feels odd that what is referred to as > "basic PCECC LSP setup" is actually PCC triggered, whereas the PCE-initiatied > option does exist as well. Even though it adds an additional PCInitiate > message, I would suggest to define the latter as the "basic setup", which > would not only address that comment, but would also mitigate the following > concerns on PCC-triggered setup. > - I have 2 issues with PCC-triggered setup: > * it adds LSP characteristics and egress-related information in the ingress > PCC, which is not straightforward in a PCECC context (certainly not for a > "base setup"); > * the behavior triggered by the initial PCRpt is implementation-specific in > the PCE: as per section 5.8.3 of RFC 8231, I could configure my PCE to just > store the reported info, or make it wait for whatever event before actually > starting something ("PCE *decides* to update the LSP", "the PCE *can* > modify LSP parameters of delegated LSPs"). > => Assuming the PCC-triggerred option is really needed, this limitation > should be more explicit. The order has been changed and the text is added to say that in case of PST=PCECC it SHOULD calculate and set up the path based on the PCECC technique. > - At the end of section 6.1, some errors are defined with respect to the > number of CCI objects in a message. If the ingress PCE has a particular role > (e.g. PLSP-ID allocation), how do other PCCs know if they are transit or > egress to be able to check the number of CCIs they get? > AFAIU, at 1st setup, only inconsistencies between the O bit and the number > of CCIs can trigger an error; on LSP updates the node may add an additional > check with respect to its role before the update: those situations need to be > clarified. The source and destination are part of the LSP-IDENTIFIERS TLV, that can help identify Ingress, egress and Transit. The text has been added for that. > - There is no reference to RFC 8741. Is it applicable? If so, then how? Added. > > > *Nits* > ------ > Global > --- > - Consistency on capitalization of ingress/transit/egress to be corrected. Done > ------ > Abstract > --- > - s/PCE-based central controller (PCECC)/PCE-based Central Controller > (PCECC)/ > - s/"calculated/setup"/"calculated/set up"/ > - s/each network devices/each network device/ Done > ------ > Section 1. > --- > - s/offload path computation function/offload the path computation > function/ > - s/PCE-based central controller (PCECC)/PCE-based Central Controller > (PCECC)/ > - s/such a way that, extensions/such a way that extensions/ > - s/extensions for other use cases is/extensions for other use cases are/ Done > ------ > Section 4. > --- > - s/PCE speaker supporting/A PCE speaker supporting/ > - s/PCE speaker needs/A PCE speaker needs/ > - s/needs the means/needs means/ > - s/PCECC based LSP/PCECC-based LSPs/ > - s/PCECC based label allocations/PCECC-based label allocations/ > - s/provide a means/provide a mean/ [x2] > - s/between PCE and PCC/between the PCE and the PCC/ Done > ------ > Sections 5. & 5.1. > --- > - s/PCE as the Central Controller/PCE as a Central Controller/ > - s/reuses existing Active stateful PCE mechanism/reuses the existing active > stateful PCE mechanism/ > - s/control the LSP/control LSPs/ Done > ------ > Section 5.4. > --- > - s/the PCEP speaker MUST send a PCErr/the receiving PCEP speaker MUST > send a PCErr/ > - OLD > [...] 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. > NEW > [...] If a PCEP speaker > which supports 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) MUST be > generated by its peer and the PCEP session will be terminated. > > - s/PCECC-CAPABILITY sub-TLV/the PCECC-CAPABILITY sub-TLV/ [x3] > - s/STATEFUL-PCE-CAPABILITY TLV/the STATEFUL-PCE-CAPABILITY > TLV/ [x2] > - s/in OPEN Object/in the OPEN Object/ > - s/it MUST send/the receiving peer MUST send/ > - s/and I flag/and the I flag/ Done > ------ > Section 5.5. > --- > - s/pertaining to PCECC/pertaining to a PCECC/ > - s/the PCECC LSP is intended/that PCECC LSP is intended/ Done > ------ > Section 5.5.1. > --- > - s/Basic PCECC LSP set up/Basic PCECC LSP Setup/ > - s/included for PCECC LSP/included for PCECC LSPs/ > - s/"so the transit LSR could"/"so a transit/egress LSR could"/ > - s/include SPEAKER-ENTITY-ID TLV/include the SPEAKER-ENTITY-ID TLV/ > - OLD > [...] sets up > the path by sending PCInitiate message to each node along the path of > the LSP. The PCC generates a Path Computation State Report (PCRpt) > and includes the central controller instruction (CCI) and the > identified LSP. The CC-ID uniquely identifies the central controller > instruction within a PCEP session. The PCC further responds with the > PCRpt messages including the CCI and LSP objects. > NEW > [...] sets up > the path by sending a PCInitiate message to each node along the path of > the LSP. The CC-ID uniquely identifies the central controller > instruction within a PCEP session. Each PCC further responds with the > PCRpt messages including the central controller instruction (CCI) and > the LSP objects. > > - s/LSP deletion operation/The LSP deletion operation/ > - s/for PCECC LSP is/for PCECC LSPs is/ > - s/does Label cleanup/does label cleanup/ > - Figure 1: all figures have the same message ordering, though the order only > matters when using PCC-allocated labels (leaving initial and last messages > apart); have you considered shuffling the message order a bit on figures for > PCE-allocated cases? It's true from the point of view of the messages on wire and thus we don't put this in the specification, but internally the PCE would still allocate label from the internal space from Egress towards Ingress as it would need to make sure the in-label and out-label matches and processing would be in the same order. Thus it is better to keep the ordering the same! > - s/and where the PCC/and then the PCC/ > - s/The PCC would further inform/The PCC further informs/ Others, updated. > ------ > Section 5.5.2. > --- > - s/know at both/known by both/ > - s/download as part of CCI,/download, as part of CCI,/ > - s/but failed/but fails/ Done > ------ > Section 5.5.3. > --- > - s/PCE Initiated PCECC LSP/PCE-Initiated PCECC LSP/ [section title + figure > tag] > - s/PCE Initiated LSP/PCE-initiated LSP/ [x2] > - OLD > the removal procedure remains the same. > NEW > the removal procedure remains the same, with just an additional > constraint on the configuration sequence. Done > ------ > Section 5.5.4. > --- > - s/the PCECC first update/the PCECC first updates/ > - s/then update to ingress/then updates the ingress/ > - s/the old instructions/the former instructions/ > - s/the old LSP/the former LSP/ > - s/Delete old CCI/Delete former CCI/ [Figure 5] > - OLD > the removal procedure remains the same. > NEW > the removal procedure remains the same, adding the sequence > constraint. Done > ------ > Section 5.5.5. > --- > - s/the instruction, it wants/the instruction it wants/ [or "the instruction > that it wants"] Updated to later. > ------ > Section 5.5.8. > --- > - s/PCC Based Allocations/PCC-Based Allocations/ > - s/would allocate the label and would report/SHOULD allocate the label and > SHOULD report/ > - s/the Label is 0 and the C flag is set,/the Label is 0 and the C flag is > set to > 1,/ > - s/the Label is 'n' and the C flag is set in the CCI/the Label is 'n' > and the C flag is set to 0 in the CCI/ > - s/the PCC should report/the PCC MUST report/ Others, updated! > ------ > Section 5.5.9. > --- > - OLD > to indicate the allocation needs to be > made by the PCE. This flag is used to indicate that the allocation > needs to be made by the PCE. A PCC would set [...] > NEW > to indicate the allocation needs to be > made by the PCE. A PCC would set [...] > > - s/for a PCE-Initiated (or delegated LSP)./for a PCE-initiated (or > delegated) LSP./ Done > ------ > Section 6. > --- > - s/6. PCEP messages/6. PCEP Messages/ > - s/6.1. The PCInitiate message/6.1. The PCInitiate Message/ > - s/for the transit LSR./for a transit LSR./ > - s/To cleanup,/To clean up entries,/ > - s/instances of CCI object would be included in the case/instances of the CCI > object can be included, in the case/ > - s/6.2. The PCRpt message/6.2. The PCRpt Message/ > - s/the receiving PCC MUST/the receiving PCE MUST/ > - s/for the transit LSR./for a transit LSR./ Done > ------ > Section 7. > --- > - s/The PCEP object defined/The PCEP objects defined/ > - s/a PCECC based mechanism./a PCECC-based mechanism./ > - s/for label download./"for label download/report."/ > - s/Flags: is used/Flags: A field used/ > - s/C-Bit/C bit/ Done > ------ > Section 11. > --- > - s/sub- registry/sub-registry/ > - s/PATH-SETUP- TYPE-CAPABILITY/PATH-SETUP-TYPE-CAPABILITY/ > - In section 11.7, a blank line before each Error-Type would improve > readability. Done > ------ > Section 12. > --- > - The following references may be moved from Normative to Informative: > * RFC 7420, > * RFC 7225, > * RFC 8126, > * RFC 8174, > * RFC 8233. > Moved RFC 7420, RFC 7525. RFC 8126, RFC 8174 are kept as Normative as per recently published RFCs. RFC 8233 is a wrong reference, it should be RFC 8232 (and moved to informational). > > ______________________________________________________________ > ___________________________________________________________ > > 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
