Hi John, Adding my thoughts, that authors can incorporate if they agree with it...
On Mon, Sep 26, 2022 at 7:52 PM John Scudder <jgs= 40juniper....@dmarc.ietf.org> wrote: > Dear Authors, > > Thanks for your patience. Here’s my review. > > I’ve supplied my comments in the form of an edited copy of the draft. > Minor editorial suggestions I’ve made in place without further comment, > more substantive comments are done in-line and prefixed with “jgs:”. You > can use your favorite diff tool to review them; I’ve attached the iddiff > output for your convenience if you’d like to use it. I’ve also pasted a > traditional diff below in case you want to use it for in-line reply. I’d > appreciate feedback regarding whether you found this a useful way to > receive my comments as compared to a more traditional numbered list of > comments with selective quotation from the draft. > > Thanks, > > —John > > --- draft-ietf-pce-vn-association-08.txt 2022-09-20 > 16:26:37.000000000 -0400 > +++ draft-ietf-pce-vn-association-08-jgs-comments.txt 2022-09-26 > 10:18:24.000000000 -0400 > @@ -140,6 +140,34 @@ > making it the base for PCE applicability for ACTN. In this text > child PCE would be same as Provisioning Network Controller (PNC), and > the parent PCE as Multi-domain Service Coordinator (MDSC) [RFC8453]. > + > +--- > +jgs: I found the last sentence to be unclear. I see that RFC 6805 §1.4 > +defines Child PCE and Parent PCE: > + > + Parent PCE: A PCE responsible for selecting a path across a parent > + domain and any number of child domains by coordinating with child > + PCEs and examining a topology map that shows domain inter- > + connectivity. > + > + Child PCE: A PCE responsible for computing the path across one or > + more specific (child) domains. A child PCE maintains a relationship > + with at least one parent PCE. > + > +and I see that RFC 8751 §1.2.1 talks about how in the ACTN context > +C-PCE maps to PNC and P-PCE maps to MDSC: > + > + In this case, the P-PCE (as MDSC) interacts with multiple C-PCEs (as > + PNCs) along the interdomain path of the LSP. > + > +So *maybe* the sentence above is trying to say something like, > + > + As [RFC8751] explains, in the context of ACTN, the Child PCE is > + identified with the PNC, and the Parent PCE is identified with > + the MDSC. > + > +If that's not it, let's discuss please? > +--- > Dhruv: Make sense to me! > > In this context, there is a need to associate a set of LSPs with a VN > "construct" to facilitate VN operations in the PCE architecture. > @@ -150,7 +178,7 @@ > policy actions, setting default behavior, etc. > > This document specifies a PCEP extension to associate a set of LSPs > - based on Virtual Network (VN). > + based on their Virtual Network (VN). > > 1.1. Requirement Language > > @@ -186,7 +214,7 @@ > > * Path Computation: When computing a path for an LSP, it is useful > to analyze the impact of this LSP on the other LSPs belonging to > - the same VN. The aim would be optimize all LSPs belonging to the > + the same VN. The aim would be to optimize all LSPs belonging to the > VN rather than a single LSP. Also, the optimization criteria > (such as minimizing the load of the most loaded link (MLL) > [RFC5541]) could be applied for all the LSPs belonging to the VN > @@ -250,19 +278,48 @@ > (MDSC) and a child PCE (PNC). When computing the path, the child PCE > (PNC) refers to the VN association in the request from the parent PCE > (MDSC) and maps the VN to the associated LSPs and network resources. > - From the perspective of Parent PCE, it receives a virtual network > + From the perspective of the Parent PCE, it receives a virtual network > creation request by its customer, with the VN uniquely identified by > - an Association ID in VNAG as well as the Virtual Network identifier. > + an Association ID in the VNAG as well as the Virtual Network > identifier. > + > +-- > +jgs: in the preceding clause, do you mean that the VN is uniquely > +identified by either the Association ID or the VNI, or do you mean > +it's uniquely identified by the tuple (Association ID, VNI)? I think > +it's probably the latter, based on the defintion of the ASSOCIATION > +object in RFC 8697: > + > + Association ID (2 bytes): The identifier of the association group. > + When combined with other association parameters, such as an > + Association Type and Association Source, this value uniquely > + identifies an association group. > + > +Assuming that is right, perhaps rewrite like > + > + From the perspective of the Parent PCE, it receives a virtual network > + creation request by its customer, with the VN uniquely identified by > + the combination of the Association ID in the VNAG and the Virtual > + Network identifier. > + > +If that's wrong, let's discuss please. > Dhruv: Instead of association ID we should use the term association parameters to uniquely identify an association. See https://www.rfc-editor.org/rfc/rfc8697.html#name-unique-identification-for-a So either association parameters or the information inside the TLV can uniquely identify the VN. I suggest this text -> From the perspective of the Parent PCE, it receives a virtual network creation request by its customer, with the VN uniquely identified by the Association parameters (section 6.1.4 of [RFC8697]) in the VNAG or the Virtual Network identifier encoded in the VIRTUAL-NETWORK-TLV. > +-- > + > This VN may comprise multiple LSPs in the network in a single domain > - or across multiple domains. Parent PCE sends a PCInitiate Message > + or across multiple domains. The Parent PCE sends a PCInitiate Message > with this association information in the VNAG Object. This in effect > binds an LSP that is to be instantiated at the child PCE with the VN. > The VN association information could be included as a part of the > response as well. Figure 1 shows an example of a typical VN > + > +-- > +jgs: Do I correctly infer that it would also be fine for the VN > association > +information to *not* be included as part of the response? ("Could" implies > +"could not".) > +-- > Dhruv: Since as per RFC 8697, the first PCRpt message MUST have an association object. John is correct to flag this as an issue. We should say - "The VN association information MUST be included as a part of the first PCRpt message." > operation using PCEP. It is worth noting that in a multi-domain > scenario, the different domains are controlled by different child > PCEs. In order to set up the cross-domain tunnel, multiple segments > - need to be stitched, by the border nodes in each domain who receives > + need to be stitched, by the border nodes in each domain who receive > the instruction from their child PCE (PNC). > > > @@ -315,13 +372,25 @@ > MPI -> PCEP > > Figure 1: Example of VN operations in H-PCE Architecture > + > +-- > +jgs: Figure 1 has a bunch of notations that are never referred to in the > +text. I'm guessing these were carried forward from some other use of the > +figure where they were actually referred to, but now they just create > +visual clutter and potential confusion. Things I flagged as not being > +referenced include: "with VNAG = 10", "A", "C", "B13", "B43", "B31", > +"B34", "B". > + > +I suggest either tidying the figure up, or expanding the explanation text > +to refer to the labels. > +-- > > Whenever changes occur with the instantiated LSP in a domain network, > the domain child PCE reports the changes using a PCRpt Message in > which the VNAG Object indicates the relationship between the LSP and > the VN. > > - Whenever an update occurs with VNs in the Parent PCE (via the > + Whenever an update occurs with VNs in the Parent PCE (due to the > customer's request), the parent PCE sends an PCUpd Message to inform > each affected child PCE of this change. > > @@ -369,12 +438,27 @@ > that uniquely identifies the VN. It SHOULD be a string of printable > ASCII [RFC0020] characters (i.e., 0x20 to 0x7E), without a NULL > terminator. The Virtual Network Identifier is a human-readable > +-- > +jgs: Is there an established practice in PCE of using US-ASCII as the > +character set for human-readable strings? This would seem to be a > +practice that's discouraged by BCP 18, BCP 166. Was this discussed in > +the WG and a decision taken to not use internationalized strings? > +-- > Dhruv: Yes. Currently all strings in PCEP are ASCII. See https://notes.ietf.org/strings-in-pcep?view This was discussed briefly in the WG. The authors decided to keep ASCII with the idea that all string processing would be better to be handled together. > string that identifies a VN and can be specified with the association > information. An implementation could use the Virtual Network > Identifier to maintain a mapping to the VN association group and the > LSPs associated with the VN. The Virtual Network Identifier MAY be > specified by the customer or set via an operator policy or auto- > generated by the PCEP speaker. > + > +-- > +jgs: At the top of the page you say it's a "mandatory TLV", but then in > +the paragraph right above you say it "can be" specified and an > +implementation "could" use it. "Can" and "could" don't imply "mandatory" > +in the normal English sense. Does "mandatory" have some special meaning > +in the context of PCE, is there some other explanation for this > +discrepancy, ... ? > +-- > Dhruv: The TLV is mandatory to include in the association object. IMHO the text about what an implementation does with it should not use normative text, maybe "An implementation uses ..." > > The VIRTUAL-NETWORK-TLV MUST be included in VNAG object. If a PCEP > speaker receives the VNAG object without the VIRTUAL-NETWORK-TLV, it > @@ -452,21 +536,29 @@ > > 6. Security Considerations > > - This document defines one new type for association, which do not add > + This document defines one new type for association, which does not add > any new security concerns beyond those discussed in [RFC5440], > [RFC8231] and [RFC8697] in itself. > > Some deployments may find the Virtual Network Identifier and the VN > associations as extra sensitive; and thus should employ suitable PCEP > security mechanisms like TCP-AO [RFC5925] or TLS [RFC8253]. > +-- > +jgs: I have a few problems with the paragraph above. First, by "extra > +sensitive" do you mean "confidential"? If so, use that term or one like > +it, but in any case "extra sensitive" isn't specific enough. > + > +Second, assuming you do mean "confidential", TCP-AO isn't a suitable > +mitigation: AO doesn't provide confidentiality, only authentication. > +-- > Dhruv: Suggestion to authors to check out the last published association type RFC (RFC 9059) and could borrow the text from there! > > 7. IANA Considerations > > 7.1. Association Object Type Indicator > > - IANA is requested to make the assignment of a new value for the sub- > - registry "ASSOCIATION Type Field" (request to be created in > - [RFC8697]) within the "Path Computation Element Protocol (PCEP) > + IANA is requested to make the assignment of a new value in the sub- > + registry "ASSOCIATION Type Field" > + within the "Path Computation Element Protocol (PCEP) > Numbers" registry, as follows: > > Value Name Reference > @@ -538,8 +630,8 @@ > 8.6. Impact On Network Operations > > [RFC8637] describe the network operations when PCE is used for VN > - operations. Section 3 further specify the operations when VN > - associations is used. > + operations. Section 3 further specifies the operations when VN > + associations are used. > > 9. References > > > Thanks! Dhruv > > _______________________________________________ > Pce mailing list > Pce@ietf.org > https://www.ietf.org/mailman/listinfo/pce >
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce
