Subject: [Pce] Shepherd review of draft-ietf-pce-stateful-pce-auto-bandwidth-08

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 18 Apr 2019 04:03:42 +0100

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

---

Abstract para 2
s/Automatic bandwidth/Automatic bandwidth adjustment/ ??

---

2.3
Down-Adjustment-Interval
s/lesser/less/

---

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.

---

3.
s/the PCC, the LSP that/the PCC, which LSPs/

---

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

---

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.)

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

---

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.

---

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.

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.

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/

---
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.

--

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?

-- 
---

5.2.1

You might explain that 604800 seconds is 7 days.

---

5.2.3
s/PCEP peer the/PCEP peer of the/

---

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?

---

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).

---

6.  Security Considerations

Please expand this section a little.  You should certainly include RFC8281
as reference. Also include more on TLS use.

--

8.3. AUTO-BANDWIDTH-ATTRIBUTES Sub-TLV

Consider making a **few** code points for experiments into this sub-TLV
range?

-

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
--

Nits

expand RBNF, LSPA

---

From: Rakesh Gandhi <[email-masked]>
Date: Tue, 23 Apr 2019 21:26:11 -0400

Thanks Adrian for the detailed review and suggestions.  Please see replies
inline with <RG>...

On 2019-04-17, 11:03 PM, "Adrian Farrel" <[email-masked]> wrote:

    Hello authors,

    I have some fairly minor review comments (below). Could you please
address

    these in a new revision.

    quickly as possible.

    Thanks,

    Adrian

    ---

<RG> Fixed.

    ---

    Abstract para 2

<RG> Added – “The automatic bandwidth feature”

    ---

    2.3

    Down-Adjustment-Interval

    s/lesser/less/

<RG> Fixed.

    ---

                                Adjustment-Internal

    Average Bandwidth.

<RG> Removed the Adjustment-Interval from the Max Average Bandwidth
definition. That should remove the circular dependency.

    ---

    3.

<RG> Fixed.

    ---

    4.1

    OLD

    NEW

       the LSP.

    END

<RG> Fixed.

    ---

    4.1 has...

       The bandwidth

<RG> Fixed.

    OLD

    NEW

       in the network.

    END

<RG> Fixed.

    ---

    4.2 has...

<RG> Updated. “by the PCC, when the PCC is the head-end node of the LSP.”

    ---

<RG> Added – “This section describes the Auto-Bandwidth feature in a
general way.”

    For example...

       The

<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”

    ---

    5.2

<RG> Added.

    ---

    Section 5.2

    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

    PCEP messages?

    --

<RG> Added.

    ---

    5.2.1

<RG> Added.

    ---

    5.2.3

<RG> Fixed.

    ---

    5.2.3

    threshold?

<RG> Added.

    ---

    5.2.3.1

    bandwidth is adjusted?

    No, you don't mean that!

    How about...

<RG> Updated.

    ---

    Please expand this section a little.  You should certainly include
RFC8281

<RG> Updated.

    --

    range?

<RG> Added.

    -

    is a sub-registry in the "PCEP TLV Type Indicators", which is not the
case.

    OLD:

    NEW:

       as "Unassigned".

    END

<RG> Updated.

    --

    Nits

    expand RBNF, LSPA

<RG> Fixed.

Thanks,

Rakesh