Subject: [Pce] WG Last Call for draft-ietf-pce-gmpls-pcep-extensions-09

From: Julien Meuric <[email-masked]>
Date: Fri, 04 Jul 2014 18:04:37 +0200

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

---

From: Jonathan Hardwick <[email-masked]>
Date: Fri, 18 Jul 2014 14:22:27 +0000

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-masked]] On Behalf Of Julien Meuric
Sent: 04 July 2014 17:05
To: [email-masked]
Subject: [Pce] WG Last Call for draft-ietf-pce-gmpls-pcep-extensions-09

Dear WG,

Regards,

JP & Julien

_______________________________________________
Pce mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/pce

---

From: Julien Meuric <[email-masked]>
Date: Mon, 21 Jul 2014 11:38:06 +0200

Hi all.

This WG last call has ended. The authors are expected to address the 
received comments (thank you John for the feedback). Chairs' review will 
follow.

Thanks,

JP & Julien

Jul. 04, 2014 - Julien Meuric:
> Dear WG,
>
> Now that you all have some time dedicated to I-Ds, please consider 
> this as part of your review list.
>
> 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
>

---

From: "Zhangxian (Xian)" <[email-masked]>
Date: Wed, 23 Jul 2014 09:17:43 +0000

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-masked]] On Behalf Of Jonathan Hardwick
Sent: 2014年7月18日 22:22
To: [email-masked]
Cc: [email-masked]
Subject: Re: [Pce] WG Last Call for draft-ietf-pce-gmpls-pcep-extensions-09

== Section 1.3 ==
Change

== Section 2.2 ==

== Section 2.3 ==

== Section 2.4 ==

== Section 2.5.1 ==

== Section 2.6 ==
Change

Change

Change

Change

== Section 2.7 ==

== Section 5.1 ==

== Section 5.5 ==

-----Original Message-----
From: Pce [mailto:[email-masked]] On Behalf Of Julien Meuric
Sent: 04 July 2014 17:05
To: [email-masked]
Subject: [Pce] WG Last Call for draft-ietf-pce-gmpls-pcep-extensions-09

Dear WG,

Regards,

JP & Julien

---

From: Julien Meuric <[email-masked]>
Date: Wed, 23 Jul 2014 11:32:05 +0200

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

---

From: Cyril Margaria <[email-masked]>
Date: Wed, 23 Jul 2014 08:07:33 -0400

H Jonathan,

Thanks a lot for your review,
please see inline.

On 18 July 2014 10:22, Jonathan Hardwick <[email-masked]>
wrote:

> Agree

> == 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.
>
>
> Agree

> Agree

> Agree

> This is correct, the TLV will be added to the list

> Agree

> Agree

> Agree

> Agree

> == 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.
>
>
This is correct

> Agree

> For ENDPOINTS the reference should be to 2.5, not 2.3
I agree,

> Agree

---

From: Cyril Margaria <[email-masked]>
Date: Wed, 23 Jul 2014 08:48:04 -0400

Hi,

Thanks a lot for your comments, please see inline

On 23 July 2014 05:17, Zhangxian (Xian) <[email-masked]> wrote:

> 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 ==

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

> 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.

> 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?

> 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

Agree

> 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

> == 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

> 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.
> 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

> I think contains the information would be more generic, so I will put
"contains the information", is it OK?

> == 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"

Agree

== Section 5.7 & 5.8 ==

> ===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.

---

From: "Zhangxian (Xian)" <[email-masked]>
Date: Thu, 24 Jul 2014 08:44:05 +0000

Hi, Cyril,

   Thank you for the quick reply. For three items requiring my further response, please see inline, marked with [Xian2]:

Regards,
Xian

From: Cyril Margaria [mailto:[email-masked]]
Sent: 2014年7月23日 20:48
To: Zhangxian (Xian)
Cc: [email-masked]; [email-masked]
Subject: Re: [Pce] WG Last Call for draft-ietf-pce-gmpls-pcep-extensions-09

Hi,

On 23 July 2014 05:17, Zhangxian (Xian) <[email-masked]<mailto:[email-masked]>> wrote:
I have also reviewed this draft (A bit late though) and find no major issues with it.

Regards,
Xian

(snip)

== Section 2.1.1 ==
OLD:
[Xian2]:  you are right, ☺.

(snip)

== Section 2.8 ==
OLD:

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?
[Xian2]: Looks good.

(snip)

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"
[Xian2]:   Sure. But how about "Accepted endpoint type and allowed TLVs in object END-POINTS with object type Generalized Endpoint"? NO strong opinion, take whichever you prefer.

 (snip)