I have also reviewed this draft (A bit late though) and find no major issues with it.
On top of Jon's suggestions, pls find mine below. If these cannot be captured together with WG LC, please consider them during the next process. Regards, Xian == Section 1.2 == s/Switching Encoding/LSP encoding type s/RSVP/RSVP-TE (in multiple places), also suggest to expand it during first use Old: We describe in this document a set of PCEP protocol extensions, including new objects, TLVs, encodings, error codes and procedures, in order to fulfill the aforementioned requirements. NEW: We describe in this document a set of PCEP protocol extensions, including new object types, TLVs, error codes and procedures, in order to fulfill the aforementioned requirements. == Section 1.3 == s/ENDPOINTS/END-POINTS In the list following “The covered PCEP extensions are:”, is there a reason why the GMPLS capability TLV added to OPEN msg is not included? OLD: A new object type is introduced for the LOAD-BALANCING object (Generalized bandwidth), NEW: A new object type is introduced for the LOAD-BALANCING object (Generalized LOAD-BALANCING). == Section 2.1.1 == OLD: Those documents define bit 0 of the PCED TLV for Path computation with GMPLS link constraints. Comment: Since it has been defined already and not clear (as least to me) in current texts, I would suggest to reword as following: NEW: Those documents has defined bit 0 in PCE-CAP-FLAGS Sub-TLV of PCED TLV as “Path computation with GMPLS link constraints”. == Section 2.1.2 == Lots of places with “Section Section XX”, I suggest to do a global replace of “Section Section” with “Section”. The description is "GMPLS capable". Not consistent with IANA section, suggest to the latter to this, replacing “GMPLS Capability”. == Section 2.3. == The title is a bit strange, suggest to change it to “BANDWIDTH object extensions”, similar to other PCEP extensions Section titles. OLD: This correspond to requirement 3,4,5 and 11 of [RFC7025]. Comment: RFC7025 Section 3.1 and 3.2 both have requirement 3, it is better to be more specific. NEW: This corresponds to requirements 3, 4, 5 and 11 of [RFC7025] Section 3.1. OLD: This document defines two OT for the BANDWIDTH object.t: NEW: This document defines two Object Types for the BANDWIDTH object: The Bw Type field determines which type of bandwidth is represented by the object. Comment: The field name in the encoding and in the text is not consistent: Bw Spec Type vs. Bw Type. Please update (in two places) == Section 2.4. == The title is a bit strange, suggest to change it to “LOAD-BALANCING object extensions”, similar to other PCEP extensions Section titles. OLD: …each path using at minimum 2VC4 container,… NEW: …each path using at minimum 2 x VC4 container,… == Section 2.5.1 == OLD: For endpoint type Point-to-Multipoint several endpoint objects may be present in the message and represent a leave… NEW: For endpoint type Point-to-Multipoint, several endpoint objects may be present in the message and each represents a leave,… s/ one of those TLV/one of those TLVs == Section 2.5.2.4 == Are the following two sentences meant the same? If so, suggest to delete the 2nd one. 1: Its format is the same as described in [RFC3471] Section 3.1 Generalized label request. 2: The fields are encoded as in the RSVP-TE. OLD: The Encoding Type indicates the encoding type, e.g., SONET/SDH/GigE etc., that will be used with the data associated. Comment: not clearly explained. Maybe change to the following? NEW: The Encoding type indicates the encoding type, e.g., SONET/SDH/GigE etc., of the LSP with which the data is associated. == Section 2.7 == s/in order to fulfill requirement 13 of [RFC7025] section 4.1,/ in order to fulfill requirement 13 of [RFC7025] section 3.1, == Section 2.8 == OLD: This object is introduced to fulfill requirement 7 of [RFC7025] section 4.1 and requirement 3 of [RFC7025] section 4.2. This object contains the the value of the PROTECTION object defined by [RFC4872] and may be used as a policy input. NEW: This object is introduced to fulfill requirement 7 of [RFC7025] Section 3.1 and requirement 3 of [RFC7025] Section 3.2. This object contains the value of the PROTECTION object defined by [RFC4872] and may be used as a policy input. Comment: contains the value or contains the information? == Section 3 == S/Bad Bandwidth Object type 3 or 4 not supported./ Object type 3 or 4 not supported == Section 4.1 == s/Accepted LOAD-BALANCING object type 3 and 4 parameters in request./ Accepted LOAD-BALANCING object type 2 parameters in request. s/Accepted endpoint type in END-POINTS object type Generalized Endpoint and allowed TLVs/Accepted endpoint type in Generalized Endpoint object type and allowed TLVs == Section 5.1 == As described in Section 2.3, Section 2.4 and Section 2.5.1 new Objects types are defined IANA… Comment: A period is missing before IANA. == Section 5.3 == 13 LSP Protection Information This document (section Section 2.8) Comment: name not consistent, should be: PROTECTION-ATTRIBUTE according to the main text. == Section 5.7 & 5.8 == s/suboject /subobject ===Section 6== Even if there is additional security issue incurred in this draft, is it worthwhile to at least mention some issues already covered by other RFCs or just some pointers? -----Original Message----- From: Pce [mailto:[email protected]] On Behalf Of Jonathan Hardwick Sent: 2014年7月18日 22:22 To: [email protected] Cc: [email protected] Subject: Re: [Pce] WG Last Call for draft-ietf-pce-gmpls-pcep-extensions-09 I've reviewed this document for the WG last call. I think this document is in good shape. I only found nits - see below. Best regards Jon == Section 1.3 == Change A new object type are introduced for the BANDWIDTH object to Two new object types are introduced for the BANDWIDTH object == Section 2.2 == Final paragraph second sentence - I think you should change this to "Otherwise, the PCE MAY use..." to make it clear that the second sentence is not intended to contradict the first sentence. == Section 2.3 == Page 9, directly under Traffic Spec field encoding table - there is a stray comma that should be deleted - change "is MUST specify..." to "it MUST specify..." - change "As specified i [RFC5440]" to "As specified in [RFC5440]" - change "BANDWIDTH object of with object type 1" to "BANDWIDTH object of object type 1" == Section 2.4 == Page 11, directly under Traffic Spec field encoding table - there is a stray full stop (period) that should be deleted - change "is MUST specify..." to "it MUST specify..." == Section 2.5.1 == List of 5 items on page 12. Should the LABEL-REQUEST TLV also be in this list? == Section 2.6 == Change IP address subobject MUST be a link subobject. to If an IP address subobject is used, then the IP address given MUST be associated with a link. Change The procedure associated with this subobject is as follow to The procedure associated with this subobject is as follows. Change MUST allocate one label of from within the set of label values to MUST allocate one label from within the set of label values Change If the PCE does not assign labels a response with a NO-PATH and a NO-PATH-VECTOR-TLV with the bit .'No label resource in range' set. to If the PCE does not assign labels then it sends a response with a NO-PATH object, containing a NO-PATH-VECTOR-TLV with the bit 'No label resource in range' set. == Section 2.7 == Is your intention that the Label Subobject can also be used in the EXRS (RFC 5521 section 2.2?) I think it is worth adding a sentence saying so. For consistency with section 2.6 (and because I think the text in 2.6 is clearer) I think you should change this: XRO Label subobjects MUST follow the numbered or unnumbered interface subobjects to which they refer. Each subobject represent one label, several XRO Labels subobject MAY be present for each link. to this: The Label subobject MUST follow a subobject identifying a link, currently an IP address subobject (Type 1 or 2) or an interface id (type 4) subobject. If an IP address subobject is used, then the IP address given MUST be associated with a link. More than one label suboject MAY follow each link subobject. == Section 5.1 == The formatting used in this section is not consistent. Use consistent indentation & column width. For BANDWIDTH object I think you mean "5-15: Unassigned" For ENDPOINTS the reference should be to 2.5, not 2.3 == Section 5.5 == "Value=q0" should be "Value=10" -----Original Message----- From: Pce [mailto:[email protected]] On Behalf Of Julien Meuric Sent: 04 July 2014 17:05 To: [email protected] Subject: [Pce] WG Last Call for draft-ietf-pce-gmpls-pcep-extensions-09 Dear WG, Now that you all have some time dedicated to I-Ds, please consider this as part of your review list. This message ignites the WG LC on draft-ietf-pce-gmpls-pcep-extensions-09. Comments should be sent to the PCE mailing list by Friday July 18, 11:59 PM, HST. Regards, JP & Julien _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
