Proportional Integral Controller Enhanced (PIE): A Lightweight Control Scheme to Address the Bufferbloat Problem
draft-ietf-aqm-pie-10

In section 5 and its children: Please keep in mind that "SHOULD" does not mean quite the same thing as "optional".

It would be nice to see some text about the nature of the "experiment". That is, why is this experimental? Do you expect to promote this to a standard in the future? (The shepherd's report speaks of this;  the draft should, too.)

- Support Ben's comment:

It would be nice to see some text about the nature of the "experiment". 
That is, why is this experimental? Do you expect to promote this to a 
standard in the future? (The shepherd's report speaks of this; the draft 
should, too)

ex: https://tools.ietf.org/html/rfc6614#section-1.3

- Minor personal preference: delay variation instead of jitter.
See https://tools.ietf.org/html/rfc5481#section-1 for a justification.
Btw, same comment for draft-ietf-aqm-eval-guidelines-11, which I forgot to mention.

- Section 1
RFC2309 is obsolete:

   RFC 2309[RFC2309]
   strongly recommends the adoption of AQM schemes in the network to
   improve the performance of the Internet. 

Not sure why [RFC2309] is different than [IETF-AQM], which is now RFC7567. So maybe using [RFC2309] was used on purpose.


- it seems that you sometimes interchange queueing latency, latency, delay, queue delay
For an example, review section 3 and section 4 first paragraph.
You should really use consistent terms, for example queueing latency, throughout the document.

OLD:

As illustrated in Fig. 1, PIE conceptually comprises three simple MUST
components: a) random dropping at enqueueing; b) periodic drop
probability update; c) latency calculation. When a packet arrives, a
random decision is made regarding whether to drop the packet. The drop
probability is updated periodically based on how far the current delay
is away from the target and whether the queueing delay is currently
trending up or down. The queueing delay can be obtained using direct
measurements or using estimations calculated from the queue length and
the dequeue rate.

NEW:
As illustrated in Fig. 1, PIE conceptually comprises three simple MUST
components: a) random dropping at enqueueing; b) periodic drop
probability update; c) queueing latency calculation. When a packet arrives, a
random decision is made regarding whether to drop the packet. The drop
probability is updated periodically based on how far the current queueing latency
is away from the target and whether the queueing latency is currently
trending up or down. The queueing latency can be obtained using direct
measurements or using estimations calculated from the queue length and
the dequeue rate.


NEW:

        Random Drop
             /               --------------
     -------/  -------------->    | | | | | -------------->
            /|\                   | | | | |
             |               --------------
             |             Queue Buffer   \
             |                     |       \
             |                     |queue   \
             |                     |length   \
             |                     |          \
             |                    \|/         \/
             |          -----------------    -------------------
             |          |     Drop      |    | Queueing        |
             -----<-----|  Probability  |<---| Latency         |
                        |  Calculation  |    | Calculation     |
                        -----------------    -------------------



- terminology: dequeue_rate or departure?
    Section 4.2 =>"dequeue rate"
    Section 4.3

        current_qdelay = queue_.byte_length()/dequeue_rate;

    Section 5.2 Departure Rate Estimation
    Section 5.2 typo "Upon a packet deque:"  (this one could fine if you speak about the
                                                                      deque(Packet packet) function, but that's not clear)
Again, be consistent across the entire doc.

- editorial: missing reference links

   CBQ has been a standard feature in most network devices today[CBQ].

    The controller parameters, alpha and beta(in the unit of hz) are
    designed using feedback loop analysis where TCP's behaviors are modeled
    using the results from well-studied prior art[TCP-Models].


- editorial:

   This draft separates the PIE design into the basic elements that are
   MUST to be implemented and optional SHOULD/MAY enhancement elements.

NEW:
   This draft separates the PIE design into the basic elements that 
   MUST to be implemented and optional SHOULD/MAY enhancement elements.




Below is Mahesh's OPS DIR review:
I have reviewed the following document as part of the Operational directorate’s ongoing effort to review all IETF documents being processed by the IESG.  These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in last call may be included in AD reviews during the IESG review.  Document editors and WG chairs should treat these comments just like any other last call comments.

Document reviewed:  draft-ietf-aqm-pie-06

Summary: Ready with nits.

    The document is on an Experimental Track. Perhaps because of it there is little discussion of either operational or management considerations. If the document progresses towards a standardization effort, the following are (only a partial list of) suggestions or areas that can be explored.
    From a deployment consideration perspective
        The document could talk about how the technology is going to be deployed or managed.
        It would help if the authors documented any scaling issues that they have run into.
        How would this AQM co-exist with other methods?
    From an installation and initial setup perspective
        The document could talk about how PIE would be configured. For the parameters that are configurable, what are the ranges and what is the default value.
        Are the parameters going to be configured by configuration manager, or will the device pull the configuration from a configuration server?
    From a network operations perspective
        The document could talk about the impact of PIE on existing networks, with or without other AQM.
        While PIE attempts to reduce latency and jitter, what is the impact on throughput when it is deployed, specially for non-real time traffic?
    From a verifying correct operations perspective
        How does one verify that the network is behaving as expected once PIE is applied?
        Other than latency and jitter, are there any other parameters of interest?
        Will the test in itself have an impact on the network or the protocol?
    From a management interoperability perspective
        The document can talk about any standard YANG models that might need to be enhanced or developed for manageability across vendors.
    From a fault or threshold conditions perspective
        The document could talk about any notifications that need to be reported for any events generated because of the algorithm. Should the events be polled or could they be pushed? Would notifications have to be throttled?


A run of idnits has revealed a few issues that will need to addressed.

tmp/draft-ietf-aqm-pie-06.txt:
tmp/draft-ietf-aqm-pie-06.txt(17): Found control character TAB in position 1.
tmp/draft-ietf-aqm-pie-06.txt(383): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(469): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(471): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(941): Line is too long: the offending characters are '.'
tmp/draft-ietf-aqm-pie-06.txt(978): Found control character TAB in position 8.
tmp/draft-ietf-aqm-pie-06.txt(982): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(1099): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1101): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1112): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1126): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1128): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1182): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(1187): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(1189): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(1253): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1254): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1257): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1258): Found control character TAB in position 13.
tmp/draft-ietf-aqm-pie-06.txt(1259): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1260): Found control character TAB in position 13.
tmp/draft-ietf-aqm-pie-06.txt(1261): Found control character TAB in position 13.
tmp/draft-ietf-aqm-pie-06.txt(1263): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1306): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1325): Found control character TAB in position 7.
tmp/draft-ietf-aqm-pie-06.txt(1327): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1328): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1330): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1352): Found control character TAB in position 4.
tmp/draft-ietf-aqm-pie-06.txt(1386): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(1391): Found control character TAB in position 9.
tmp/draft-ietf-aqm-pie-06.txt(1393): Found control character TAB in position 9.


[Stuff deleted]

  Checking boilerplate required by RFC 5378 and the IETF Trust (see
  http://trustee.ietf.org/license-info):
  ----------------------------------------------------------------------------

     No issues found here.

  Checking nits according to http://www.ietf.org/id-info/1id-guidelines.txt:
  ----------------------------------------------------------------------------

     No issues found here.

  Checking nits according to http://www.ietf.org/id-info/checklist :
  ----------------------------------------------------------------------------

  ** There is 1 instance of too long lines in the document, the longest one
     being 1 character in excess of 72.

  ** There are 31 instances of lines with control characters in the document.


  Miscellaneous warnings:
  ----------------------------------------------------------------------------

  == The copyright year in the IETF Trust and authors Copyright Line does not
     match the current year


  Checking references for intended status: Experimental
  ----------------------------------------------------------------------------

  == Missing Reference: 'IETF-AQM' is mentioned on line 748, but not
     defined
     '[IETF-AQM]     Baker, F. and Fairhurst, G., "IETF Recommendations...'

  == Missing Reference: 'CoDel' is mentioned on line 730, but not
     defined
     '[CoDel]        Nichols, K., Jacobson, V., "Controlling Queue Dela...'

  == Missing Reference: 'CBQ' is mentioned on line 727, but not
     defined
'[CBQ]          Cisco White Paper,...'

  == Missing Reference: 'FQ-Implement' is mentioned on line 739, but not
     defined
     '[FQ-Implement] Baker, F. and Pan, R. "On Queueing, Marking and...'

  == Missing Reference: 'DOCSIS-PIE' is mentioned on line 736, but not
     defined
     '[DOCSIS-PIE]   White, G. and Pan, R., "A PIE-Based AQM for DOCSIS...'

  == Missing Reference: 'HPSR-PIE' is mentioned on line 742, but not
     defined
     '[HPSR-PIE]     Pan, R., Natarajan, P. Piglione, C., Prabhu, M.S.,...'

  == Missing Reference: 'PI' is mentioned on line 755, but not defined
     '[PI]	          Hollot, C.V., Misra, V., Towsley, D. and Gong, W.,...'

  == Missing Reference: 'QCN' is mentioned on line 759, but not
     defined
'[QCN]	  "Data Center Bridging - Congestion Notification",...'

  == Missing Reference: 'TCP-Models' is mentioned on line 762, but not
     defined
     '[TCP-Models]   Misra, V., Gong, W., and Towsley, D., "Fluid-base...'

  == Missing Reference: 'IETF-ECN' is mentioned on line 751, but not
     defined
'[IETF-ECN]     Briscoe, B. Kaippallimalil, J and Phaler, P.,...'

  -- Obsolete informational reference (is this intentional?): RFC 2309
     (Obsoleted by RFC 7567)


     Summary: 2 errors (**), 0 flaws (~~), 11 warnings (==), 2 comments (--).

Mahesh Jethanandani

performed the opsdir review

- Thanks to the IPR declaring folks for not using the term
"standard" in their declaration. (I first looked at one of
the earlier ones [1] that did say that, and I was about to
start my usual whine when I noticed the most recent one [2]
doesn't have that problem:-)

   [1] https://datatracker.ietf.org/ipr/2123/
   [2] https://datatracker.ietf.org/ipr/2540/

- Seems odd to mention one specific vendor in the security
considerations section. Why is that important to include?

Section 4.1:
QDELAY_REF is used here without definition. Suggest something like:

s/is smaller than half of QDELAY_REF/is smaller than half of the
target delay value (QDELAY_REF)/

Section 4.2:

Not sure what this piece of pseudo-code intends to achieve. Isn't this a no-op?

        } else {
            p = p;
        }

Section 5.3:

What does the following text intend to say? Do you mean when it is turned off?

"When PIE is optionally turned on and off, ..."

Section 5.4:

The forced drop at 8.5/PIE->drop_prob_ packets is not consistent
with the pseudo-code in page 24. The drop_early() does not the
drop packets if the queue is not congested. Either the
pseudo-code or text in this section needs to be adjusted.