Summary: Has 2 DISCUSSes. Has enough positions to pass once DISCUSS positions are resolved.
(1) Section 6, Per “The answer can make that the LSP traverses some geographical place known to the attacker where some sniffing devices could be installed”, this is a concern. Good that it is here. However, it seems like the consequences could be even more expansive – confidentiality (sniffing), integrity (modifying the traffic) or availability (choose to drop it). (2) Section 6, [RFC8253] is mentioned a few times as having a variety of capabilities to mitigate the described threats. This is the right reference. However, the current text doesn’t explicitly state whether and how this guidance should be followed (should, must, is recommended?)
(1) Section 2.3, Nit (missing commas and periods), s/(SDH/SONET, G.709, ATM, MEF etc)/ (SDH/SONET, G.709, ATM, MEF, etc.)/ (2) In a few section. Typo (duplicate “section Section”). Recommend global s/section Section/Section/g (3) Section 6. Duplicate word. s/against against/against/
This document makes some well-needed extensions to existing PCEP concepts such as bandwidth, but I'm not convinced that the way they interact with existing PCEP functionality is sufficiently well specified to admit interoperable implementation. Specifically, we introduce the generalized bandwidth structures and reuse that encoding for the generalized load balancing structures, which includes a notion of "minimum bandwidth specification". But now that the bandwidth specification is a compound data structure instead of a scalar type, it's not guaranteed that we have a strict linear ordering with well-defined minimum. If we consider the specific case of Intserv, do I insist upon all three of the minimum bucket rate, minimum bucket size, and minimum peak data rate? Or perhaps I only care about the peak data rate and not the bucket size/rate. We need more text in order to specify what "minimum" actually means/measures. Similarly, I'm not sure all the referenced generalized bandwidth types/traffic parameters in Section 2.3 clearly indicate which structures/fields we are to incorporate by reference (see COMMENT). Section 2.1.2 says: GMPLS-CAPABILITY TLV it is RECOMMENDED that the PCC does not make use of the objects and TLVs defined in this document. Why is this not "the PCC MUST NOT make use of the objects and TLVs defined in this document"? Ignoring the peer's (non-)advertisement and plowing ahead seems like a recipe for non-interoperability. Section 2.5.1 notes that: <p2mp-endpoints> ::= <endpoint> [<endpoint-restriction-list>] [<endpoint> [<endpoint-restriction-list>]]... For endpoint type Point-to-Multipoint, several endpoint objects MAY be present in the message and each represents a leave, exact meaning depend on the endpoint type defined of the object. If all <endpoint>s represent leaves, then how is the head node specified? I couldn't find a full spcification for some of the fields in the XRO Label subobject (Section 2.7) by chasing the indicated references (see COMMENT).
Section 1 Please expand OTN and WSON on first use. Section 1.4 It's very unclear to me what kind of support, from/by what entities/data structures, under what conditions, these tables are attempting to indicate. We should probably be consistent whether we talk about just "FOO" or "FOO object" as the hanging text for these bulleted lists. From [RFC8282]: o SWITCH-LAYER: address requirements (1, 2 and 3) for the TE-LSP and indicates which layer(s) should be considered, can be used to represent the RSVP-TE generalized label request. [...] nit: this looks like a comma splice. The PCEP extensions defined later in this document to cover the gap are: Two new object types are introduced for the BANDWIDTH object (Generalized bandwidth, Generalized bandwidth of existing TE-LSP for which a reoptimization is requested). I'm confused by this language "new object types are introduced for the BANDWIDTH object". My understanding was that objects did not nest: that is, objects have a given structure and can sometimes contain TLVs, but do not contain other objects. So, my current understanding is that new objects are introduced that can appear where the BANDWIDTH object would previously have appeared, but they are separate object (type)s from the RFC 5440 BANDWIDTH objects. (This language is used in the next couple items as well.) To be clear, this is at most an editorial consideration, essentially whether to use "introduced for" or something like "introduced akin to". Section 2.1.2 If the PCE does not include the GMPLS-CAPABILITY TLV in the OPEN message and the PCC does include the TLV, it is RECOMMENDED that the PCC indicates a mismatch of capabilities. Moreover, in case that the PCC does not receive the Indicate how, to whom? Section 2.2 This granularity applies to all links in the path, right? So I can't request label-level granularity for one hop and indicate that I only care about node-level granularity for the other hops? Section 2.3 [similar comments apply here to what I mentioned at the end of Section 1.4] The Bw Spec Type correspond to the RSVP-TE SENDER_TSPEC (Object Class 12) C-Types Should we ask IANA to update the SENDER_TSPEC registry to note that it is used for PCEP as well as RSVP? The encoding of the fields Generalized Bandwidth and Reverse Generalized Bandwidth is the same as the Traffic Parameters carried in RSVP-TE, it can be found in the following references. Object Type Name Reference 2 Intserv [RFC2210] 4 SONET/SDH [RFC4606] 5 G.709 [RFC4328] 6 Ethernet [RFC6003] 7 OTN-TDM [RFC7139] 8 SSON [RFC7792] It's quite confusing to have the table heading be just "object type" when this is the value in the field named "Bw Spec Type" and corresponds to class type values in the SENDER_TSPEC registry. Also, I looked up the Intserv case, and RFC 2210 doesn't really give me a clear picture of what I'm supposed to encode as the "transport parameters". I think it's supposed to be the 12-octet assembly consisting of the token bucket rate, token bucket size, and peak data rate, but I have very low confidence in that assessment. On the other hand, RFC 4606 has a very nice data structure layout in Section 2.1, "SONET/SDH Traffic Parameters". On the gripping hand, there's not a clear "bandwidth" number in that structure that I can apply a comparison to for load-balancing purposes. It doesn't look like I'll have time to check the other four cases right now, but that will need to be done before final publication. Section 2.4 I'm having trouble parsing: The LOAD-BALANCING object [RFC5440] is used to request a set of maximum Max-LSP TE-LSP having in total the bandwidth specified in BANDWIDTH, each TE-LSP having a minimum of bandwidth. Is it intended to read: The LOAD-BALANCING object [RFC5440] is used to request allocation of a set of at most Max-LSP TE-LSPs, having in total the bandwidth specified in BANDWIDTH, with each TE-LSP having at least a specified minimum bandwidth. ? [similar comments apply here to what I mentioned at the end of Section 1.4] Bandwidth Spec Length (16 bits): the total length of the Min Bandwidth Spec field. It is to be noted that the RSVP-TE traffic specification MAY also include TLV different from the PCEP TLVs. The length MUST be strictly greater than 0. It's not entirely clear to me why the note about different TLVs in RSVP-TE and PCEP belongs here. Section 2.5.1 Endpoints label restriction may not be part of the RRO or IRO, they can be included when following [RFC4003] in signaling for egress endpoint, but ingress endpoint properties can be local to the PCC and not signaled. [...] nit: the first comma looks like a comma splice. 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". [...] s/TBD/TBA-15/ The TLVs present in the request object body MUST follow the following [RFC5511] grammar: It feels a bit like a type error to use RBNF to describe the layout of TLVs within a TLV block, as RBNF acts on objects. Section 220.127.116.11 The LABEL-REQUEST TLV indicates the switching capability and encoding type of the following label restriction list for the endpoint. Its format and encoding is the same as described in [RFC3471] Section 3.1 Generalized label request. [...] Presumably the "Its" refers to just the value portion of the TLV? That should probably be stated explicitly. Section 18.104.22.168 Is there any reason for the section title to not be "LABEL-SET TLV" for consistency with the other sections? A LABEL-SET TLV represents a set of possible labels that can be used on an interface. If the L bit is cleared, the label allocated on the first endpoint MUST be within the label set range. [...] Is this MUST binding on the PCC that generates a request, or on the computed LSP returned by the PCE? A LABEL-SET TLV with the O and L bit set MUST trigger a PCErr message with error type="Reception of an invalid object" error value="Wrong LABEL-SET TLV present with O and L bit set". A LABEL-SET TLV with the O bit set and an Action Field not set to 0 (Inclusive list) or containing more than one subchannel MUST trigger a PCErr message with error type="Reception of an invalid object" error value="Wrong LABEL-SET TLV present with O bit and wrong format". If a LABEL-SET TLV is present with O bit set, the R bit of the RP object MUST be set, otherwise a PCErr message MUST be sent with error type="Reception of an invalid object" error value="LABEL-SET TLV present with O bit set but without R bit set in RP". nit: I don't know if it makes more sense to use the TBA-25, TBA-26, and TBA-24 values in these descriptions. Section 2.6 The IRO as defined in [RFC5440] is used to include specific objects in the path. RSVP-TE allows to include label definition, in order to fulfill requirement 13 of [RFC7025] the IRO needs to support the new subobject type as defined in [RFC3473]: nit: this looks like a comma splice. (A similar construction appears in Section 2.7 as well.) Section 2.7 U (1 bit): see [RFC3471]. C-Type (8 bits): the C-Type of the included Label Object as defined in [RFC3471]. Label: see [RFC3471]. Sorry, where exactly in RFC 3471? I do not see discussion of a U bit or C-Type therein. (Perhaps RFC 3473 was intended? Though, RFC 3473 seems to refer back to 3471 for the U parameter, again without section reference.) Section 6 It seems that a malicious PCC might be able to effect a denial of service attack on the PCE by attempting to make many requests that consume lots of resources (whether on the PCE itself or in the managed network elements). In addition Technology specific data plane mechanism can be used (following [RFC5920] Section 5.8) to verify the data plane connectivity and deviation from constraints. nit: "In addition, technology-specific" Appendix A It's not entirely clear to me why this specific group of examples was chosen and no others. (The appendix does not seem to be referenced from elsewhere in the document, so it appears fairly random to a reader making it that far.)
I think Section 2.1.2 needs to better explain what happens when a PCC sends extensions but the recipient does not support them. Please respond to the Gen-ART review.
* Section 2.5.2 "In this object type the order of the TLVs MUST be followed according to the object type definition." Not sure what this means. Can you clarify? * Section 2.7 "C-Type (8 bits): the C-Type of the included Label Object as defined in [RFC3471]." I could not find any references to C-Types in RFC3471. Shouldn't you be referring to RFC3473 instead? I have a similar comment for the Label field.
Purely editorial comment: I would recommend to move most of the gap analysis of section 1 (actually most of the text of the whole section) into the appendix and only summarise the extensions specified in this doc instead. nit: sec 6: s/In order to protect against against the malicious PCE case/In order to protect against the malicious PCE case/ -> 2x against
Hi, thanks for this document. I have a discuss point that shouldn't be difficult to resolve: Why do you define a flag field in the GMPLS-CAPABILITY TLV if you don't have any flag? I guess the easy answer is that there might be some in the future. If so, I tend to think that creating a registry for that field would be a good thing to do now. -m