Hi Xian. Since your comments are mostly editorial and don't raise any strong issue, authors are expected to take them into account to improve their further revision of the I-D.
Thank you, Julien Jul. 23, 2014 - Zhangxian (Xian): > 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 > _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
