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 <noreply@xxxxxxxx> 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 > Gen-art@xxxxxxxx > https://www.ietf.org/mailman/listinfo/gen-art
