Hi Shuping, Thanks for the update. Please see [JM] below.
Cheers, Julien On 14/11/2020 07:17, Pengshuping (Peng Shuping) wrote: > 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: [email protected] [mailto:[email protected]] >> Sent: Wednesday, October 21, 2020 10:51 PM >> >> 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. [JM] Thank you for the changes. I believe both of them do improve the I-D. >> - 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. [JM] Fine, thanks. >> - There is no reference to RFC 8741. Is it applicable? If so, then how? > Added. [JM] OK, fine. >> *Nits* <snip> >> - 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! [JM] This looks implementation specific to me. I don't see anything precluding pushing multiple CCIs in parallel, relying on crankback by updating both ends of a link in case of initial allocation issue. I agree on one thing: shuffling the figure may not be worth the effort. [JM] By the way, some of the arrows on the figures deserve to be polished: * end point alignment, space vs. dash... * spacing after comas are inconsistent (no space seem to be readable). <snip> >> ------ >> 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). [JM] Fine for 8174. RFC 8126 is targeted to document authors, it isn't mandatory reading for readers/implementers. But that's a super-nit I'd be happy to defer to the RFC Editor. ;-)
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
