Hi, Thanks a lot for your comments, please see inline
On 23 July 2014 05:17, Zhangxian (Xian) <zhang.x...@huawei.com> wrote: > 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 > Agree > s/RSVP/RSVP-TE (in multiple places), also suggest to expand it during > first use > > Agree > 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. > Agree > > == Section 1.3 == > s/ENDPOINTS/END-POINTS > > Agree > 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? > > There is no technical reason, this will be added. > 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). > > > Agree > == 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”. > > Agree, "have defined" maybe? > == Section 2.1.2 == > Lots of places with “Section Section XX”, I suggest to do a global replace > of “Section Section” with “Section”. > > Agree, for sure > The description is "GMPLS capable". Not consistent with IANA section, > suggest to the latter to this, replacing “GMPLS Capability”. > > Agree > == Section 2.3. == > The title is a bit strange, suggest to change it to “BANDWIDTH object > extensions”, similar to other PCEP extensions Section titles. > > Agree > 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. > > Agree > OLD: > This document defines two OT for the BANDWIDTH object.t: > NEW: > This document defines two Object Types for the BANDWIDTH object: > > > Agree > 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) > > Agree, I will change for Bw Spec Type > == 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. > > Agree > OLD: > …each path using at minimum 2VC4 container,… > NEW: > …each path using at minimum 2 x VC4 container,… > > Agree > == 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,… > > Agree > s/ one of those TLV/one of those TLVs > > Agree > == 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. > > Agree, I will replace by "Its format and encoding is the same as described" > 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. > > Agree > == 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, > > Agree > == 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. > Agree > Comment: contains the value or contains the information? > > I think contains the information would be more generic, so I will put "contains the information", is it OK? > == Section 3 == > S/Bad Bandwidth Object type 3 or 4 not supported./ Object type 3 or 4 not > supported > > Agree > == Section 4.1 == > s/Accepted LOAD-BALANCING object type 3 and 4 parameters in request./ > Accepted LOAD-BALANCING object type 2 parameters in request. > > Agree > 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 > > I would like to keep the END-POINTS object, what about "Accepted endpoint type in object END-POINTS with object type Generalized Endpoint 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. > Agree > == 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. > > Agree == Section 5.7 & 5.8 == > s/suboject /subobject > > Agree > ===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? > > OK, we will enhance this section . Thanks for your review. > > > > > > > > -----Original Message----- > From: Pce [mailto:pce-boun...@ietf.org] On Behalf Of Jonathan Hardwick > Sent: 2014年7月18日 22:22 > To: draft-ietf-pce-gmpls-pcep-extensi...@tools.ietf.org > Cc: pce@ietf.org > 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:pce-boun...@ietf.org] On Behalf Of Julien Meuric > Sent: 04 July 2014 17:05 > To: pce@ietf.org > 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 > Pce@ietf.org > https://www.ietf.org/mailman/listinfo/pce > > _______________________________________________ > Pce mailing list > Pce@ietf.org > https://www.ietf.org/mailman/listinfo/pce > _______________________________________________ > Pce mailing list > Pce@ietf.org > https://www.ietf.org/mailman/listinfo/pce >
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce