Thank you Jonathan. Those changes sufficiently address my concerns. Yours, Joel On 4/11/17 5:55 AM, Jonathan Hardwick wrote:
Hi Joel Many thanks for these comments. I'm picking up this thread and replying as PCE working group chair, as the authors are unavailable. I apologise for the delay. Please see my proposed resolutions inline below, marked with "Jon>" Best regards Jon -----Original Message----- From: Joel Halpern [mailto:jmh@xxxxxxxxxxxxxxx] Sent: 17 February 2017 03:38 To: gen-art@xxxxxxxx Cc: draft-ietf-pce-stateful-pce.all@xxxxxxxx; pce@xxxxxxxx; ietf@xxxxxxxx Subject: Review of draft-ietf-pce-stateful-pce-18 Reviewer: Joel Halpern Review result: Ready with Issues <snip> ==== Minor issues: At the end of section 5.4, the text talks about a PCE accepting status updates even if the stateful capability has not been negotiated. Which is fine. But as written, the text seems to say that doing so means that the PCE will be able to "build and maintain an up to date view of the state of the PCC's LSPs". However, if the capability has not been negotiated, I do not see how the PCE can count on getting full and timely state reports. Trying to infer why a PCC is sending such a report in the absence of the agreement seems problematic. Jon> I agree. I propose that we delete this sentence: " Note that even if the update capability has not been advertised, a PCE can still accept LSP Status Reports from a PCC and build and maintain an up to date view of the state of the PCC's LSPs. " ==== This comment may be a misunderstanding or mis-expectation on my part. I would have expected one of the ways o using an active PCE is to have the PCE decide (under suitable circumstances) that an LSP is needed between two PCCs. As far as I can tell, the text in section 5.8.2 and 5.8.3 prohibits that. A PCE is only allowed to send an LSP Update Request (in a PCUpd message) for an LSP that has been delegated to it. At that point I thought that a PCC could delegate a block of unsetup LSPs to the PCE. But then looking at 5.8.2, the text states that for each delegation, the PCC must request an initial path. That seems to prohibit delegating a block of LSPs for future updates. Is the intention to prohibit the driving of LSP creation from the PCE? Jon> Yes, the intention is that the PCE does not drive creation of the LSPs. That behaviour is allowed but is described by a separate draft (draft-ietf-pce-pce-initiated-lsp). I propose to clarify this in the introduction to the draft, so that future readers don't get caught by the same mis-expectation. We'll add the following text to section 1, and add an informative reference to draft-ietf-pce-pce-initiated-lsp. OK? NEW The extensions that this document describes do not permit the PCE to drive creation of an LSP. The companion document [I-D. ietf-pce-pce-initiated-lsp] specifies PCE-initiated LSP creation. ==== I have looked but I can not find the text explaining the significance and use of the Symbolic path name. It is mandated by the draft. There seems to be an implicit assumption taht it is needed by the PCE. If the explanation of how or why it is needed is not present, it should be. Jon> Agreed. Several of our reviewers have picked up on this. I think we should clarify this in section 7.3.2 "Symbolic Path Name TLV". OLD Each LSP (path) MUST have a symbolic name that is unique in the PCC. This symbolic path name MUST remain constant throughout an LSP's lifetime, which may span across multiple consecutive PCEP sessions and/or PCC restarts. The symbolic path name MAY be specified by an operator in a PCC's configuration. If the operator does not specify a unique symbolic name for a path, the PCC MUST auto-generate one. The SYMBOLIC-PATH-NAME TLV MUST be included in the LSP object in the LSP State Report (PCRpt) message when during a given PCEP session an LSP is first reported to a PCE. A PCC sends to a PCE the first LSP State Report either during State Synchronization, or when a new LSP is configured at the PCC. The symbolic path name MAY be included in the LSP object in subsequent LSP State Reports for the LSP. <snip> Symbolic Path Name (variable): symbolic name for the LSP, unique in the PCC. NEW Each LSP MUST have a symbolic path name that is unique in the PCC. The symbolic path name is a human-readable string that identifies an LSP in the network. The symbolic path name MUST remain constant throughout an LSP's lifetime, which may span across multiple consecutive PCEP sessions and/or PCC restarts. The symbolic path name MAY be specified by an operator in a PCC's configuration. If the operator does not specify a unique symbolic name for an LSP, then the PCC MUST auto-generate one. The PCE uses the symbolic path name as a stable identifier for the LSP. If the PCEP session restarts, or the PCC restarts, or the PCC re-delegates the LSP to a different PCE, the symbolic path name for the LSP remains constant and can be used to correlate across the PCEP session instances. The other protocol identifiers for the LSP cannot reliably be used to identify the LSP across multiple PCEP sessions, for the following reasons. - The PLSP-ID is unique only within the scope of a single PCEP session. - The LSP-IDENTIFIERS TLV is only guaranteed to be present for LSPs that are signalled with RSVP-TE. The SYMBOLIC-PATH-NAME TLV MUST be included in the LSP object in the LSP State Report (PCRpt) message when during a given PCEP session an LSP is first reported to a PCE. A PCC sends to a PCE the first LSP State Report either during State Synchronization, or when a new LSP is configured at the PCC. The initial PCRpt creates a binding between the symbolic path name and the PLSP-ID for the LSP which lasts for the duration of the PCEP session. The PCC MAY omit the symbolic path name from subsequent LSP State Reports for that LSP on that PCEP session, and just give the PLSP-ID. <snip> Symbolic Path Name (variable): symbolic name for the LSP, unique in the PCC. It SHOULD be a string of printable ASCII characters and SHOULD be NULL-terminated. The Symbolic Path Name (including its NULL terminator) MUST be padded to 4-bytes alignment; the padding itself MUST NOT be included in the Length field. END NEW ==== Nits/editorial comments: Should the text on the S bit in section 7.3 (the LSP Object definition) note that it should be set to 0 on all messages sent by the PCE? Should that also be stated for the R bit? And the O bits? Jon> Yes. We will add a note to the description of these fields. ==== Section 9.2 seems very odd. It states that the IETF "SHOULD" do some additional work. I understand why teh work is needed. But this does not seem to match the RFC 2119 meaning of SHOULD as it does not apply to eitehr the implementor or the implementation. Jon> I agree - this is incorrect use of SHOULD. It also references the MIB without even mentioning the YANG, which is a symptom of this document's advanced age. We'll change this as follows (and add two informative references). NEW The PCEP YANG module [I-D.ietf-pce-pcep-yang] should include - advertised stateful capabilities and synchronization status per PCEP session - the delegation status of each configured LSP. The PCEP MIB [RFC7420] could also be updated to include this information. ====