[Pce] Shepherd Review of draft-ietf-pce-gmpls-pcep-extensions

[Julien Meuric <[e-mail]>]

Hi all,

Please find below my review of draft-ietf-pce-gmpls-pcep-extensions, dug
out of the backlog. Let us discuss the main identified issues first, we
will look at the minor comments and nits afterwards.

Generally speaking, the main items to improve are related to
clarification. Normative behavior and especially error handling often
need more accurate descriptions to limit ambiguities. Sorting error
values may also be deserved, especially with respect to existing error
types.

------
Section 2.2
---
- s/The PCE MAY try to follow/The PCE SHOULD  try to follow
- s/Otherwise, the PCE MAY use/Otherwise, the PCE MUST use/
------
Section 2.3
---
- The bidirectional symmetric bandwidth is defined with 2 MUST's and a
MUST NOT: the error case is not specified if the 3 requirements are not
followed. I guess a sentence pointing to Error-Type 10/Value TBA-14
would address this.
- s/asymmetric bandwidth, it SHOULD/asymmetric bandwidth, it MUST/
------
Section 2.4
---
- The following text is unnecessary and should be dropped:
"A PCC SHOULD be allowed to request a set of TE-LSP also in case of GMPLS
 bandwidth specification.
 The LOAD-BALANCING has the same limitation as the BANDWIDTH for GMPLS
 networks."
- Like above, the bidirectional symmetric bandwidth with LOAD-BALANCING
is defined with 2 MUST's and a MUST NOT: the error case is not specified
if the 3 requirements are not followed. I guess a sentence pointing to a
relevant error (Type 10, new Value?) would address this.
- s/while specifying load balancing constraints, it SHOULD/while
specifying load balancing constraints, it MUST/
- About the last paragraph ("For example [...] corresponding request"):
  * Have you considered moving into appendix?
  * Codepoints are mentioned instead of TBA-2, -4...
------
Section 2.5.1
---
- s/restriction are not always/restriction may not be/

OLD:
   Endpoint type 0 MAY be accepted by
   the PCE, other endpoint type MAY be supported if the PCE
   implementation supports P2MP path calculation.  A PCE not supporting
   a given endpoint type MUST respond with a PCErr with error code "Path
   computation failure", error type "Unsupported endpoint type in END-
   POINTS Generalized Endpoint object type".
NEW:
   A PCE may accept only Endpoint Type 0: Endpoint Types 1-4 apply if the
   PCE implementation supports P2MP path calculation. A PCE not
   supporting a given Endpoint Type SHOULD respond with a PCErr with
   Error Type 4, Value TBD "Unsupported endpoint type in END-POINTS
   Generalized Endpoint object type". As per [RFC5440], a PCE unable to
   process Generalized Endpoints may respond with Error Type 3 or 4,
   Value 2.

OLD:
   A PCE not supporting one of those TLVs in a PCReq MUST respond with a
   PCRep with NO-PATH with the bit "Unknown destination" or "Unknown
   source" in the NO-PATH-VECTOR TLV, the response SHOULD include the
   ENDPOINT object in the response with only the TLV it did not
   understood.
NEW:
   When receiving a PCReq, a PCE unable to resolve the identifier in one of
   those TLVs MUST respond using a PCRep with NO-PATH and set the bit
   "Unknown destination" or "Unknown source" in the NO-PATH-VECTOR TLV.
   The response SHOULD include the ENDPOINT object with only the TLV it
   did not understand.

- s/error type="Path computation failure"/Error Type 4/
------
Section 2.5.2.5.
---
- The I-D has chosen to allocate 2 TLV codepoints for LABEL-SET and
SUGGESTED-LABEL-SET. Any reason not to use just one including a
strict/loose bit to distinguish them?
- s/allocated on the first link/allocated on the first endpoint/
- The text "has the same encoding as the LABEL-SET TLV, it" is redundant
and should be dropped.

OLD:
      This Bit SHOULD be set to 0 in a SUGGESTED-LABEL-SET
      TLV Set and ignored on receipt. This Label MAY be reused. The R
      bit of the RP object MUST be set.
NEW:
      The R bit of the RP object MUST be set to 1. In a SUGGESTED-LABEL-SET
      TLV, this bit SHOULD be set to 0 and ignored on receipt.

- In the 1st paragraph on page 19 ("Several LABEL_SET TLVs [...] be
ignored"), it seems that SHOULD's should be MUST's.
------
Sections 2.6. & 2.7
---
- To be consistence with RFC 7896, both sentences about the L bit should
be dropped.
------
Sections 3 & 5.5
---
- Several errors (e.g., TBD-15, -28, -29...) may be moved to Type 4.
---

Best regards,

Julien