Re: [Pce] [Gen-art] Genart telechat review of draft-ietf-pce-gmpls-pcep-extensions-14

[Alissa Cooper <[e-mail]>]

Thanks Elwyn. I entered a No Objection ballot and pointed to your review.

Alissa

> On Apr 10, 2019, at 8:47 AM, Elwyn Davies via Datatracker <[e-mail]> wrote:
> 
> 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/
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art