Subject: [Pce] Shepherd's review of draft-ietf-pce-pcep-service-aware-09

From: Jonathan Hardwick <[email-masked]>
Date: Wed, 22 Jun 2016 16:40:07 +0000

Hi there

I have reviewed this document as document shepherd.  Please consider these comments along with any other working group last call comments that you have received.

Best regards
Jon

General
I found a lot of mistakes in the use of English in the text.  These need to be fixed before the document is advanced.  I've agreed with the authors to provide my mark-ups separately.

Section 1 "Introduction"
"Further, there exist mechanisms ... which is inefficient" It's not clear what mechanisms you are referring to, but this is too much of a value judgment.  There's no need to call out other mechanisms as being unsuitable.  Just delete this sentence.

You ought to define, early on in this document, that the phrase "network performance constraints" means any combination of latency, latency variation, packet loss and link bandwidth utilization constraints.  You can then use the term "network performance constraints" without ambiguity.

Section 3 "PCEP Requirements"
I couldn't quite parse this sentence:

   2.  A PCC MUST be able to request for E2E network performance
       constraint(s) in PCReq message as the key constraint to be
       optimized or to suggest boundary condition that should not be
       crossed.

How about: "A PCC must be able to specify any network performance constraint as a constraint in a PCReq message, and must be able to indicate whether it is providing a metric that must be optimised on the computed path or is providing a bound constraint that must not be exceeded by the computed path."

How is (3) in this list different from (2)?

Items (6) and (7) should be reworded to talk about what PCEs and PCCs should be able to do, to be consistent with (1) to (5).  "A PCE MUST be able to..."

Delete "It is assumed that".

Section 4 "PCEP Extensions"
4.1 "This document defines the following optional types for the METRIC object."  You should then list the types, along with the corresponding values of the T bit, and say that they are defined in the following subsections.  Also, I don't think you need the word "optional".

4.1 Delete "For explanation of these metrics,"

4.1.1 You variously call it "path delay metric" and "P2P latency metric". Please be consistent.

4.1.3 There is a missing close-parenthesis in the formula for the overall loss fraction on the path.

4.1.4 You also need to give the Error-Value codes for the PCEP-ERROR message in both cases.

4.1.5 This text:
   In a PCReq message, a PCC MAY insert more than one METRIC object to
   be optimized, in such a case PCE SHOULD find the path that is optimal
   when both the metrics are considered together.

How is the PCE to consider both metrics together, in detail?  Doesn't that require a new objective function?  You haven't defined one.

4.2.2 In this section, it would add clarity if you also gave an explicit fomula for LRBU, i.e. "LRBU = LBU - (Residual BW - Available BW)"

4.2.3 Why is a new object needed for unreserved bandwidth?  Isn't this just another type of METRIC?  E.g. "optimize unreserved bandwidth (using the given objective function)" or "use this value as the lower bound of the unreserved bandwidth" depending on the setting of the B bit.

4.2.3.1 You also need to give the Error-Value codes for the PCEP-ERROR message in both cases.

4.2.3.1 You should spell out what the PCE is supposed to do with this object - currently you just say "factor in".  How about "A PCE that supports this object MUST ensure that no link on the computed path has a LBU percentage exceeding the given value."

4.3 Should "Maximum Reserved bandwidth" be "Maximum Reservable bandwidth"?

Section 5 "Stateful PCE"
Section 5 "The passive stateful PCE implementation MAY use..." - delete "passive" since active stateful PCEs also use these messages.

Section 5.1 - this subsection should be moved into section 6 as it describes an expension to a PCEP message, like the other subsections in section 6.  I think all that needs saying in place of section 5.1 is that "The PCRpt message is extended to support BU object (see 6.XXX).  The BU object on a PCRpt specifies the upper limit set at the PCC at the time of LSP delegation to an active stateful PCE."

Section 7 "Other Considerations"
Section 7.2 - I think this section should be expanded to explain the process.

"PCC can monitor the setup LSPs" - how does it monitor them?

"In case of drastic change" - the word "drastic" is open to interpretation.  Instead "If the bandwidth utilization percentage of any of the links in the path changes to a value less than that required when the path was set up, or otherwise less than an implementation-specific threshold, then..."

What about a stateful PCE proactively reoptimizing paths?  This should also be discussed.

Section 7.3 - why are these not defined in section 4.1?  I think that splitting the definitions of new METRIC types across two sections of the document is unclear.

Section 7.3.3 typo "(T) = = TBD10" should be "(T) = TBD10"

References
[ISIS-TE-METRIC-EXT] is now [RFC7810]
[TE-EXPRESS-PATH] is now [RFC7823]

---

From: Dhruv Dhody <[email-masked]>
Date: Thu, 23 Jun 2016 07:01:16 +0000

Hi Jon,

Thanks for being the shepherd and providing valuable comments.
I have attached the working copy of the document with diff.
See inline..

From: Pce [mailto:[email-masked]] On Behalf Of Jonathan Hardwick
Sent: 22 June 2016 22:10
To: [email-masked]
Cc: [email-masked]
Subject: [Pce] Shepherd's review of draft-ietf-pce-pcep-service-aware-09

Hi there

Best regards
Jon

General
I found a lot of mistakes in the use of English in the text.  These need to be fixed before the document is advanced.  I've agreed with the authors to provide my mark-ups separately.
[Dhruv] Thanks for fixing this, will incorporate those once we agree on these comments.

Section 1 "Introduction"
[Dhruv] OK

[Dhruv] Added

How about: "A PCC must be able to specify any network performance constraint as a constraint in a PCReq message, and must be able to indicate whether it is providing a metric that must be optimised on the computed path or is providing a bound constraint that must not be exceeded by the computed path."
[Dhruv] thanks, much better wording; But see below comment -

How is (3) in this list different from (2)?
[Dhruv] We were trying to differentiate between constraints that are applied to E2E path (cumulative) - (2) and one that are applied to each link on the path - (3)

I think we can remove all these details from this section and simply state -

(2) A PCC must be able to specify any network performance constraint
    in a PCReq message to be applied during the path computation.

(3) A PCC must be able to specify any network performance information
    as an optimization criteria.

Remove (4)

What do you think?

[Dhruv] Ok

[Dhruv] Ok

[Dhruv] Done

[Dhruv] Done

4.1.1 You variously call it "path delay metric" and "P2P latency metric". Please be consistent.
[Dhruv] Done, fixed for other metric too

[Dhruv] fixed

[Dhruv] fixed

4.1.5 This text:

How is the PCE to consider both metrics together, in detail?  Doesn't that require a new objective function?  You haven't defined one.
[Dhruv] You are right, I should remove this and keep the behaviour as per RFC5440 which will requires OF to be defined if there is a need to consider multiple-parameter optimization.

[Dhruv] fixed

4.2.3 Why is a new object needed for unreserved bandwidth?  Isn't this just another type of METRIC?  E.g. "optimize unreserved bandwidth (using the given objective function)" or "use this value as the lower bound of the unreserved bandwidth" depending on the setting of the B bit.
[Dhruv] METRIC in its current usage and description fits to the cost associated with the E2E path (cumulative cost), whereas with Bandwidth Utilization (similar to BANDWIDTH object) is applied to each link on the path. Based on this we concluded to not use METRIC object.

[Dhruv] fixed

[Dhruv] fixed

[Dhruv] yes, fixed

Section 5 "Stateful PCE"
[Dhruv] fixed

Section 5.1 - this subsection should be moved into section 6 as it describes an expension to a PCEP message, like the other subsections in section 6.  I think all that needs saying in place of section 5.1 is that "The PCRpt message is extended to support BU object (see 6.XXX).  The BU object on a PCRpt specifies the upper limit set at the PCC at the time of LSP delegation to an active stateful PCE."
[Dhruv] fixed and rearranged

What about a stateful PCE proactively reoptimizing paths?  This should also be discussed.
[Dhruv] changed section to -
7.2.  Reoptimization Consideration

   [RFC6374] supports measurement of loss, delay, and related metrics
   over Label Switched Paths (LSPs).  PCC can utilize such measurement
   technique and in case of degradation of network performance
   parameters below the constraint when the path was setup or an
   implementation-specific threshold, it MAY ask PCE for reoptimization
   with R bit set in RP object as per [RFC5440].

   Based on the changes in the network performance parameters of the
   link in TED (as populated by IGP), PCC can identify the impacted LSP
   and MAY also issue a reoptimization request.  For example, PCC can
   monitor the link bandwidth utilization along the path by monitoring
   changes in the bandwidth utilization parameters of one or more links
   on the path in the TED.  If the bandwidth utilization percentage of
   any of the links in the path changes to a value less than that
   required when the path was set up, or otherwise less than an
   implementation-specific threshold, then issue an reoptimization
   request to a PCE.

   A stateful PCE can also determine which LSP should be re-optimized
   based on network events or trigger from external monitoring systems.
   For example, when a particular link deteriorates and its loss
   increases, this can trigger the stateful PCE to automatically
   determine which LSP are impacted and should be reoptimized.

Section 7.3 - why are these not defined in section 4.1?  I think that splitting the definitions of new METRIC types across two sections of the document is unclear.
[Dhruv] rearranged the document

[Dhruv] fixed

References
[Dhruv] Done!

Thanks!

Regards,
Dhruv

---

From: Jonathan Hardwick <[email-masked]>
Date: Thu, 23 Jun 2016 17:01:25 +0000

Hi Dhruv

Thanks for responding so quickly.  I have inserted my replies below.

As promised, I have also attached my "language" mark-ups, where I decided to directly edit the XML file that you sent to me.  Please check that you are happy with these.  Most are simple things (inserting definite and indefinite articles was a very common one!) but in some cases I felt that a rewording for clarity was necessary.  All of my direct changes to the XML are editorial in nature i.e. do not affect the protocol.

Best regards
Jon

From: Dhruv Dhody [mailto:[email-masked]]
Sent: 23 June 2016 08:01
To: Jonathan Hardwick <[email-masked]>; [email-masked]
Cc: [email-masked]
Subject: RE: Shepherd's review of draft-ietf-pce-pcep-service-aware-09

Hi Jon,

From: Pce [mailto:[email-masked]] On Behalf Of Jonathan Hardwick
Sent: 22 June 2016 22:10
To: [email-masked]<mailto:[email-masked]>
Cc: [email-masked]<mailto:[email-masked]>
Subject: [Pce] Shepherd's review of draft-ietf-pce-pcep-service-aware-09

<snip>

Remove (4)

What do you think?
[JEH] I think fine.  Although rather than "network performance information" could we have "network performance metrics"?

<snip>

4.1.4 You also need to give the Error-Value codes for the PCEP-ERROR message in both cases.
[Dhruv] fixed
[JEH] Thanks, but I have another concern: is "error-type 5 (policy violation)" appropriate for "not capable of service aware path computation"?  I would have thought error-type 4 (not supported object) would fit better.  I think "policy violation" is for cases where a service has been denied to a client for reasons of operator policy, not because of an implementation limitation.
[JEH] The same comment applies to 4.2.3.1.

<snip>

4.2.3 Why is a new object needed for unreserved bandwidth?  Isn't this just another type of METRIC?  E.g. "optimize unreserved bandwidth (using the given objective function)" or "use this value as the lower bound of the unreserved bandwidth" depending on the setting of the B bit.
[Dhruv] METRIC in its current usage and description fits to the cost associated with the E2E path (cumulative cost), whereas with Bandwidth Utilization (similar to BANDWIDTH object) is applied to each link on the path. Based on this we concluded to not use METRIC object.
[JEH] OK, I understand this now.
[JEH] New comment (sorry).  In this section you have some inconsistent capitalization of terms (Residual bandwidth, / Residual Bandwidth / Available bandwidth / Available Bandwidth / Maximum reservable bandwidth).  I suppose this is true in the rest of the document too (but I haven't checked).  Please could you standardize on a convention?

<snip>

[JEH] New comment on section 4.3.  It says:

"The new metric types specified
   in this document MAY continue to use the existing objective functions
   like Minimum Cost Path (MCP).  Path Delay and Path Delay Variation
   are well suited to use MCP as an optimization criteria.  For Path
   Loss following new OF is defined"

I can't reconcile the first sentence with the other two.  The first sentence implies that all current objective functions (including MCP) apply to all metric types in your draft.  The third sentence says that MCP is no good for the Path Loss metric and a new OF is needed instead.  I think you want to say that the MCP OF is applicable to the Path Delay and Path Delay Variation metrics, and that a new OF is needed for the Path Loss metric.  Is that right?

<snip>

[JEH] I think this is good to say, although I have suggested some minor rewording for clarity in the attachment.
[JEH] I think you need to define the term TED in your glossary.

<snip>

[JEH] I think the title of section 8.5 should be "New Error-Values".

---

From: Dhruv Dhody <[email-masked]>
Date: Fri, 24 Jun 2016 10:24:45 +0000

Hi Jon,

Thank you fixing the grammatical issues with the document.
One change I would like to make is to the term "Fractional Percentage Link Loss" to just  "Fractional Link Loss".

I have attached a diff of the recent changes only.

See inline...

From: Jonathan Hardwick [mailto:[email-masked]]
Sent: 23 June 2016 22:31
To: Dhruv Dhody <[email-masked]>; [email-masked]
Cc: [email-masked]
Subject: RE: Shepherd's review of draft-ietf-pce-pcep-service-aware-09

Hi Dhruv

Best regards
Jon

From: Dhruv Dhody [mailto:[email-masked]]
Sent: 23 June 2016 08:01
To: Jonathan Hardwick <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>
Cc: [email-masked]<mailto:[email-masked]>
Subject: RE: Shepherd's review of draft-ietf-pce-pcep-service-aware-09

Hi Jon,

<snip>

Remove (4)

What do you think?
[JEH] I think fine.  Although rather than "network performance information" could we have "network performance metrics"?
[Dhruv2] I am worried that might be seen as excluding the bandwidth utilization related optimization criteria. I wanted to use a generic term and thus  "network performance information".

<snip>

4.1.4 You also need to give the Error-Value codes for the PCEP-ERROR message in both cases.
[Dhruv] fixed
[JEH] Thanks, but I have another concern: is "error-type 5 (policy violation)" appropriate for "not capable of service aware path computation"?  I would have thought error-type 4 (not supported object) would fit better.  I think "policy violation" is for cases where a service has been denied to a client for reasons of operator policy, not because of an implementation limitation.
[JEH] The same comment applies to 4.2.3.1.
[Dhruv2] You are right, and I think we should have error for both the cases. I have done that now ->

   If the PCE understands but does not support the new METRIC type, and
   the P bit is set in the METRIC object header, then the PCE MUST send
   a PCErr message containing a PCEP-ERROR Object with Error-Type = 4
   (Not supported object) with Error-value = TBD11 (Unsupported network
   performance constraint).  The path computation request MUST then be
   cancelled.

   If the PCE understands the new METRIC type, but the local policy has
   been configured on the PCE to not allow network performance
   constraint, and the P bit is set in the METRIC object header, then
   the PCE MUST send a PCErr message containing a PCEP-ERROR Object with
   Error-Type = 5 (Policy violation) with Error-value = TBD12 (Not
   allowed network performance constraint).  The path computation
   request MUST then be cancelled.

<snip>

[Dhruv2]  Done!
<snip>

I can't reconcile the first sentence with the other two.  The first sentence implies that all current objective functions (including MCP) apply to all metric types in your draft.  The third sentence says that MCP is no good for the Path Loss metric and a new OF is needed instead.  I think you want to say that the MCP OF is applicable to the Path Delay and Path Delay Variation metrics, and that a new OF is needed for the Path Loss metric.  Is that right?
[Dhruv2] Yes, and I have reworded this.

The new metric types for
path delay and path delay variation can continue to use the existing
objective function - Minimum Cost Path (MCP) [RFC5541].  For path
  loss, the following new OF is defined.

<snip>
<snip>

[Dhruv2] Done.

<snip>

[Dhruv2] Done.

Thanks again!!

Regards,
Dhruv

---

From: Jonathan Hardwick <[email-masked]>
Date: Fri, 24 Jun 2016 11:19:39 +0000

Thanks Dhruv,

A couple more typos I noticed:
Section 3: "a consistent ways of" should be "... way of"
Section 4.1.1.1: "the encoding for path delay metric value" should be "... the path delay metric value"

See also [JEH2] inline below.

Cheers
Jon

Sent: 24 June 2016 11:25

Hi Jon,

Thank you fixing the grammatical issues with the document.
One change I would like to make is to the term "Fractional Percentage Link Loss" to just  "Fractional Link Loss".
[JEH2] OK. But in section 4.3 you actually changed it to "percentage link loss".  Also, in 4.1.3 and 4.3, I think the abbreviation should be FL(L) not FPL(L)?  I read the P in FPL as "Percentage".

<snip>

What do you think?
[JEH] I think fine.  Although rather than "network performance information" could we have "network performance metrics"?
[Dhruv2] I am worried that might be seen as excluding the bandwidth utilization related optimization criteria. I wanted to use a generic term and thus  "network performance information".
[JEH2] I'm only concerned that the existing sentence does not scan well.  How about "3. A PCC must be able to request that a PCE optimizes a path using any network performance criteria."

<snip>

---

From: Dhruv Dhody <[email-masked]>
Date: Fri, 24 Jun 2016 12:07:37 +0000

Hi Jon,

I have done the latest set of changes, should I upload this version?
See also inline...

Thanks!
Dhruv

Sent: 24 June 2016 16:50

Thanks Dhruv,

[Dhruv3] : Fixed.

Cheers
Jon

Hi Jon,

Thank you fixing the grammatical issues with the document.
One change I would like to make is to the term "Fractional Percentage Link Loss" to just  "Fractional Link Loss".
[JEH2] OK. But in section 4.3 you actually changed it to "percentage link loss".  Also, in 4.1.3 and 4.3, I think the abbreviation should be FL(L) not FPL(L)?  I read the P in FPL as "Percentage".
[Dhruv3] : When I wrote it first PL was packet loss and FPL was fractional packet loss. I removed packet from packet loss, as was done in ISIS document.
I can see how this could be confusing and I have used your recommendation.

<snip>

What do you think?
[Dhruv3] : OK

<snip>

---

From: Jonathan Hardwick <[email-masked]>
Date: Fri, 24 Jun 2016 13:14:26 +0000

Hi Dhruv

This version looks good to me.

Thanks
Jon

Sent: 24 June 2016 13:08

Hi Jon,

Thanks!
Dhruv

From: Jonathan Hardwick [mailto:[email-masked]]
Sent: 24 June 2016 16:50
To: Dhruv Dhody <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>
Cc: [email-masked]<mailto:[email-masked]>
Subject: RE: Shepherd's review of draft-ietf-pce-pcep-service-aware-09

Thanks Dhruv,

Cheers
Jon

Hi Jon,

<snip>

What do you think?

<snip>