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

[Jonathan Hardwick <[e-mail]>]

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]