Hello, I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see ​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft. Document: draft-ietf-pce-pce-initiated-lsp-09.txt Reviewer: Victoria Pritchard Review Date: 06 April 2017 IETF LC End Date: Intended Status: Standards Track Summary: I have some minor concerns about this document that I think should be resolved before publication. Comments: Although I was not very familiar with PCEP, I found the draft for the most part easy to understand, but did need to look up some things in the referenced documents and was unclear on a couple of small points. I have some suggestions that may help improve the draft for other readers, and I have some queries which may require clarification in the document. However, as most readers will be more familiar with the subject, perhaps not all comments will require any action. Major Issues: No major issues found. Minor Issues and Nits: Section 1, 1st paragraph Path Control Element / Path Computation Element ? Section 1, 2nd paragraph Stateful pce / Stateful PCE Reference link [I-D.ietf-pce-stateful-pce] points to section 3.1, not the references section. The 2nd sentence was hard to read, could be split into two sentences. Section 2 Last paragraph: Routing Backus-Naur Format / Routing Backus-Naur Form, to match the RFC title. Section 3.1 At the end of the 1st paragraph, "possible agile software-driven network operation" is then repeated in the next paragraph as "A possible use case is a software-driven network" Section 3.2 The acronyms SRP, PLSP and ERO are used a few times in this section. It may well be OK to assume most readers will be familiar with these, but would be good to have the expansion here too. Section 3.2, 3rd paragraph SRP-id-number / SRP-ID-number, for consistency The sentence beginning "The PCE MAY update" could be moved to a new paragraph, to separate it from the text regarding instantiation. Section 3.2, last paragraph Suggest to replace the "and" with a comma in this sentence: "During State Synchronization, a PCC reports the state of its LSPs to the PCE using PCRpt messages and setting the SYNC flag in the LSP Object. " "include the Create Flag" / "set the Create Flag" - also the create flag has not yet been mentioned. Actually I think this overview could be much briefer and simpler. There is a lot of detail about objects, flags and options, which is explained in later sections but complicates this overview. I think it might be good to also summarise here what the extension adds in terms of messages and flags, to clearly indicate what's new compared to the referenced documents. Section 4 After the first sentence, I would rephrase: "First, the Open message must include the Stateful PCE Capability TLV, defined in []." Then continue to the sentence beginning "A new flag is introduced in this TLV, ...". Section 4.1 In the flag bit description, "that the PCE may attempt to instantiate LSPs" could be changed to "that the PCE supports instantiating LSPs". Also rather than "in order to support", use "in order to enable". Not sure this section is necessary in this form with Figure 1. For example, I think the sync-optimizations draft specifies flags in a nice way (Section 7 of that draft), suggesting the bit to use without a diagram. The description here in section 4.1 could be rolled into the main body of Section 4. Also, is there any need to mention the U flag or the S flag in this draft? I noticed a mix and match between terminology of "STATEFUL-PCE-CAPABILITY TLV" vs "Stateful PCE Capability" TLV - could be made consistent, I'd suggest "Stateful PCE Capability TLV" throughout for readability. Also the same applies to "Path Computation LSP Initiate Message", "Path Computation LSP Initiate Request", "LSP Initiate Message", "LSP Initiate Request", "LSP initiation request". Would be nice to see this consistent throughout. Section 5.1 The 1st paragraph could be simplified by removing text about other objects and missing objects, and moving the final two sentences into sections 5.3 and 5.4. The 2nd paragraph states "The format ... for LSP instantiation", but this looks like it applies to deletion too. Suggest to remove "for LSP instantiation". On first read (although most readers will be familiar already) I would have liked to see some mention of what the Common Header is, maybe even just a reference to its definition in RFC5440. Section 5, final paragraph I would suggest you dont need the 3rd sentence at all, as correlation is already mentioned in the 1st sentence in this paragraph. Also, is SRP-ID-number incremented when an operation is requested *from* the PCE? Or *by* the PCE, or in either direction? Is it clearer to say "The PCE increments the current PCEP session's SRP-ID-number before including it in the PCInitiate message" (assuming any other usage is unchanged from the Stateful PCE draft)? Section 5.2 This section could be condensed in a similar way as I mentioned before regarding the I flag in the Capability TLV in Section 4.1. The text could be included at the bottom of section 5.1, and there is no need to draw the SRP object. Also no need for the reference as it's already in section 5.1. Perhaps also state the alternative case, that if the flag is set to 0, it indicates an instantiation request. Section 5.3, 1st paragraph "LSP instantiation is done by" / "The LSP is instantiated by". "an PCInitiate" / "a PCInitiate" Suggest removing the sentence beginning "The LSP is set up" Section 5.3 in general I suggest reorganising this section: -First discuss message contents that should be included for instantiation, i.e., objects mentioned in the format section above, and their contents. -Then once you have defined what the PCInitiate should look like, in new paragraph(s) talk through checking validity of the PCInitiate (non-zero PLSP-ID and missing ERO or SYMBOLIC-PATH-NAME) and discuss the error messages. -Then use the text describing LSP setup based on the info included. -Then discuss the PCRpt. You currently mention PCRpt in a couple of places in this section and it would be easier to read if it was in one place. For clarity, also state that in the PCRpt, both the Delegate and Create flags are in the LSP object. Section 5.3, 8th paragraph. "The PCEP-ERROR object SHOULD include the RSVP Error Spec TLV (if an ERROR SPEC was returned to the PCC by a downstream node)." Is that already covered by the 1st sentence in this paragraph, "relay to the PCE errors it encounters"? Could re-phrase to "If an RSVP Error Spec TLV was returned to the PCC by a downstream node, it should be included in the PCEP-ERROR object in the PCErr message". Also would prefer not to use 2 terms "RSVP Error Spec TLV" and "ERROR SPEC". suggeseted / suggested Is the sentence "After the LSP is set up, errors in RSVP..." necessary? By that I mean does that behaviour differ from normal, is it particular to this extension? Section 5.3, paragraphs 8 and 9 Would you want to inform the PCE of any limits during the capability exchange rather than sending an error later and ignoring further PCE requests? Section 5.3.1 The create flag could be described earlier. As mentioned above, Section 4 would be a good place to detail all the bits newly defined in this draft, the new message, the new flags. Again, I dont think you need to draw a diagram, just describe the flag added and its position within the LSP Object. Section 5.3.2 could be rolled into the description of the create flag since the two would be used together. Also, back in Section 3 it said you SHOULD include the SPEAKER-IDENTITY-ID TLV, whereas 5.3.2 instead uses MAY. SPEAKER-IDENTITY-ID is not actually defined in the sync-optimizations draft - assume you mean SPEAKER-ENTITY-ID? Also just to make it clear, you are re-using that TLV but this time within the LSP Object, and to give the PCE's identity, rather than in the OPEN object to give the speaker's identity? Also in the final sentence: "the TLV MUST be ignored the and the PCE MUST send a PCErr" - there's an extra "the" in the middle, and being very fussy, the TLV is not really ignored if you send an error message. Also if you do send the error message, is the rest of the PCRpt message ignored? Section 5.4 may benefit from splitting into multiple paragraphs, one for each error type, plus another for the final part beginning at "Following the removal". Section 6, 1st paragraph "are automatically delegated": suggest this reads "MUST be delegated". Automatically might imply you dont need to do anything to make this happen. If the PCE returns a delegation to the PCC, would the PCC then end up sending that PCE a PCRpt with the delegation bit set to zero? The first paragraph states that this is an error, but in that case, would it be? As "Redelegation Timeout Interval" and "State Timeout Interval" are both terms defined in the Stateful PCE draft, I would try to use the exact same terminology and same capitalisation found there throughout. Section 6, 3rd paragraph. Where it says "In case of PCEP session failure", does that mean failure at any point in time, or just a failure after the PCE has returned delegation in order to transfer the LSP to a different PCE? Also, is the LSP considered an orphan as soon as the initiating PCE returns delegation to the PCC? Or only if a PCEP session fails? Having these two bits of information in two different paragraphs makes it seem like they are separate but I would think they go together? If I have interpreted this correctly, I would suggest the following text: A PCE MAY return a delegation to the PCC to allow for LSP transfer between PCEs. The PCC will also regain control over a PCE-initiated LSP if the PCEP session fails and the Redelegation Timeout Interval timer expires. In both cases, the LSP is considered an "orphan" and the PCC MUST trigger the State Timeout Interval timer for that LSP ([I-D.ietf-pce-stateful-pce]). But, what is not clear to me, is what is happening at this point to try to delegate to another PCE? From looking at the Stateful PCE draft, I believe the PCC would send a report to 1/any(?) PCE it was connected to, setting the delegate flag to 1 to try to get that PCE to accept delegation. If I interpreted that correctly, i.e. the PCC actively tries to switch delegation to another PCE, it might be worth stating that here. Assuming a reply comes in from that PCE, with the delegate flag set, within the state timeout interval, all is good. However, I was slightly concerned by the statement from the Stateful PCE draft that says: "If the PCE accepts the LSP Delegation, it MUST set the Delegate flag to 1 when it sends an LSP Update Request for the delegated LSP (note that this may occur at a later time)." I dont know how far in the future "a later time" could be, but if the State Timeout Interval expires at the PCC first, wont the LSP get flushed? The text also says that to obtain control, a PCE can send a PCInitiate. So as an alternative to my initial interpretation, does that mean the PCC could advertise the LSP with delegate flag set to zero, and wait for a PCE to take control by sending a PCInitiate as described? This also makes me wonder how/when the initial PCE would decide to give up control? Is that in scope for this draft? Section 6, final paragraph The explanation about the timeout sounds like it applies to Stateful PCE in general, and therefore may not be necessary to explain in this draft? Also, on PCE crash (bearing in mind paragraph 3 above), does the Redelegation Timeout Interval occur first, and then the State Timeout Interval? Section 8.1 Meaning: Initiate. Being really picky, I would like this to match the full term used in this draft "Path Computation LSP Initiate Request" (or whatever term you settle on - see comment above between Section 4.1 and 5.1). This would then match RFC5440's way of doing it for PCReq. Kind regards, Victoria