Subject: [Pce] Routing directorate review of draft-ietf-pce-stateful-pce-auto-bandwidth-09

From: Jonathan Hardwick <[email-masked]>
Date: Tue, 18 Jun 2019 09:52:54 +0000

Hi there

I have reviewed this draft for the routing directorate as part of preparing it for IETF last call and IESG review.

I was familiar with this document from the time that I chaired the PCE working group, but this was the first time I read it all the way through and paid attention to all details.  I found it easy to read and understand.  I think it is basically ready to go with a few small clarifications and nits, below.

Cheers
Jon

Document: draft-ietf-pce-stateful-pce-auto-bandwidth-09
Reviewer: Jon Hardwick
Review Date: 18 June 2019
IETF LC End Date: LC not started yet
Intended Status: Standards Track

Comments
Section 3 is somewhat redundant IMO.
4.1 you should ideally provide a reference for how to do MBB signalling.
4.3 "Similarly, if a PCC gets overwhelmed due to signaling churn, it can notify the PCE to temporarily suspend new LSP setup requests."  I think this is covered by 5.7 as well as the PCE case, but you only refer to 5.7 for the latter. Please point to 5.7 for both cases.
5.1 Not a big deal, but I wonder if there is any practical reason to differentiate the final two bullets.
5.6 Why are AUTO-BANDWIDTH-ATTRIBUTES required (MUST) in the LSPA object of a PCRpt?  If the LSP is PCE-initiated, then the PCE already knows what attributes were specified.  If the LSP is PCC-Initiated, then the attributes are the PCC's business - the PCE can't change them (per 5.5) and I don't think the PCE even needs to know what they are.
7.2 Misuses RFC 2119 language to request an action from a working group.  In other documents (when there is not already a draft in progress to do it) we have reworded this as "the YANG / MIB could be updated" etc.

Nits
5: "Extensions to the PCEP" would sound better as "PCEP Extensions"
7: In RFC 6123 it says "The Manageability Considerations section SHOULD be placed immediately before the Security Considerations section in any Internet-Draft." - but here, it comes after.

---

From: Rakesh Gandhi <[email-masked]>
Date: Fri, 21 Jun 2019 13:29:53 -0400

Hi Jon,

Thank you for the review comments. Please see inline with <RG>...

On Tue, Jun 18, 2019 at 5:53 AM Jonathan Hardwick <Jonathan.Hardwick=
[email-masked]> wrote:

> Hi there
>
>
>
> I have reviewed this draft for the routing directorate as part of
> preparing it for IETF last call and IESG review.
>
>
>
> I was familiar with this document from the time that I chaired the PCE
> working group, but this was the first time I read it all the way through
> and paid attention to all details.  I found it easy to read and
> understand.  I think it is basically ready to go with a few small
> clarifications and nits, below.
>
>
>
> Cheers
>
> Jon
>
>
>

<RG> We can keep it given the Figure showing the extensions unless there is
a preference to remove it.

<RG> Added [RFC3209].

4.3 “Similarly, if a PCC gets overwhelmed due to signaling churn, it can
> notify the PCE to temporarily suspend new LSP setup requests.”  I think
> this is covered by 5.7 as well as the PCE case, but you only refer to 5.7
> for the latter. Please point to 5.7 for both cases.
>

<RG> Added.

5.1 Not a big deal, but I wonder if there is any practical reason to
> differentiate the final two bullets.
>

<RG> There is a precedence for the second bullet error message in [RFC
8231] (e.g. error-value 2). The first bullet error message just comes from
the existing behaviour without this extension.

5.6 Why are AUTO-BANDWIDTH-ATTRIBUTES required (MUST) in the LSPA object of
> a PCRpt?  If the LSP is PCE-initiated, then the PCE already knows what
> attributes were specified.  If the LSP is PCC-Initiated, then the
> attributes are the PCC’s business – the PCE can’t change them (per 5.5) and
> I don’t think the PCE even needs to know what they are.
>

<RG> Agree. Removed the sentence.

> 7.2 Misuses RFC 2119 language to request an action from a working group.
> In other documents (when there is not already a draft in progress to do it)
> we have reworded this as “the YANG / MIB could be updated” etc.
>

<RG> Updated the text.

>
> Nits
>
> 5: “Extensions to the PCEP” would sound better as “PCEP Extensions”
>

<RG> Fixed.

7: In RFC 6123 it says “The Manageability Considerations section SHOULD be
> placed immediately before the Security Considerations section in any
> Internet-Draft.” – but here, it comes after.
>

<RG> Updated.

Thanks,
Rakesh

_______________________________________________
> Pce mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/pce
>

---

From: Jonathan Hardwick <[email-masked]>
Date: Sat, 22 Jun 2019 08:49:26 +0000

Thanks Rakesh, sounds good to me.
Jon

From: Rakesh Gandhi <[email-masked]>
Sent: 21 June 2019 18:30
To: Jonathan Hardwick <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [Pce] Routing directorate review of draft-ietf-pce-stateful-pce-auto-bandwidth-09

NOTE: Message is from an external sender
Hi Jon,

On Tue, Jun 18, 2019 at 5:53 AM Jonathan Hardwick <Jonathan.Hardwick=[email-masked]<mailto:[email-masked]>> wrote:
Hi there

Cheers
Jon

Comments

<RG> We can keep it given the Figure showing the extensions unless there is a preference to remove it.

<RG> Added [RFC3209].

4.3 “Similarly, if a PCC gets overwhelmed due to signaling churn, it can notify the PCE to temporarily suspend new LSP setup requests.”  I think this is covered by 5.7 as well as the PCE case, but you only refer to 5.7 for the latter. Please point to 5.7 for both cases.

<RG> Added.

<RG> There is a precedence for the second bullet error message in [RFC 8231] (e.g. error-value 2). The first bullet error message just comes from the existing behaviour without this extension.

5.6 Why are AUTO-BANDWIDTH-ATTRIBUTES required (MUST) in the LSPA object of a PCRpt?  If the LSP is PCE-initiated, then the PCE already knows what attributes were specified.  If the LSP is PCC-Initiated, then the attributes are the PCC’s business – the PCE can’t change them (per 5.5) and I don’t think the PCE even needs to know what they are.

7.2 Misuses RFC 2119 language to request an action from a working group.  In other documents (when there is not already a draft in progress to do it) we have reworded this as “the YANG / MIB could be updated” etc.

<RG> Updated the text.

Nits

<RG> Fixed.

7: In RFC 6123 it says “The Manageability Considerations section SHOULD be placed immediately before the Security Considerations section in any Internet-Draft.” – but here, it comes after.

<RG> Updated.

Thanks,
Rakesh

_______________________________________________
Pce mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/pce