[Pce] Genart telechat review of draft-ietf-pce-gmpls-pcep-extensions-14

[Elwyn Davies via Datatracker <[e-mail]>]

Reviewer: Elwyn Davies
Review result: Almost Ready

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-pce-gmpls-pcep-extensions-14
Reviewer: Elwyn Davies
Review Date: 2019-04-10
IETF LC End Date: 2018-10-29
IESG Telechat date: 2019-04-11

Summary:
Almost ready, but with a large collection of nits in language and non-expansion
of abbreviations. I am also concerned about the specification of behaviour in
case PCC/PCEs with and without the extensions attempt to interact.  The
requirements and behaviour are rather woolly, and are not fully covered by
capability negotiations as the negotation capability itself is not in the
original PCEP specification.

Major issues:
None

Minor issues:
Interacting with PCCs that do not support these GMPLS extensions: The draft is
not very clear on interactions between PCCs that do support the extensions and
ones that do not.  It is unclear whether a PCC that proposes to use the
extensions must support the RFC 5088 or 5089 capability negotiation extensions
and use them to determine if a PCEP exchange can use the extensions.  The text
in para 1 of s2.1.2 appears to require that a node that does not support RFC
5088 or 5089 still has to understand that it has received the GMPLS-CAPABILITY
type indicator and indicate a mismatch.  It seems to me that some additional
explanation is needed to describe how mismatched PCC/PCEs understand the
problem and deal with cases where a message with the new extensions is received
(and presumably rejected) by a node that does not implement the extensions.

s9.2, RFC7025: Given the references to the requirements document for this work,
I would consider RFC 7025 to be normative.

Nits/editorial comments:
General: s/e.g. /e.g., /g

Abstract: s/The Path Computation Element (PCE)/A Path Computation Element (PCE)/

s1: Expand abbreviations OTN (Optical Transport Networks) and WSON (Wavelength
Switched Optical Networks).

s1, para 2: s/considered/addressed/, s/those application/these applications/

s1.2, para 1: s/PCEP extension/PCEP extensions/, s/broken down in/broken down
into/

s1.2: Expand following acronyms/abbreviations on first occurrence: LSP, TE-LSP,
L2SC, TDM, SONET, SDH, LSC [Query: Is LSC different from L2SC?], PCC, ERO

s1.2, bullet 2: A reference for the G.709 standard is needed.

s1.2 and s1.3.1, items (4) and (5): There doesn't seem to be a definition of
Concatenation Number in any of the documents mentioned here or anywhere on the
web.  I suspect it is supposed to be the number of streams that are
concatenated but this needs to be properly defined or a reference provided.

s1.2, bullet 5:  s/Label restriction/label restriction/.  I take it this refers
to the use of Label Set objects as described in RFC 3473.  If so please add a
reference.  If not lease provide the appropriate reference.

s1.3.1: Expand following acronyms/abbreviations on first occurrence: TE-LSP,
ODU, IRO, XRO, RRO, LSPA

s1.3.1, item (4): s/Its scoped/It is scoped/ [English language note: 'Its' is
the possessive pronoun derived from the third person singular impersonal
pronoun 'it', whereas "It's" is a contraction of 'it is' that is not normally
used in formal documents.]

s1.3.1, item (4):

> related to the BANDWIDTH object in MPLS networks
I assume this relates to the BANDWIDTH object in RFC 5440 - please add a
reference.

s1.3.2, item (1):  The previous two comments on s.1.3.1, item (4) apply also to
this item.

s1.4:

OLD:

 1.4   GMPLS Support and Limitation of Base PCEP Objects

   The support for requirements [RFC7025] is summarized in Table 1 and
   Table 2

NEW:

1.4   Existng Support for GMPLS in Base PCEP Objects and its Limitations

   The support provided by specifications in [RFC8282] and [RFC5440]  for the
   requirements listed in [RFC7025] is summarized in Table 1 and Table 2.  In
   some cases the support may not be complete, as noted, and additional support
   need to be provided in this specification.

ENDS

s1.4, RFC 5440 bullets, ERO:  A reference for the RSVP specification covering
ERO is needed.

s1.4, XRO object. 1st bullet: Expand SRLG.

s1.4, SWITCH-LAYER bullet: s/address/addresses/

s1.4, list of coverage gaps: Expand NVC.

s1.4, added functionality needed: s/to cover the gap/to cover the gaps/

s2: Expand PCReq and PCRep message names to reflect names in RFC 5440.

s2.1.2:

> Moreover, in case that the PCC does not receive the
>    GMPLS-CAPABILITY TLV it is RECOMMENDED that the PCC does not make use
>    of the objects and TLVs defined in this document.
I would have thought this ought to have been a MUST (when communicating with
the PCC that didn't support the extensions.

s2.1.2: s/OPEN message/Open message/ in (at least) 3 places - for consistency
and to match RFC 5440

s2.1.2, GMPS-CAPABILITY TLV spec

>    IANA has allocated value TBA-1 from the "PCEP TLV Type Indicators"
>    sub-registry, as documented in Section 5.3 ("New PCEP TLVs").  The
>    description is "GMPLS-CAPABILITY".  Its format is shown in the
>    following figure.
>
>      0                   1                   2                   3
>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>     |               Type=14         |           Length              |
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>     |                             Flags                             |
>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Should 'Type=14' in the diagram be 'Type=TBA-1'?

s2.2:

> Depending
>    on policies or switching layer, it can be necessary for the PCC to
>    use explicit label control or expect explicit link,
                                                                ^^^^^^^^^^^^^^^^^^^^^^^

I cannot parse the marked phrase 'or expect explicit link'.  Please clarify.

s2.2, next to last para: s/requested route granularity/requested routing
granularity/

s2.3, para 1: s/to allow to express/to allow the object to express/, s/set of
encoding/set of encodings/

s2.3, para at top of page 12: s/The Bw Spec Type correspond to/The Bw Spec Type
corresponds to/

s2.3, Table 4:Expand MEF, SSON on first occurrence.

s2.3, next to last para:

OLD:

  The Object Type TBA-3 MAY be used instead of object type 2
   to indicate the existing TE-LSP bandwidth.  A PCC that requested a
   path with a BANDWIDTH object of object type 1 SHOULD use object type
   2 to represent the existing TE-LSP BANDWIDTH.

NEW:

   The Object Type TBA-3 MAY be used instead of the previously specified object
   type 2 to indicate the existing TE-LSP bandwidth originally specified with
   object type TBA-2. A PCC that requested a path with a BANDWIDTH object of
   object type 1 SHOULD use object type 2 to represent the existing TE-LSP
   BANDWIDTH.

ENDS

It is not clear to me why this is a SHOULD and not a MUST.

s2.4, para below the format: s/MAY also include TLV different from the PCEP
TLVs./MAY also include TLVs different from the basic PCEP TLVs./

s2.4, para at top of page 14:

>  The encoding of the fields Min Bandwidth Spec and Min Reverse
>    Bandwidth Spec is the same as in RSVP-TE SENDER_TSPEC object, it can
>    be found in Table 4 from Section 2.3.
Might be helpful to add (after Section 2.3) 'of this document' - the use of
RSVP-TE immediately before this got me thinking this is in some RSVP-TE
document.

s2.5, last bullet on page 14: s/constraints in/constraints on/

s2.5.1, para 7 (one after bullet #5):  s/This TLVs express/These TLVs express/
(probably, but could be 'This TLV expresses' as I think this applies to just
the Label set TLV.)

s2.5.1, para 7: s/label set allows to indicate which label/label set allows
indication of which labels/

s2.5.1, para 7: s/restricting then in GMPLS the possible label ranges on the
interface/consequently restricting  the possible label ranges on the interface
in GMPLS/

s2.5.1: Expand P2MP on first occurrence.

s2.5.1, para after Table 5: s/Value TBD/Value TBA-14/

s2.5.1, page 17, para after generalized-endpoint-tlvs grammar: Suggest

OLD:

   For endpoint type Point-to-Multipoint, several endpoint objects MAY
   be present in the message and each represents a leave, exact meaning
   depend on the endpoint type defined of the object.

NEW:

   For endpoint type Point-to-Multipoint, several endpoint objects MAY
   be present in the message and each represents a leaf of the P2MP tree,
   with the exact meaning depending on the endpoint type defined for the object.
ENDS

s2.5.1, last para on page 17: For consistency, the (new) vlaue of the Type 4
error should be specified here (TBA-15, i think).

s2.5.2.x, last para in each case: s/responded/returned/

s2.5.2.3, para 1 s/as follow/as follows/

s2.5.2.4, paa 1: s/The LABEL-REQUEST TLV use/The LABEL-REQUEST TLV uses/

s2.5.2.4, para 1: Suggest using fully expanded name of G-PID - Generalized
Payload Identifier.

s2.5.2.4, para 2: Suggest adding reference(s) for where the definitions of
Tspec and FlowSpec can be found.

s2.5.2.5, para 1: s/Those TLV/These TLVs/, s/responded/returned/, s/as
follow/as follows/

s2.5.2.5, first bullet: s/set of label/set of labels/

s2.5.2.5, para after diagram: s/Those parameters/These parameters/

s2.5.2.5:  There are two more or less equivalent descriptions of the functions
of the U, O and L flag bits.  For avadance of error, it would be better if the
first summary was removed and the reader referred to the full description after
the diagram.  This would mean adding the text 'following the semantic of
SUGGESTED_LABEL defined by  [RFC3471]'  to the full description.

s2.5.2.5, last para on page 20:

> At most 2 LABEL_SET TLVs MUST be present with
>    the O bit set, at most one with the U bit set and at most one with
>    the U bit cleared.
Since all LABEL_SET TLVs contain a U bit field, I read the above text as
limiting the number of LABEL_SETs to two (it isn't clear that the U constraint
is a sub-constraint after the O constraint).  I think what is meant is

> At most 2 LABEL_SET TLVs MAY be present with
>    the O bit set, with at most one of these having the U bit set and
>    at most one of these having the U bit cleared.
s2.5.2.5, last 3 paras: For consistency the new error values TBA-x should be
specified.

s2.6, para 1: s/allows to include label definition/allows the inclusion of a
label definition/

s2.7, para 1: s/allows to exclude labels/allows the exclusion of certan labels/

s2.7: More information about U, C-Type and Label is allegedly found in RFC
3471.  I can't find a reference to C-Type in that document, and it is unclear
which part of the document has the Label info.  Is this a miatake?

s2.8, para 1: s/fulfill/fulfil/

s2.8: Pointers to the relevant sections in RFC 4872 and 4873 would help.

s2.9: s/The NO-PATH object MAY carries/The NO-PATH object MAY carry/,
s/like/such as/, s/Few/Several/, s/and no/but no/

s3, para 1: s/few error/several error/, s/Error- values/Error-values/

s3, para 2: s/but not path found/but the PCe is unable to find a path/,
s/NO-PATH is to be used/the NO-PATH error is to be used/

s4.1, last para: s/Those parameters configuration are/The configuration of the
above parameters is/

s4.2: s/This document does not introduces new ERO sub object,/This document
does not introduce any new ERO sub objects, so that the/

s4.4: s/should be covered by/should satisfy/

s5:  It would be worth adding a note to IANA that TBA-11 is not used (assuming
this is not a bug).

s5.1 and s5.3: s/(section /(/ (removing duplicate word 'section') in several
places

s5.2, para 2: s/qualities/attributes/

s6, para 1: s/amount/volume/

s6, first bullet: s/The answer can make that the LSP traverses/The resulting
LSP might be arranged to traverse/

s6, first bullet: s/the answer can omit/the rsulting LSP can omit/, s/the
answer can lead to provide a LSP/the result can lead to the creation of an LSP/