Re: [Pce] Shepherd Review of draft-ietf-pce-gmpls-pcep-extensions

[<[e-mail]>]

Hi Cyril,

[CC'ing the WG to share the comment resolution below]

Thanks a lot for editing this update. Please find here some
complementary fixes and nits.

------
Abstract
---
- s/extensions for the/extensions to the/
------
Section  1.
---
- s/current PCEP RFCs/most preexisting PCEP RFCs/
- s/PCEP requirements for/PCEP Requirements for/
- s/support and limitation of existing PCEP objects/Support and
Limitation of Base PCEP Objects/
- s/to any GMPLS networks/to all GMPLS-controlled networks/
- s/ERO : Unnumbered endpoints/ERO: Unnumbered IDs/
- No space before ':' (multiple times)
- s/existing route/existing path/
- s/existing PCEP object/base PCEP object/
- "IRO/XRO" should be expanded at first occurrence: "The Include/eXclude
Route Objects (IRO/XRO) do not [...]"
- s/of labels/of labels./
- s/Current attributes/Base attributes/
------
Section 2.
---
- s/PCEP objects and extensions/PCEP Objects and Extensions/
- s/GMPLS capability advertisement/GMPLS Capability Advertisement/
- s/OPEN Object extension/OPEN Object Extension/
- s/Moreover ,/Moreover,/
- s/RP object extension/RP Object Extension/
- s/This correspond to/This corresponds to/
- s/SHOULD try to follow/SHOULD follow/
- s/granularity it likes on the route/granularity on the path/ 
[dropping "it likes"]
- s/BANDWIDTH object extensions/BANDWIDTH Object Extensions/
- s/the request size/the requested size/
- s/BANDIDTH/BANDWIDTH/
- s/are to be supported by the new encoding/are supported by the
extended encoding/
- s/TE LSP/TE-LSP/
- s/The payload/The body/
- Put consistency in capitalization of field names, both in the figure
and the following text.
- s/16 bit Bandwidth/16-bit Bandwidth/
- s/RSVPT-TE/RSVP-TE/
- s/the field generalized bandwidth and reverse generalized
bandwidth/the fields Generalized Bandwidth and Reverse Generalized
Bandwidth/
- Below the object type summary, prefix the title with a Table number
(will ease further references).
- s/symetric/symmetric/
- s/procedures described/procedure described/
- s/is unchanged,/is unchanged:/
- s/of the path/of the path./
- s/TE LSP/TE-LSP/  [x2]
- s/LOAD-BALANCING object extensions/LOAD-BALANCING Object Extensions/
- s/BANDWIDTH object a new object/BANDWIDTH object, a new object/
- s/generalized load balancing object/Generalized Load Balancing
object/  [x2]
- Put consistency in capitalization of field names, both in the figure
and the following text.
- s/include TLV different than/include TLVs different from/
- s/(8 bits) :/(8 bits):/
- s/it correspond to/it corresponds to/
- s/RSVPT-TE/RSVP-TE/
- s/C-Types/C-Types./
- s/TE LSPs in the set/TE-LSPs in the set/
- s/Specifies/specifies/  [x2]
- s/the set of TE LSPs./the TE-LSP set./  [x2]
- s/the field Min Bandwidth Spec and Min Reverse Bandwidth spec/the
fields Min Bandwidth Spec and Min Reverse Bandwidth Spec/
- s/in the following references./in table X from section 2.3./
- Drop the (duplicated) table.
- s/TE LSPs/TE-LSPs/  [x2]
- s/each TE LSP have to have at least have a bandwidth/each TE-LSP must
at least have a bandwidth/
- s/END-POINTS Object extensions/END-POINTS Object Extensions/
- s/From [RFC5440]the/From [RFC5440], the/
- s/This new C-Type/This new type/
- s/to be use./to be used./
- s/this object type object./this object./
- s/each endpoint type/each Endpoint Type
- s/The Label Set/The Label set/
- s/restrictions provided by signaling/restrictions which may apply to
signaling/

OLD
   an explicit
   value (Label set describing one label), mandatory range restrictions
   (Label set), OPTIONAL range restriction (Label set with L bit set)
   and single suggested value with the L bit and a single value
NEW
   an explicit or a suggested value (Label set describing one label, with
   the L bit respectively cleared or set), mandatory range restrictions
   (Label set with L bit cleared) and optional range restriction (Label set
   with L bit set).

- s/restrictionmay not/restrictions may not/
- s/GMPLS networks/GMPLS-controlled networks/
- s/sender and receivers/senders and receivers/
- s/endpoint type/Endpoint Type/  [on the figure]
- s/is received/is received./
- s/the endpoint type/the endpoint type/  [x2]
- The grammar should refer to RFC 5511, which should also be added to
the reference section.
- The intermediate definitions of <source-endpoint> and
<destination-endpoint> don't bring any summarization but make the P2MP
case quite cumbersome. I would map <p2p-endpoints> directly to endpoints
and restrictions.
- s/IPV4-ADDRESS,IPV6-ADDRESS/IPV4-ADDRESS, IPV6-ADDRESS/
- s/UNNUMBERED-ENDPOINT TLV./UNNUMBERED-ENDPOINT TLVs./
- s/The response SHOUd./The response SHOULD include the END-POINTS
object with only the unsupported TLV(s)./  [based on -11]
- s/LABEL-REQUEST or LABEL-SET TLV./LABEL-REQUEST or LABEL-SET TLVs./
- s/NO-PATH- VECTOR/NO-PATH-VECTOR/
- s/ENDPOINT object/END-POINTS object/
- s/the TLV where the PCE could not meet the constraint./the TLV(s)
related to the constraints the PCE could not meet./
- s/TLVs extensions/TLV Extensions/
- s/2.5.2.1.  IPV4-ADDRESS/2.5.2.1.  IPV4-ADDRESS TLV/
- s/This TLV represent/This TLV represents/  [x3]
- s/in [RFC3477] The/in [RFC3477]. The/
- s/satisfy requirement 13 for/satisfy requirement 13 of [RFC7025] for/
- s/have in the future more than one/have more than one/
- Drop "(the scope is then more clear)".
- s/O Bit/O bit/
- s/an old label./a previously allocated label./
- s/SHOULD be within/MUST be within/
- s/section 3.5.1/section 3.5.1./
- s/U, 0 and L/U, O and L/  [letter O != zero]
- s/to 1.If the/to 1. If the/
- s/is set the Action/is set, the Action/
- s/set of preferred (ordered) set of labels/set of preferred (ordered)
labels/
- s/without L bit set./with L bit cleared./
- s/2 LABEL_SET TLV/2 LABEL_SET TLVs/
- s/value if more/value, if more/
- s/the following TLV/the following TLVs/
- s/set or a PCErr message with/set, otherwise a PCErr message MUST be
sent with/
- s/IRO extension/IRO Extension/
- s/fulfill requirement 13/fulfill requirement 13 of [RFC7025]/
- s/interface id/interface ID/
- s/the IP address given/the given IP address/
- s/the PCE allocates/the PCE is able to allocate/
- s/e.g/e.g./
- s/assign labels then/assign labels, then/
- s/XRO extension/XRO Extension/
- s/labels ([RFC6001], in/labels ([RFC6001]). In/
- s/the XRO needs/PCEP's XRO needs/
- s/to support label exclusion./to enable label exclusion./
- At this stage, the figure depicting the XRO subobject should not
include any value for the type (3 is only suggested and it may be
confusing during IANA allocation step).
- Section 2.7 has many unnecessary blank lines.
- s/Label subobject is TBA/Label subobject is TBA-39/
- s/[RFC5521],The/[RFC5521], the/
- s/interface id/interface ID/
- s/the IP address given/the given IP address/
- s/LSPA extensions/LSPA Extensions/
- s/end-to-end protection context this/end-to-end recovery context, this/
- s/used by implementation/used by a PCE implementation/
- s/some more additional/some additional/
- s/provide a route/provide a path/  [x4 in bit-associated error messages]
------
Section 3.
---
- s/Additional Error Type/Additional Error-Types/
- s/Error- values/Error-values/
- s/for Error-Type 10 and A new/for Error-Types 4 and 10. A new/
- For the newly defined Error-Type (TBA-27), please add "value=0:
Unassigned". Without specific allocated Error-values, it is likely that
the IANA will give 1 to 4.
------
Section 4.
---
- s/implementation MAY allow/implementation may allow/
- s/BANDWIDTH object type TBA and TBA (Generalized
Bandwidth)parameters/BANDWIDTH object type TBA-2 and TBA-3 parameters/
- s/LOAD-BALANCING object type TBA parameters/LOAD-BALANCING object type
TBA-4 parameters
- s/..etc/etc./
- s/in CCAMP working group/in the TEAS or CCAMP working groups/
------
Section 5.
---
- Section 5.1 suggests some codepoints by indicating the Unassigned
values. This pool remains up to the IANA, please drop the 3 instances
related to the unassigned Object-Types.
- s/END-POINTS object/ END-POINTS Object/
- s/endpoint type/Endpoint Type/
- s/PCEP Error-Type and Error Values/PCEP Error-Types and Error-values/
- s/provide a route/provide a path/  [x4 in bit-associated error messages]
------
Appendix A.
---
- s/LOAD-BALANCING usage/LOAD-BALANCING Usage/
- s/V4 components/VC-4 components/
- s/n x VC4 traffic/n x VC-4 traffic/
- s/a SDH/an SDH/
- s/VC4 container/VC-4 containers/  [x2]
- s/BANDWIDTH object with OT=3/BANDWIDTH object with OT=TBA-2/
- Comas should be followed by a space, at least within sentences.
- s/5 path/5 paths/
- s/BANDWIDTH OT=3/BANDWIDTH OT=TBA-2/
------

Cheers,

Julien


Jul. 26, 2018 - [e-mail]:
> Hi, 
>
> Thanks a lot for the review.  Before posting a new revisions, please
> find my comments inline and a new version of the draft attached (with
> diff) 
>
>
>
>
> On 24 May 2018 at 09:26, Julien Meuric <[e-mail]
> <mailto:[e-mail]>> wrote:
>
>     Hi all,
>
>     Please find below my review of
>     draft-ietf-pce-gmpls-pcep-extensions, dug
>     out of the backlog. Let us discuss the main identified issues
>     first, we
>     will look at the minor comments and nits afterwards.
>
>     Generally speaking, the main items to improve are related to
>     clarification. Normative behavior and especially error handling often
>     need more accurate descriptions to limit ambiguities. Sorting error
>     values may also be deserved, especially with respect to existing error
>     types.
>
>     ------
>     Section 2.2
>     ---
>     - s/The PCE MAY try to follow/The PCE SHOULD  try to follow
>
> [MC] Agree 
>
>     - s/Otherwise, the PCE MAY use/Otherwise, the PCE MUST use/
>
> [MC] Agree 
>
>     ------
>     Section 2.3
>     ---
>     - The bidirectional symmetric bandwidth is defined with 2 MUST's and a
>     MUST NOT: the error case is not specified if the 3 requirements
>     are not
>     followed. I guess a sentence pointing to Error-Type 10/Value TBA-14
>     would address this.
>
>
> [MC] Rephrased with SHOULD:  In case a PCC does send a BANDWITH object
> with both generalized
> bandwidth and reverse generalized bandwidth and intends it to be a
> symetric bandwidth, then it is more a PCC internal error rather a PCEP
> protocol error. From PCEP point of view, it will be interpreted as
> asymetrical BW request.  
>
>     - s/asymmetric bandwidth, it SHOULD/asymmetric bandwidth, it MUST/
>
>
> [MC] Agree
>  
>
>     ------
>     Section 2.4
>     ---
>     - The following text is unnecessary and should be dropped:
>     "A PCC SHOULD be allowed to request a set of TE-LSP also in case
>     of GMPLS
>      bandwidth specification.
>
>
> [MC] Agree
>
>  
>
>      The LOAD-BALANCING has the same limitation as the BANDWIDTH for GMPLS
>      networks."
>     - Like above, the bidirectional symmetric bandwidth with
>     LOAD-BALANCING
>     is defined with 2 MUST's and a MUST NOT: the error case is not
>     specified
>     if the 3 requirements are not followed. I guess a sentence
>     pointing to a
>     relevant error (Type 10, new Value?) would address this.
>
>
> [MC] rephrased, same comment as BANDWIDTH
>
>  
>
>     - s/while specifying load balancing constraints, it SHOULD/while
>     specifying load balancing constraints, it MUST/
>
>
> [MC] agree
>
>  
>
>     - About the last paragraph ("For example [...] corresponding
>     request"):
>       * Have you considered moving into appendix?
>       * Codepoints are mentioned instead of TBA-2, -4...
>
>
> [MC] Agree
>
>  
>
>     ------
>     Section 2.5.1
>     ---
>     - s/restriction are not always/restriction may not be/
>
> [MC] Agree
>
>  
>
>     OLD:
>        Endpoint type 0 MAY be accepted by
>        the PCE, other endpoint type MAY be supported if the PCE
>        implementation supports P2MP path calculation.  A PCE not
>     supporting
>        a given endpoint type MUST respond with a PCErr with error code
>     "Path
>        computation failure", error type "Unsupported endpoint type in END-
>        POINTS Generalized Endpoint object type".
>     NEW:
>        A PCE may accept only Endpoint Type 0: Endpoint Types 1-4 apply
>     if the
>        PCE implementation supports P2MP path calculation. A PCE not
>        supporting a given Endpoint Type SHOULD respond with a PCErr with
>        Error Type 4, Value TBD "Unsupported endpoint type in END-POINTS
>        Generalized Endpoint object type". As per [RFC5440], a PCE
>     unable to
>        process Generalized Endpoints may respond with Error Type 3 or 4,
>        Value 2.
>
> [MC] Agree
>  
>
>     OLD:
>        A PCE not supporting one of those TLVs in a PCReq MUST respond
>     with a
>        PCRep with NO-PATH with the bit "Unknown destination" or "Unknown
>        source" in the NO-PATH-VECTOR TLV, the response SHOULD include the
>        ENDPOINT object in the response with only the TLV it did not
>        understood.
>     NEW:
>        When receiving a PCReq, a PCE unable to resolve the identifier
>     in one of
>        those TLVs MUST respond using a PCRep with NO-PATH and set the bit
>        "Unknown destination" or "Unknown source" in the NO-PATH-VECTOR
>     TLV.
>        The response SHOULD include the ENDPOINT object with only the
>     TLV it
>        did not understand.
>
> [MC] Agree
>  
>
>     - s/error type="Path computation failure"/Error Type 4/
>
> [MC] Agree
>  
>
>     ------
>     Section 2.5.2.5.
>     ---
>     - The I-D has chosen to allocate 2 TLV codepoints for LABEL-SET and
>     SUGGESTED-LABEL-SET. Any reason not to use just one including a
>     strict/loose bit to distinguish them?
>
>  
> [MC] The codepoints follow RFC3471 logic. There is not specific reason
> not to use a strict/Loose bit. The impact is mainly on the processing
> rules, as its possible to combine a strict label contraint and a list
> of preferred one. The encoding and processing rules have been changed
> to include a L bit.
>
>
>     - s/allocated on the first link/allocated on the first endpoint/
>
> [MC] Agree
>  
>
>     - The text "has the same encoding as the LABEL-SET TLV, it" is
>     redundant
>     and should be dropped.
>
> [MC] Agree
>  
>
>     OLD:
>           This Bit SHOULD be set to 0 in a SUGGESTED-LABEL-SET
>           TLV Set and ignored on receipt. This Label MAY be reused. The R
>           bit of the RP object MUST be set.
>     NEW:
>           The R bit of the RP object MUST be set to 1. In a
>     SUGGESTED-LABEL-SET
>           TLV, this bit SHOULD be set to 0 and ignored on receipt.
>
> [MC] Agree
>  
>
>     - In the 1st paragraph on page 19 ("Several LABEL_SET TLVs [...] be
>     ignored"), it seems that SHOULD's should be MUST's.
>
> [MC] Agree
>  
>
>     ------
>     Sections 2.6. & 2.7
>     ---
>     - To be consistence with RFC 7896, both sentences about the L bit
>     should
>     be dropped.
>
> [MC] Agree
>  
>
>     ------
>     Sections 3 & 5.5
>     ---
>     - Several errors (e.g., TBD-15, -28, -29...) may be moved to Type 4.
>     ---
>
>
> [MC] Agree
>  
>
>     Best regards,
>
>     Julien
>
>
>     _______________________________________________
>     Pce mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/pce
>     <https://www.ietf.org/mailman/listinfo/pce>
>
>


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.