Re: [Pce] Shepherd Review of draft-ietf-pce-gmpls-pcep-extensions
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 and don't bring any summarization but make the P2MP case quite cumbersome. I would map 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/
[Pce] Shepherd Review of draft-ietf-pce-gmpls-pcep-extensions
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
- s/Otherwise, the PCE MAY use/Otherwise, the PCE MUST use/
--
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.
- s/asymmetric bandwidth, it SHOULD/asymmetric bandwidth, it MUST/
--
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.
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.
- s/while specifying load balancing constraints, it SHOULD/while
specifying load balancing constraints, it MUST/
- About the last paragraph ("For example [...] corresponding request"):
* Have you considered moving into appendix?
* Codepoints are mentioned instead of TBA-2, -4...
--
Section 2.5.1
---
- s/restriction are not always/restriction may not be/
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.
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.
- s/error type="Path computation failure"/Error Type 4/
--
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?
- s/allocated on the first link/allocated on the first endpoint/
- The text "has the same encoding as the LABEL-SET TLV, it" is redundant
and should be dropped.
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.
- In the 1st paragraph on page 19 ("Several LABEL_SET TLVs [...] be
ignored"), it seems that SHOULD's should be MUST's.
--
Sections 2.6. & 2.7
---
- To be consistence with RFC 7896, both sentences about the L bit should
be dropped.
--
Sections 3 & 5.5
---
- Several errors (e.g., TBD-15, -28, -29...) may be moved to Type 4.
---
Best regards,
Julien
___
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce
