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

Reply via email to