Hi,
In Dublin, JP indicated that the authors of this I-D thought it was pretty much ready for working group last call, so here is my review as WG chair before we get into the business of a last call. I think there are sections still needing to be filled in. Cheers, Adrian === idnits (http://tools.ietf.org/tools/idnits/\) shows 1 error and 7 warnings all in the references sections. === The I-D is currently Standards Track, I am slightly nervous about this given the current implementaiton status and the fact that you are defining substantial new messages and procedures. Have you considered Experiemental status? Conversely, Iâm worried that this I-D defines the PCE-ID object that is used by some instances in BRPC. === Abstract s/where a domain is referred to as/where a domain referrs to/ s/PCE-based environments/PCE-based environments,/ s/states, statistics/states, and statistics/ === 1. Terminology Please re-order sections so that Section 1 is the Introduction. This will mean that you need to expand all of the acrinyjms in the Introduction. On the other hand... AS: Autonomous System. Not used in the document. LSR: Label Switching Router. Not used in the document. TED: Traffic Engineering Database. Not used in the document. === 2. Introduction s/where a domain is referred to as/where a domain referrs to/ s/states, statistics/states, and statistics/ This document specifies procedures and extensions to the Path Computation Element Protocol (PCEP) ([I-D.ietf-pce-pcep]) in order to monitor the path computation chain and gather various performance metrics. I think you need to introduce the concept of a âpath computation chainâ before talking about monitoring it. I would simply move this paragraph to after the next paragraph, but even then the term is not clearly explained. There is a good paragraph in section 3.1. s/The aim of this document is to specify/This document specifies/ s/metrics collection can be gathered/metrics can be gathered/ s/one or more PCE(s)/one or more PCEs/ s/collection of a PCE state metric(s)/collection of PCE state metrics/ === 3. Path Computation Monitoring messages s/to enforce a PCE/to require a PCE/ === 3.1. Path Computation Monitoring Request message (PCMonReq) You need to mention how this message will be handled by a âlegacyâ PCE that does not recognise the message. Also how it would be handled by a PCC. OLD Note that this set of objects is by all means not limitative and may be extended in further revision of this document. NEW Note that this set of objects may be extended in future. OLD Example 1: PCC1 requests to check the path computation chain should a path computation be requested for a specific TE LSP named T1. NEW Example 1: PCC1 requests to check the path computation chain that would be used should it request a path computation a specific TE LSP named T1. s/PCRep message also comprises a PROC-TIME/PCRep message also carries a PROC-TIME/ Example 3 s/contains a set of PCE-ID objects/contains a sequence of PCE-ID objects/ === 3.2. Path Monitoring Reply message (PCMonRep) If the Monitoring object is missing, the receiving PCE MUST send an error message to the requesting PCC. Which error code? === 4. Path Computation Monitoring Objects s/computation request./computation requests./ === 4.1. MONITORING Object The MONITORING object MUST be present within PCMonReq and PCMonRep messages ("out of band" monitoring requests) and MAY be carried within PCERep and PCReq messages ("in band" monitoring requests). There MUST be exactly once instance of the MONITORING object There is a slight contradiction between MAY and MUST. if more than one instance of the MONITORING object is present, the recipient MUST only process the first instance and ignore other instances. But now you have a contradiction with the MUST as well. Perhaps this should read... The MONITORING object MUST be present within PCMonReq and PCMonRep messages ("out of band" monitoring requests) and MAY be carried within PCERep and PCReq messages ("in band" monitoring requests). There SHOULD NOT be more than one instance of the MONITORING object: if more than one instance of the MONITORING object is present, the recipient MUST process the first instance and MUST ignore other instances. Please indent the bit numbering correctly +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Reserved | Flags |I|C|P|G|L| +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ Very unusual split. 10 bits reserved and 22 bits of flags. OLD +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | monitoring-id-number | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ NEW +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Monitoring-id-number | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ s/Flags: 18 bits/Flags: 22 bits/ The P bit MUST always be set in a PCMonRep message if also set in the corresponding PCMonReq message. No objection, but why? Doesnât the PROC-TIME object convey the same information? I note that the L and G bits are not defined in the PCMonRep. Anyway, you need to talk not just about PCMonReq/Rep, but the use in PCReq/Rep (cf C bit). OLD I (Incomplete) - 1 bit: the I bit MUST be set by a PCE that supports the PCMonReq message, which does not trigger any policy violation but that cannot provide the set of requested performance metrics for unspecified reasons. NEW I (Incomplete) - 1 bit: If a PCE supports a received PCMonReq message and that message does not trigger any policy violation, but the PCE cannot provide any of the set of requested performance metrics for unspecified reasons, the PCE MUST set the I bit. The I bit has no meaning in a request and SHOULD be ignored on receipt. Question... Monitoring-id-number (32 bits). The monitoring-id-number value combined with the source IP address of the PCC and the PCE address uniquely identify the monitoring request context. âthe PCE addressâ? The request is not specifically targeted at a PCE, or may include a list of PCEs. How does this work? Isnât the PCC ID and the monitoring-id-number sufficient? Later you say that the same monitoring-id-number can be used for requests sent to different PCEs, but this seems to be ambiguous for chains of PCEs. And you say âThe path computation monitoring reply is unambiguously identified by the IP source address of the replying PCEâ but the replying PCE might not be known by the source PCC. Question The monitoring-id- number MUST be incremented each time a new monitoring is sent to a PCE. Same problem as was raised by the IESG for PCEP. The requirement is just that this number allows disambiguation of active requests/responses. Why do you require âincrementâ and by what value should the increment be made? s/Conversely, different/Conversely, a different/ No optional TLVs are currently defined. So, why do we have TLVs? Would it be better to put some of the response objects into this object as TLVs? E.g., PROC-TIME, CONGESTION, etc. === 4.2. PCE-ID Object A set of PCE-ID objects may be inserted within a PCReq or a PCMonReq message to specify the PCE for which PCE state metrics are requested and in a PCMonRep or a PCRep message to record the IP address of the PCE reporting PCE state metrics or that was involved in the path computation chain. So... If I send a PCReq with a list of PCE-IDs because these are the PCEs I want to process the request, can I control the list of PCEs that supply the monitoring information? For example, I way want the processing time form just the last PCE of a chain of PCEs. Please align the bit numbers on the figures correctly. A PCE MUST use the same IP address as the address used in the PCE- ADDRESS sub-TLV defined in [RFC5088] and [RFC5089] should a dynamic discovery mechanism be used for PCE discovery. Should that read âA PCC MUST ...â ? After all, the PCE has no control over the PCE- ID supplied in a request. === 4.3. PROC-TIME Object The PROC-TIME object MUST be present within a PCMonRep or a PCRep message if the P bit of the MONITORING object carried within the corresponding PCMonReq or PCReq message is set. âMUSTâ? Compare with the I bit. Consider local policy that says âI am not going to answer thatâ s/the PROC-TIME object always report/the PROC-TIME object always reports/ s/thus the E bits/thus the E bit/ Please align the bit numbers correctly. +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Reserved | Flags | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Processing-time |E| +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ Wouldnât it be more sensible to move the E bit into the Flags field so that all time fields are the same format? Current-processing-time: This field indicates in milliseconds the The field is called âProcessing-timeâ in the figure. === 4.4. CONGESTION Object C (Congestion) - 1 bit: when set, this indicates that PCE is congested, in which case the congestion duration may be non nul. When cleared this indicates that the PCE is not congested. Congestion duration - 16 bits: This field indicates in seconds the estimated congestion duration. Confused! What is the meaning of C-bit set, Congestion Duration = 0? === 4.5. TIMESTAMP Object A TIMESTAMP object will be specified in a further revision of this document Will you do this before WG last call, or will you delete this section? === 5. Multi-destination monitoring Capitilise the seciton heading, please. In a further revision of this document, a new object will be specified allowing a PCC or a user to gather PCE state metrics for a set of destinations using a single PCMonReq message. Need to resolve this TBD. === 7. Elements of procedure I bit processing: as indicated in section Section 4.1, the I bit MUST be set by a PCE that supports the PCMonReq message, which does not trigger any policy violation but that cannot provide the set of required performance metrics for unspecified reasons. Once set, the I bit MUST NOT be changed by a receiving PCE. Revise for clarity as proposed for section 4.1 Reception of a PCMonReq message: upon receiving a PCMonReq message: 1) If the PCE does not support the PCMonReq message, the PCE MUST send a PCErr message with Error-type=14 and Error-value=1. Wait a moment. You are defining a new error code to handle non-support of a new feature. Surely you should use an existing error code as see in the PCEP spec. For example, error-type=2 === 8. Manageability Considerations To be addressed in a further revision of this document. Please resolve this TBD === 9. To be considered in a further revision of this document It might be desirable to modify the format of the PCMonReq and PCMonRep messages to support the bundling of multiple performance metrics collection for a set of TE LSPs. Please resolve this TBD === 10. IANA Considerations Need to give better references to the registries defined in the PCEP I-D. === 11. Security Considerations To be addressed in a further revision of this document. Hmmm? _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
