Re: [Pce] Shepherd's Review of draft-ietf-pce-pcep-extension-for-pce-controller-07
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) ;
> 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*
>
>
> >> - 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).
>
>
>
> >> --
> >> Section 12.
> >> ---
> >> - The fo
Re: [Pce] 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-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
>>
>> 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*
>> - 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).
>> --
>> 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
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce
