Re: [Pce] Chair's Review of draft-ietf-pce-stateful-pce-11

[Julien Meuric]

Hi Robert. Thank you for your help to move this forward. Please find my comments below [JM]. Note that a couple of your answers are not aligned with the proposed resolutions currently included the I-D: I was fine with these, therefore please make sure you are so that I can send to the IESG. Julien Jan. 18, 2016 - n...@hq.sk: Hello, please find my comments on the pending items. On 10/26/2015 10:10 PM, Ina Minei wrote: Julien,  Thank you for the detailed review, please find answers inline below ###.  I have incorporated the overwhelming majority of the comments, explained the reason for not incorporating a couple of them, and am still working with the co-authors on a couple of items marked "pending", which we will close on shortly. Two questions and one ask 1. Forward references to SRP object and SRP-ID - there are several in the comments, though the relevant section is always mentioned. How should such forward references be addressed? 2. Section 7 - s/defined in this document/defined in that (aforementioned) document/ The comment was not clear to me. The intention is for the flags to be set as explained for the new objects we are defining here, can you clarify the comment? 3. Can you please review the comments that were not incorporated and let us know if you agree? Thank you,  Ina  On Fri, Oct 16, 2015 at 7:40 AM, Julien Meuric <julien.meu...@orange.com> wrote: Dear authors, To prepare the upcoming move to the IESG, please find below my review of the aforementioned I-D (at last!). _Summary_ Main qualities: - core specification is clear; - wording is smooth, with very few typos; - the manageability and security sections have been included with relevant text. Main issues to point out: - RFC 2119 keywords (picky me, but so is the IESG: ask JP about 5440); - a few corner/error cases; - consistency with existing RFCs (5440, 5886). I also take the opportunity to remind the WG that including codepoint values from existing registries before allocation by the IANA (which can be requested early) is a very bad idea, whatever the WG. _Detailed Comments_ [snip] - s/include an empty ERO/include an empty RRO/ [Along with RFC 5440 (section 7.10), the object sent by a PCC to report to a PCE is an RRO: let us keep it consistent.] ### XXX Pending In this case PCRpt differs from PCReq. The PCE needs to know the ERO object for each LSP, as may have been pushed by a different PCE. Reporting RRO is not sufficient, as that contains the effective LSP path, e.g. with loose hops expanded by the PCC. That is why ERO is a mandatory object in PCRpt (as part of intended_path), hence we specify an empty object for the end-of-sync marker. [JM] OK, this is addressed in the current version.   - Avoiding "positive acknowledgements for properly received synchronization messages" has scalability benefits in normal situations, but the PCC is blind and may keep on sending PCRpt to dead processes behind up PCEP sessions. Have you consider acknowledgement, possibly using a compression mechanism like the one defined later in the I-D? ### XXX Pending  The association between a PCEP session and PCE processes is something which I would consider an internal PCE detail, and it should be covered by the next sentence (e.g. raise PCErr 20/1). [JM] I still feel unwise to consider a lack of feedback as a proof of synchronization. What if, from time to time, a PCRpt gets lost? I do not think acknowledgement would be a pain to add, but its lack can easily turn to that in operational situations. - When mentioning errors, adding a sentence reminding that RFC 5440 already defines a set of applicable error codes would be valuable. ### XXX Pending I agree, this is an extension, so implementations should reuse RFC5440 errors when appropriate. [JM] OK, aligned with the I-D. - In section 5.5.1, it is not clear if an empty LSP Update Request with a Delegate flag to 1 is an acceptable way for a PCE to send a delegation acknowledgement: to be clarified.  ### XXX Pending It is not, as that would be seen as a request to modify the LSP setup to empty. Such an acknowledgement would have to include full configuration as previously reported -- which would be handled as a normal update. [JM] The I-Ds says the contrary: to be checked. Note that empty could be loose, which seems possible to handle at the signaling level. - s/SHOULD return the LSP delegation/MUST return the LSP delegation/ ### This should remain SHOULD. The nice way to do it is to return it explicitly, but it may choose to wait until the next update and return the delegation then by not setting the delegate flag.    - In section 5.5.3, assuming an LSP was delegated, does the reception by the PCC of a non empty LSP Update Request with a Delegate Flag to 0 trigger an error? ### XXX Pending It could, if we want to be strict about it, but it does not really have an impact on protocol operation: delegate=0 should kick in first, which means the PCC can safely discard any extra payload. [JM] OK, seems aligned to the I-D.   - s/<ERO><attribute-list>/<RRO><attribute-list>/ [Per RFC 5440, a report from PCC to PCE is RRO.] ### XXX Pending - The use of the optional xRO is mentioned, but its relationship with the RRO (formerly ERO) is not clear. I suspect some assumptions are made on the way the ERO/RRO are populated; RFC 5440 only says ERO for PCE->PCC and RRO for PCC->PCE. ### XXX Pending The idea here is that ERO contains the path as pushed by the PCE. It may contain loose hops, which the PCC can expand as it sees fit. The RRO contains the effective path the LSP is currently taking, e.g. any loose hops are resolved. Since a backup PCE is not required to share state with the primary PCE, and there is no way to derive ERO from RRO, the PCEP session needs to communicate both, so a backup PCE can pick up the previous PCE's policy decision as well as the current LSP path. [JM] The current I-D addresses my concerns on this, thanks for the clarification.   - The behavior associated to the resource limit per PCC rather looks like a Notifcation than an Error (e.g., in RFC 5440, cancelling a set of pending requests relies on PCNtf). Please consider the use of Notification instead of Error here. ### XXX Pending Current wording is based on the assumption that the PCE has to have a consistent point-in-time view of the PCC's state. In this regard a PCRpt of a new LSP which exceeds PCE implementation-internal limit on the number of LSPs it supports would break that assumption, hence we chose PCErr. This makes it consistent with what would happen if that LSP is reported during initial state resynchronization. [JM] Please note that the current I-D uses "PCNtf", and I am fine with that resolution. I was not questioning the expected behavior, which must remain. I was just suggesting the expected type of message to be consistent with RFC 5440: the PCC has not made anything wrong, it is informed that the PCE no more accepts its reports similarly to the way a PCE is able to tell about overload or cancel some requests. - It would be nice to elaborate on the reason why the SYMBOLIC-PATH-NAME MUST be included and not SHOULD. - I do not see why SYMBOLIC-PATH-NAME may be included in SRP Object: defining the LSP Object as its single place seems enough and much simpler. ### XXX  Pending The MUST is there to maintain a single global identifier for the LSP. PLSP-ID is then used as a shorthand. I do not recollect the exact reasoning as to why the TLV can be in SRP, as the placement and semantics of that TLV has changed quite a bit over the past couple of years. If I were to venture a guess, I think it was retrofitted to allow the PCE to update the symbolic path name. [JM] OK about the "MUST". About SYMBOLIC-PATH-NAME in SRP, please choose: either it is legacy and must be dropped (current version), or there is a reason and it must be documented in the I-D. Thanks, Robert Thank you, Julien

_______________________________________________
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce