I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at . Document: draft-ietf-pce-association-diversity-10 Reviewer: Dale R. Worley Review Date: 2019-10-12 IETF LC End Date: 2019-10-18 IESG Telechat date: not known Summary: This draft is on the right track but has open issues, described in the review. There are various exposition issues, as detailed below. There are some minor technical issues about reporting of error conditions, the significance of the P bit in DISJOINTNESS-STATUS-TLV, the detailed definition of the new objective functions, etc. as detailed below. But it seems like a number of complex cases haven't been clearly specified: (1) the effect of the P bit if multiple LSPs have P=1, (2) the interaction of SVEC and disjoint-groups, and (3) the interaction of partially overlapping disjoint-groups and/or SVEC sets. Major issues: The relationship of this mechanism with SVEC seems to be important but is not clearly stated. The relevant sections of the text seem to be: section 4 para 2, section 5.3, and section 5.4 from "[RFC5440] uses SVEC diversity flag" on. I think that they need to be pulled into one section. Then it will be possible to have a good description of the interaction with SVEC. At the end of section 5.6 is the revelation that (1) the DISJOINTNESS-CONFIGURATION-TLV is not attached to a group (in some sense), but instead a separate copy of it is attached to every LSP in the group, (2) these copies must match in the group IDs they carry and also the T, S, N, and L flags in them must agree, (3) (I suspect) any OF-code must agree, and (4) the P flags in them may differ. This information is needed as background for a number of parts of the text to make sense, particularly the description of the P bit in section 5.2 and section 5.5. I recommend that the last para of section 5.5 be moved to section 5.1 or the beginning of section 5.2, so the reader is aware that the descriptions of each LSP carry separate, and not necessarily identical, DISJOINTNESS-CONFIGURATION-TLVs. The path computation effects of the P bit are described in the "P" item in section 5.2 and section 5.5. But the descriptions are unclear, or perhaps they presume that there are only two LSPs in the group. I think the intended meaning is that all of the LSPs in the group with P=1 are computed first, and then with those LSPs fixed, the LSPs in the group with P=0 are computed. This will cause shortest-path constraints (and other objective functions) to be optimized on the P=1 LSPs, and those paths will not be de-optimized by competition from the other paths. This should probably be pulled out of the description of the "P" in its TLV and put into a separate paragraph. Minor issues: Nits/editorial comments: In general, check the uses of "could" and "would". I think "would" is sometimes used to mean "... in that situation, the XXX MUST ...", and "could" is sometimes used where "MAY" would be clearer. The running title is given as "ASSOC-DISJOINT". I think "PCEP Diversity Constraint Signaling" would be clearer. Abstract The proposed extension allows a Path Computation Client (PCC) to advertise to a PCE that a particular LSP belongs to a disjoint-group, thus the PCE knows that ... It would be clearer if "a disjoint-group" was amplified to "a particular disjoint-group". Table of Contents There are a few section titles in which important words aren't capitalized ("functions" in section 5.4), or unimportant words are capitalized ("On" in sections 8.5 and 8.6). The title of section 8.4 is a verb phrase, compared to the titles of the other subsections of section 8. I suggest "Verification of Correct Operations" or maybe "Correct Operation Verification". Perhaps change the title of Appendix A to just "Contributors", since there is no other enumeration of contributors. 1. Introduction This document specifies a PCEP extension to signal that a particular group of LSPs should use diverse paths including the requested type , ---------------------------------------^ of diversity. Add a comma as indicated, to make it clear that "including" modifies "signal" rather than "paths". This document specifies a PCEP extension to signal that a particular group of LSPs should use diverse paths including the requested type of diversity. This implies that the "group" here is an instance of the association "grouping" of the preceding paragraph, but the Intro doesn't state that this document's grouping is an instance of the generic grouping mechanism. Perhaps This document specifies a PCEP grouping to signal that ... 3. Motivation A customer may request that the operator provide two end-to-end disjoint paths across the IP/MPLS core. I suggest "the operator's IP/MPLS core". [There are various places where the document seems to be phrased from the point of view of the network vendor rather than the point of view of the endpoint/user. The IETF convention is to focus on the endpoint or user.] The customer may use those paths as primary/backup or active/active. This phrasing uses an adjective as a noun. I prefer: The customer may use these paths with a primary/backup or an active/active configuration. -- Different level of disjointness may be offered: Perhaps "Different levels of disjointness can be defined:" 4. Applicability In the figure above, let us consider that the customer wants to have two disjoint paths between CE1/CE2 and CE3/CE4. I think that to make this accurate, you need to say "two disjoint paths, one between CE1 and CE2 and one between CE3 and CE4." This section seems disorganized. In particular, 2nd and 3rd paras don't seem to add much. Para 2 should be relocated to the SVEC discussion (see "Major issues). Para 3 seems to have two sentences of background information (I think it's duplicated in section 1) and one sentence that essentially states that this document defines a "diversity group" of LSPs which is a type of the generalized "LSP group". Using PCEP, the PCC could indicate that a disjoint path computation is required, such indication should include disjointness parameters such as the type of disjointness, the disjoint group identifiers, and any customization parameters according to the configured local policy. As mentioned previously, the extension described in [I-D.ietf-pce-association-group] is well suited to associate a set of LSPs with a particular disjoint-group. This touches on the right topics, but doesn't really connect them. Also, don't use "could" for an action that this document defines how to do. You want to say something like Using the extension to PCEP defined in this document, the PCC uses the [I-D.ietf-pce-association-group] extension to indicate that a group of LSPs are required to be disjoint; such indication should include disjointness parameters such as the type of disjointness, the disjoint group identifiers, and any customization parameters according to the configured local policy. -- Figure 2 - Sample use-cases for carrying disjoint-group over PCEP session In this figure, there seems to be a conflict in notation: Case 1 uses "PCC" seemingly in the same role for which Case 2 uses "PE 1" and "PE 3". The captions of both cases refer to "PCC". Also, the Case 1 and Case 2 captions run into each other. Using the disjoint-group within a PCEP messages may have two purpose: Better would be "The disjoint-group within PCEP messages is used for:" 5.1. Association Group As per [I-D.ietf-pce-association-group], LSPs are associated with other LSPs with which they interact by adding them to a common association group. To be exact, you want to say "by adding all of them"; by default "them" refers only to the "LSPs" that is the subject of the sentence. The Association parameters, as described in [I-D.ietf-pce-association-group] as the combination of the mandatory fields Association type, Association ID and Association Source in the ASSOCIATION object, that uniquely identify the association group belonging to this association. If the optional TLVs - Global Association Source or Extended Association ID - are included, then they are included in combination with mandatory fields to uniquely identify the association group. Use either "field" or "TLV" consistently (unless the TLVs really are different from the "fields"). This is awkward. Better would be As described in [I-D.ietf-pce-association-group], the association group is uniquely identified by the combination of these fields in the ASSOCIATION object: Association Type, Association ID, Association Source, and (if present) Global Association Source or Extended Association ID. ... This document defines a new ... Start a new para with this sentence. o Association type = TBD1 ("Disjointness Association Type") for Disjoint Association Group (DAG). This seems redundant, unless there exists another type that is also associated with Disjoint Association Group (DAG). Otherwise, you can probably use o Association type = TBD1 Disjoint Association Type -- This capability exchange for the association type described in this document (i.e. Disjointness Association Type) MUST be done before using the disjointness association. This can be simplified to This capability exchange for Disjointness Association Type MUST be done before using the disjointness association. There seem to be multiple uses of "Disjoint Association Group (DAG)" and "Disjointness Association Type (TBD1)", each of which states the equivalence of two things. You should be able to use one instance of each. Also, it's probably worth putting "Disjointness Association Type" into section 2. This association type is considered to be both dynamic and operator- configured in nature. It might be worth providing references for the meaning of "dynamic" and "operator-configured". A disjoint group can have two or more LSPs, [...] The significance of this clause to the rest of the sentence is unclear; it seems obvious that a disjoint group needs at least two members before it is significant. However, as a matter of protocol, can a group that has only one LSP be established? The disjoint status is informed to the PCC. This seems to be describing how to report to the PCC that the computation was not done as requested. I think you want to provide more details here, or a pointer to them. In any case, "disjoint status" isn't really a clear statement that the error condition will be reported. Associating a particular LSP to multiple disjoint groups is authorized from a protocol perspective, however there is no assurance that the PCE will be able to compute properly the multi-disjointness constraint. You want to state *much* more clearly what might happen. This starts with "A particular LSP MAY be associated to multiple disjoint groups, but in that case, the PCE MAY ..." What are the outer boundaries of the results that the PCE MAY return? Is the PCE required to provide any particular error indication? 5.2. Disjoint TLVs See "Major issues" regarding explaining that there is a separate copy of DISJOINTNESS-CONFIGURATION-TLV for each LSP, and they needn't all be the same. All of the constraints described in the last paragraph of section 5.5 should be moved to here, or perhaps section 5.1. 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Type = TBD2 | Length | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Flags |T|P|S|N|L| +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ "Flags" should be "Reserved". * P (Shortest path) bit: when set, this indicates that the computed path of the LSP SHOULD satisfy all the constraints and objective functions first without considering the diversity constraint. ... See "Major issues" for providing a separate paragraph to explain the semantics of the "P" bit. * T (Strict disjointness) bit: when set, if disjoint paths cannot be found, PCE SHOULD return no path for LSPs that could not be be disjoint. When unset, the PCE is allowed to relax disjointness by either applying a requested objective function (cf. Section 5.4 below) or using any other behavior if no objective function is requested (e.g. using a lower disjoint type (link instead of node) or fully relaxing disjointness constraint). Further see Section 5.6 for details. Instead of "or using any other behavior if no objective function is requested" it would be more conventional to say "or, if no objective function is requested, using local policy". The DISJOINTNESS-STATUS-TLV uses the same format as the DISJOINTNESS- CONFIGURATION-TLV with a different type TBD3 (in the TLV). The L, N, and S flags are set based if the computed path meet the disjointness criteria. Should be "... are set if the respective disjointness criterion was requested and the computed paths meet it" The P flag is set to indicate that the computed path is the shortest Surely "the shortest" needs further explanation -- the shortest path that meets what constraints? I suspect that this just reflects the P flag in the CONFIGURATION-TLV -- is there a situation where it would not? and the T flag has no meaning in the DISJOINTNESS- STATUS-TLV and MUST NOT be set while sending and ignored on receipt. This clause should be a separate sentence. Any new flag defined for the DISJOINTNESS-CONFIGURATION-TLV is be automatically applicable to the DISJOINTNESS-STATUS-TLV. This can't be correct, as there is no defined way to automatically apply a new flag to DISJOINTNESS-STATUS-TLV. (The previous paragraph defines three separate semantics for flags in DISJOINTNESS-STATUS-TLV!) But you can say: Any document defining a new flag for the DISJOINTNESS-CONFIGURATION-TLV automatically defines a new flag with the same name and in the same location in DISJOINTNESS-STATUS-TLV; the semantics of the flag in DISJOINTNESS-STATUS-TLV MUST be specified in the document that specifies the flag in DISJOINTNESS-CONFIGURATION-TLV. 5.3. Relationship to SVEC The PCE would consider both the objects I think you mean MUST instead of "would". In case no such path is possible (or the constraints are incompatible), the PCE MUST send a path computation reply (PCRep) with a NO-PATH object indicating path computation failure as per [RFC5440]. Adding "(or the constraints are incompatible)" to "no such path is possible" is redundant! This discussion is not clear to me. Partly it is because I'm not familiar with SVEC. But I suspect that "synchronization of a set of path computation requests" means that "the computations have to be done together" and "dependent requests" means something similar to the constraints between computed paths specified by the DISJOINTNESS-CONFIGURATION-TLV. Is it possible to provide a comparison between the requirements specified by SVEC and the requirements specified by DISJOINTNESS-CONFIGURATION-TLV? Also, is there any specific relationship between the set of LSPs in an SVEC and the set of LSPs in a disjoint-group? What if multiple SVECs and disjoint-groups overlap in a complex way? 5.4. Disjointness Objective functions To minimize the common shared resources (Node, Link or SRLG) between a set of paths during path computation three new OF-codes are proposed: Actually, the new codes are to allow specification of minimization of shared resources. Also, an RFC doesn't just proposed the codes, it specifies them. So this is better phrased: Three new OF-codes allow specification of minimization of common shared resources (Node, Link or SRLG) between a set of paths during path computation: -- MSL * Name: Minimize the number of shared (common) Links. * Objective Function Code: TBD4 * Description: Find a set of paths such that it passes through the least number of shared (common) links. MSS * Name: Minimize the number of shared (common) SRLGs. * Objective Function Code: TBD5 * Description: Find a set of paths such that it passes through the least number of shared (common) SRLGs. MSN * Name: Minimize the number of shared (common) Nodes. * Objective Function Code: TBD6 * Description: Find a set of paths such that it passes through the least number of shared (common) nodes. The use of "it" in the above definitions is unclear, since the paths are "they". There are a number of similar but different ways that "the number of shared" entities can be defined. E.g., if four LSPs pass through a single node, does that count as 1 (a single node is shared), 4 (four LSPs pass through a node that is shared), 6 (six pairs of LSPs conflict, in that the paths in the pair share a node)? That is, are we counting (and thus minimizing) shared entities, the number of LSPs that violate the requested constraint, or the number of pairs of LSPs that overlap in an unacceptable way? If the OF-list TLV is included in the Association Object, the OF-code inside the OF Object MUST include one of the disjoint OFs defined in this document. If this condition is not met, the PCEP speaker MUST respond with a PCErr message with Error-Type=10 (Reception of an invalid object) and Error-Value=TBD9 (Incompatible OF code). I find it surprising that you've defined a specific error code for an unexpected OF-code, but you don't apply that error code to the situation where there are multiple OF-codes. Instead, you have all but the first silently ignored. There's also the intersection case where the first OF-code is legitimate, but the second OF-code is not one in this set. Does that get an "Incompatible OF code" error, or is the second one just ignored? I think it's cleaner to change TBD9 to "Incompatible OF-list for disjoint-group" and require it for all of these cases. [RFC5440] uses SVEC diversity flag for node, link or SRLG to describe the potential disjointness between the set of path computation requests used in PCEP protocol. This paragraph seems very important, but completely out of place here. Might it fit better in section 5.3? This document defines three new OF-codes to maximize diversity as much as possible, in other words, minimize the common shared resources (Node, Link or SRLG) between a set of paths. This paragraph seems redundant after the detailed discussion of these codes. The remaining paragraphs of this section seem like they should go in section 5.3. 5.5. P Flag Considerations See above queries about the P flag. As mentioned in Section 5.2, the P flag (when set) indicates that the computed path of the LSP SHOULD satisfies all constraints and objective functions first without considering the diversity constraint. There are complexities that don't seem to be addressed. E.g., if there are two LSPs with P=1 and two with P=0, I would expect that the first two would be computed with full consideration of the disjointness constraint between them. So "without considering the diversity constraint" really isn't a good description of what is intended. 5.6. Disjointness Computation Issues Also, in case of network event leading to an impossible strict disjointness, the PCE MUST send a PCUpd message ... I think you want to start a new paragraph here and say In case of a network event which makes it impossible to satisfy the constraints, the PCE MUST send a PCUpd message ... -- The actual level of disjointness computed by the PCE ... More correct to say The actual level of disjointness of the paths computed by the PCE ... -- While the DISJOINTNESS-CONFIGURATION-TLV defines the expected level of disjointness required by configuration, the DISJOINTNESS-STATUS-TLV defines the actual level of disjointness computed. Better to s/expected/desired/ and s/actual/achieved/. There are some cases where the PCE may need to completely relax the disjointness constraint in order to provide a path to all the LSPs that are part of the association. A mechanism that allows the PCE to fully relax a constraint is considered by the authors as more global to PCEP rather than linked to the disjointness use case. As a consequence, it is considered as out of scope of the document. This para doesn't seem to make any difference. As long as the T bit is clear and the local policy allows complete relaxation of the requested disjointness, this case is covered within the mechanisms of this document. And this case will be reported in a straightforward way by a DISJOINTNESS-STATUS-TLV with all bits clear. 6. Security Considerations This document defines one new type for association ... Better "This document defines one new PCEP association type ..." After para 1, you probably want to add this case: "A spurious LSP can have flags that are inconsistent with those of the legitimate LSPs of the group and thus cause LSP allocation for the legitimate LSPs to fail with an error."