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=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 sp
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=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
Re: [Pce] Shepherd's Review of draft-ietf-pce-pcep-extension-for-pce-controller-07
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=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 ca
[Pce] 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.
- 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.
- There is no reference to RFC 8741. Is it applicable? If so, then how?
*Nits*
--
Global
---
- Consistency on capitalization of ingress/transit/egress to be corrected.
--
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/
--
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/
--
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/
--
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/
--
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/
--
Section 5.5.
---
- s/pertaining to PCECC/pertaining to a PCECC/
- s/the PCECC LSP is intended/that PCECC LSP is intended/
--
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
