Hi, This is another fly-by review as I just saw the new revision of the draft pop up. I think it is important and helpful that implementers of IETF protocol work get together to document their experiences with the technology, so thanks to the authors for their work.
However, I am concerned when implementers try to require other implementers to not make the same internal design mistakes that their friends may have made. We do not need to tell people how to write code, only how to ensure interoperability. This document seems to be a mixture of changes to the specifications (not all of which I agree are the right way of doing things), observations on how you might implement internal data structures or stores, and observations on how protocol exchanges work. I wonder how much of that needs to be in an IETF Standards Track document. I'm going to respond separately on https://mailarchive.ietf.org/arch/msg/pce/skstm9VsYiHOpjlQkQVBuBxE1xg/ because there seems to be some overlap with my previous concern and the discussion there. More detailed comments below. Thanks for your work implementing and testing PCE. Best, Adrian == Significant == 3.1 LSP-DB contains two types of objects: LSPs and Tunnels. An LSP is identified by the LSP-IDENTIFIERS TLV. A Tunnel is identified by the PLSP-ID in the LSP object and/or the SYMBOLIC-NAME. See [RFC8231]. While I can appreciate that there is a need to correlate between LSPs in the LSP-DB to know whether they are associated and to determine the many different types of association (where sharing PLSP-ID is one such association), I baulk at the idea that the LSP-DB contains Tunnels. Why would it contain tunnels? But more to the point, what is this section actually telling a PCE implementation? Is there some requirement that PCEs hold specific format information? --- 3.2 Both PCE and PCC maintain their separate copies of the LSP-DB. The PCE LSP-DB is only modified by PCRpt messages, no other PCEP message may modify the PCE LSP-DB. The PCC LSP-DB is built from actual forwarding state that PCC has installed. PCC uses PCRpt messages to synchronize its local LSP-DB to the PCE. Why must the PCC maintain a copy of the LSP-DB? Especially after the PCRpt has been sent? I mean, it may do (especially if the PCC is the head end of a soft state protocol, or if the PCC is responsible for imposing the path information on every packet), but why is this a PCE requirement? --- 3.2 The PCE MUST always act on the latest state of the PCE LSP DB. Why "MUST"? - Is this a quote from somewhere or are you updating a specification? - Why is the PCE not at liberty to perform "crooked" path computations? - In what way is this an interop issue? --- 3.2 The LSP-DB on both the PCC and the PCE only stores the actual state in the network, it does not store the desired state. I agree that the PCE's LSP DB needs to store the actual state in the network. Why, however, do you mandate that it cannot also store the desired state? In the case of multiple simultaneous transitions of related LSPs it could be very useful to know both the current and intended states. And anyway, why is this an interop issue? --- 3.3.1 While I can accept that the change you describe reflects what has been implemented (and it is very important to document when we have multiple interoperable implementations), it seems to me that you could equally have used "PCReq with delegation" whereby a PCC would ask for a computation and delegate in one go. This would make the PCRpt always say what is in the network, and the PCReq always request an explicit action from the PCE. (I suppose I'm a little grumpy that you have "Hey, this is what we built, so standardise it" approach to consensus.) I would note that under previous definitions, a PCE receiving a PCRpt with delegation is not obliged to actually do anything! It can, of course, issue an update in a new PCUpd whenever it wants, but it is not obliged to unless it thinks it is the right time. This, I think, is what lies behind the text you quote from 8231. You are changing this by saying that a PCRpt with no ERO or an empty ERO has an explicit meaning equivalent to explicitly requesting PCE action. Before closing on this (and acknowledging that I prefer my proposal :-) I'd like you to think about whether a PCRpt with an empty ERO might sometimes be totally valid in the <intended-path> of a PCRpt. For example, if a PCC (for whatever reason) launched a path into the network without an ERO (say it's using RSVP-TE) and learns the installed path from the RRO, then wouldn't the subsequent delegation look like "empty ERO with full RRO" (i.e., empty <intended-path> with full <actual-path>)? In that case, are you actually asking the PCE to immediately compute a path and issue PCUpd? Maybe you are. Maybe you want the PCE to pin the path that has previously be soft-computed in the network. But will it pick that one (looking at the RRO) or will it look at the empty ERO and make its own computation? Maybe, reading further through the section, you intend the Oper-Down flag to have more significance. I note here that your example shows an LSP getting into the PCE's LSP-DB in a way that shows its presence without its path. This is not entirely contradictory to your previous statement about not showing intended paths, but it is ambiguous with your statement that the LSP-DB only stores the actual state. --- What is 3.4 adding to the discussion? --- I'd also ask what 3.5 adds. But I note that 3.5 has a stage in it (old LSP up, new LSP down) that doesn't appear in 3.4. That makes the two cases seem very different when, in fact, they only differ in the last steps: - new goes up old is removed - new is removed == Minor == Title The title claims "Operational Clarification" and I see where you are coming from. But the Abstract reports this as clarifications to the protocol based on interop, and that suggests "Implementation Clarifications". It would be good to get this clear and aligned. --- Abstract This document updates, simplifies and clarifies certain aspects of the PCEP protocol. The use of "updates" is a trigger! The implication is that you are proposing updates to existing RFCs. You are totally allowed to do that, but you need to set them out both in the metadata, and in the Abstract and Introduction. It looks like you mean to at least update 8231. --- Section 3 Alternatively, we could rename LSP to "Instance". Well, you don't appear to have followed this route. Except for in 3.1 where you say an LSP is an instance of a tunnel. It's true. So much of the PCE work (even 7399) pre-dates segment routing. But you could take the approach of 8664, or more precisely, you could borrow from draft-ietf-pce-segment-routing-ipv6 which has: Further, note that the term LSP used in the PCEP specifications, would be equivalent to a SRv6 Path (represented as a list of SRv6 segments) in the context of supporting SRv6 in PCEP. Then you can move on, and not get tied in knots. --- What is 3.4 adding to the discussion? Does it in any way affect interoperability? --- Section 4 starts with: PCEP Association is a group of zero or more LSPs. What exactly is "an association of zero LSPs". I can understand why you might have an association group with just one LSP while you are in the process of adding more LSPs. But I can't see what it means to have an association of zero LSPs. Obviously, an implementation may decide to allocate association IDs for particular purposes, but not have populated them with LSPs. But that is an implementation thing and nothing to do with the protocol. Indeed, there is no way to communicate an association ID in the absence of a PCEP message that communicates about an LSP. --- AS idnits points out, Section 4.1 has "NOT REQUIRED" which is not a BCP 14 phrase. You could... OLD PCC updates the first LSP, the PCC is NOT REQUIRED to send the ASSOCIATION object in this PCRpt, since the LSP is already in the Association. NEW PCC updates the first LSP, it is OPTIONAL for the PCC to send the ASSOCIATION object in this PCRpt, since the LSP is already in the Association. END However, be careful! Are you defining new behavior here or quoting an existing behavior? If it's new then you need to be clear what you're updating. If it is neither (i.e. a clarification of what is already written, or even just a restatement), then maybe you don't need BCP 14 language. Personally, I find 6.3.1 of 8697 pretty clear: When an LSP is first reported to the PCE, the PCRpt message MUST include all the association groups that it belongs to. Any subsequent PCRpt message SHOULD include only the associations that are being modified or removed. Why was there any need to comment on this in your document? --- At the end of 4.1 you have... PCC decides to remove the first LSP from the Association, but not delete the LSP itself. PCC sends PCRpt(R-FLAG=0, PLSP-ID=100, LSP- ID=1, ASSO_PARAM=A, ASSO_R_FLAG=1). The PCE ASSO DB is now empty. +---------------------------------------------------------------+ | ASSO | LSP | +-----------------+---------------------------------------------+ | ASSO_PARAM=A | | +---------------------------------------------------------------+ Figure 13: Content of PCE ASSO DB But what you've shown in Figure 13 is not an empty ASSO DB. It is an ASSO DB with an Association in it, where that Association contains no LSPs. --- Section 4.2 begins with Below, we give an example to illustrate how a Tunnel goes through MBB and switches from Association A to Association B. But where does it say that tunnels are associated? 8697 is quite clear that associations are of LSPs (indeed, it gives the example of associating multiple LSPs from the same tunnel). And your section 4 begins with "PCEP Association is a group of zero or more LSPs." (Although, refer back to my comment on that point!) Having read the whole of 4.2, I wonder what it adds to interop or clarification of the specs. --- 5. For any PCEP object that does not have an explicit removal flag, the absence of that object indicates removal of the constraint specified by that object. For clarity, you mean... For any PCEP object that specifies a path computation constraint and that does not have a defined explicit removal flag, the absence of that entire object on a repeat or follow-up message indicates removal of the constraint previously specified by that object. In the text that follows, you might replace "state-report" with the actual PCEP message you are referring to. However, isn't this a bit confusing wrt delegation? You appear to be saying that a PCC that reports a set of computation constraints on a PCRpt could change them on a later PCRpt. In the case where there is no delegation, the computation constraints are not of any great value to the PCE and, while it might record them in an LSP-DB, I don't see what it would use them for. In the case where the first PCRpt delegates the LSP, the second appears to be trying to re-assume control and change the parameters without revoking delegation. So, what I think you are really saying is that "As well as reporting any state change in the network on a PCRpt message, a PCC may also change the parameters of a delegated LSP. For example, it may remove or modify the computation constraints that it wishes the PCE to apply as it computes any updated paths in the future. For any PCEP object that specifies a path computation constraint and that does not have a defined explicit removal flag, the absence of that entire object on a repeat or follow-up message indicates removal of the constraint previously specified by that object." I'd agree with you that 8231 is not crystal clear on that point, although it seemed a bit obvious to me. Making a clarifying statement could be helpful in an explicit update to that RFC. --- Section 6 has: the values of an SR-ERO/SRv6-ERO and SR-RRO/SRv6-RRO (respectively) are in practice the same I know why you say that, but you should explain in the text with "because there is no feedback mechanism in the forwarding or data plane that can collect and report the recorded route of a Segment Routing path when there were loose hops in the explicit route specified." That certainly seems to be where you are going with the final paragraph of that section. The following applies to SR-TE only. If both ERO and RRO are present for the same LSP, it SHOULD be interpreted as the RRO being the actual path the LSP is taking but MAY interpret only the ERO as the actual path. In the absence of RRO a PCE MUST interpret the ERO as the actual path for the LSP. Until SR-TE introduces some form of signaling similar to RSVP-TE, the use of RRO is discouraged for SR-TE LSPs. Of course, you only need to make this clarification for SR-MPLS-TE because [I-D.ietf-pce-segment-routing-ipv6] is a work in progress and you can make the text of that document precise. That leaves us with: - What about management tools and OAM that can report a more precise path than in the original ERO? - What does "the use of RRO is discouraged" mean? == Nits == You might move the Requirements Language away from the document header and place it as Section 2.1. The RFC Editor would likely do this anyway, but you can tidy the document yourselves. --- Throughout you have "PCEP protocol" which perhaps could be abbreviated as PCEPP so that you could later write "PCEPP protocol" :-) --- Section 2 is fine, but you are not defining these terms here, I think. Thus you should include citations for each term. For example, PCC and PCE are clearly lifted from 4655. --- Section 2 is missing "LSP-DB" --- Not sure whether the term "ERO object" etc. is an over-statement. I recall having this all the time as we wrote 5440, and we solved it by either expanding when we wanted to use the word object, or sticking with the simple abbreviation. _______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce