Thanks Adrian for the detailed review and suggestions. Please see replies inline with <RG>...
On 2019-04-17, 11:03 PM, "Adrian Farrel" <[email protected]> wrote: Hello authors, Sorry about the clunk as this draft shifted between shepherd. I have some fairly minor review comments (below). Could you please address these in a new revision. Meanwhile, I will start on the shepherd write-up ready to move ahead as quickly as possible. Thanks, Adrian --- Odd leading space on 4th line of Abstract <RG> Fixed. --- Abstract para 2 s/Automatic bandwidth/Automatic bandwidth adjustment/ ?? <RG> Added – “The automatic bandwidth feature” --- 2.3 Down-Adjustment-Interval s/lesser/less/ <RG> Fixed. --- I found the definition of Maximum Average Bandwidth and the definitions of Up/Down-Adjustment-Interval to be circular. Unless, perhaps, the definition of Adjustment-Interval is missing. That is, Maximum Average Bandwidth is defined as max {Bandwidth-Sample(i)} for each sample interval i in the Adjustment-Internal But Up/Down-Adjustment-Interval both appear to be dependent on Maximum Average Bandwidth. <RG> Removed the Adjustment-Interval from the Max Average Bandwidth definition. That should remove the circular dependency. --- 3. s/the PCC, the LSP that/the PCC, which LSPs/ <RG> Fixed. --- 4.1 OLD Auto-Bandwidth feature allows automatic and dynamic adjustment of the reserved bandwidth of an LSP over time, i.e. without network operator intervention to accommodate the varying traffic demand of the LSP. NEW The Auto-Bandwidth feature allows automatic and dynamic adjustment of the reserved bandwidth of an LSP over time (i.e., without network operator intervention) to accommodate the varying traffic demand of the LSP. END <RG> Fixed. --- 4.1 has... The bandwidth adjustment uses the make-before-break (MBB) signaling method so that there is no disruption to the traffic flow carried by the LSP. I think this should be... Bandwidth adjustment must not cause disruption to the traffic flow carried by the LSP. One way to achieve this is to use the make- before-break (MBB) signaling method. This is the softest way I can think of saying what you don't want to say which is that RSVP-TE signaling supports in-place bandwidth adjustmnt simply by sending a new Path message. (Noting that failure to make the adjustment can be seen either in a non-fatal PathErr or in a Resv with unchanged bandwidth.) <RG> Fixed. Similarly in 4.3 maybe... OLD It should be noted that any bandwidth change requires re-signaling of an LSP in a make-before-break fashion, which can further trigger preemption of lower priority LSPs in the network. NEW It should be noted that any bandwidth change requires re-signaling of an LSP, which can further trigger preemption of lower priority LSPs in the network. END <RG> Fixed. --- 4.2 has... When the Auto-Bandwidth feature is enabled, the measured traffic rate is periodically sampled at each Sample-Interval (which can be configured by an operator and the default value as 5 minutes) by the PCC which is the head-end node of the LSP. As you know, in the PCE architecture, the PCC is not necessarily the head-end LSR. You need either to restrict this function to only operating when the PCC *is* the head-end LSR, or you need to re-word slightly. Either works for me. <RG> Updated. “by the PCC, when the PCC is the head-end node of the LSP.” --- I wonder whether you intend that Section 4 (in particular Section 4.2) is normative in this document. It could be that you are just describing the procedures in a general way - in which case a one line statement of that would address my concern. Or it could be that you intend to define how auto-bandwidth adjustment must be implemented. That is, the document claims to describe changes to PCEP to support auto-bandwidth, but this section appears to be describing how auto- bandwidth must be implemented, and I don't think I agree that the description is completely wise. <RG> Added – “This section describes the Auto-Bandwidth feature in a general way.” For example... The PCC, in-charge of calculating the bandwidth to be adjusted, will adjust the bandwidth of the LSP to the highest traffic rate sample (MaxAvgBw) amongst the set of bandwidth samples collected over the adjustment-interval period (in the Up or Down direction). ...means that a single spike in a potentially very small window over a potentially very large period (defaults are 5 minutes in 24 hours) could see a readjustment of the LSP's bandwidth. But such a spike could easily be a freak and it seems unwise to take bandwidth out of the network for it without allowing the application of operator policy. <RG> Reworded the sentence. “The PCC, in-charge of calculating the bandwidth to be adjusted, can decide to adjust the bandwidth of the LSP to the highest traffic rate sample depending on the operator policy” More generally, if this document is defining auto-bandwidth as an MPLS-TE function, it belongs in TEAS (or possibly MPLS), while if it is defining PCEP extensions, it is fine where it is. --- 5.2 s/PCEP peer the/PCEP peer of the/ <RG> Added. --- Section 5.2 There are various MUSTs and MUST NOTs for the attribute values, there is a need to include a description of the error processing when these conditions are not met. <RG> Added – “ The adjustment-interval parameter MUST NOT be less than the sample-interval, otherwise the Sub-TLV MUST be ignored and the previous value is maintained.” -- Section 5.2 Since the AUTO-BANDWIDTH-ATTRIBUTES TLV is a MUST for all LSPs with Auto-BW feature, do we want to encode all the sub-TLVs every-time in all PCEP messages? Instead, could we say that sub-TLV is only included if there is a change between the last information and now? The default values for missing sub-TLVs apply for the first PCEP message for the LSP? -- <RG> Added. --- 5.2.1 You might explain that 604800 seconds is 7 days. <RG> Added. --- 5.2.3 s/PCEP peer the/PCEP peer of the/ <RG> Fixed. --- 5.2.3 Is it worth noting that regardless of how the threshold are set, the adjustment will not be made until at least one sample-interval simply because no sample will be made on which to base a comparison with a threshold? <RG> Added. --- 5.2.3.1 If the difference between the current MaxAvgBw and the current bandwidth reservation is greater than or less than or equal to the threshold value, the LSP bandwidth is adjusted to the current bandwidth demand (MaxAvgBw). "greater than or less than or equal to" You mean regardless of the setting of this threshold value, the LSP bandwidth is adjusted? No, you don't mean that! How about... If the modulus of difference between the current MaxAvgBw and the current bandwidth reservation is greater than or equal to the threshold value, the LSP bandwidth is adjusted to the current bandwidth demand (MaxAvgBw). <RG> Updated. --- 6. Security Considerations Please expand this section a little. You should certainly include RFC8281 as reference. Also include more on TLS use. <RG> Updated. -- 8.3. AUTO-BANDWIDTH-ATTRIBUTES Sub-TLV Consider making a **few** code points for experiments into this sub-TLV range? <RG> Added. - It says that "AUTO-BANDWIDTH-ATTRIBUTES Sub-TLV Types" is a sub-registry in the "PCEP TLV Type Indicators", which is not the case. OLD: This document specifies the AUTO-BANDWIDTH-ATTRIBUTES Sub-TLVs. IANA is requested to create an "AUTO-BANDWIDTH-ATTRIBUTES Sub-TLV Types" sub-registry in the "PCEP TLV Type Indicators" for the sub-TLVs carried in the AUTO-BANDWIDTH-ATTRIBUTES TLV. New sub-TLV are assigned by Standards Action [RFC8126]. NEW: This document specifies the AUTO-BANDWIDTH-ATTRIBUTES Sub-TLVs. IANA is requested to create an "AUTO-BANDWIDTH-ATTRIBUTES Sub-TLV Types" sub-registry within the "Path Computation Element Protocol (PCEP) Numbers" registry to manage the type indicator space for sub-TLVs of the AUTO-BANDWIDTH-ATTRIBUTES TLV. New sub-TLV are assigned by Standards Action [RFC8126]. The valid range of values in the registry is 0-65535. IANA is requested to initialize the registry with the following values. All other values in the registry should be marked as "Unassigned". END <RG> Updated. -- Nits expand RBNF, LSPA <RG> Fixed. Thanks, Rakesh
_______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
