Hi Julien, Many thanks for your review and comments!
We will update the draft as per your comments, especially with an editorial cleanup of figures and the reference. Best regards, Shuping > -----Original Message----- > From: julien.meu...@orange.com [mailto:julien.meu...@orange.com] > Sent: Wednesday, November 18, 2020 11:13 PM > To: Pengshuping (Peng Shuping) <pengshup...@huawei.com>; > draft-ietf-pce-pcep-extension-for-pce-control...@ietf.org > Cc: pce@ietf.org > Subject: Re: Shepherd's Review of > draft-ietf-pce-pcep-extension-for-pce-controller-07 > > 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-contr > oller/ > > and > https://www.ietf.org/rfcdiff?url1=draft-ietf-pce-pcep-extension-for-pce-con > troller-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 > >> > >> 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. ;-) > _______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce
