Re: [aqm] I-D Action: draft-ietf-aqm-codel-03.txt

[Michael Menth <[e-mail]>]

Dear Polina, dear all,

Am 07.07.2016 um 13:49 schrieb Goltsman, Polina:
> Dear Andrew, Dear Wesley,
> 
> 
> First of all, sorry for the long waiting time. I still haven't processed
> "nits" but I have 2 major issues.
> 
> 
> *Issue 1: *This issue is still related to the reentering dropping state.
> 
> 
> As of the latest I-D, the "reentering dropping state"  appears in form
> of one sentence in the text and a small code block in the pseudo-code.
> 
> 
> The text is on page 13:
> 
>>Additional logic prevents re-
>>entering the dropping state too soon after exiting it and resumes the
>>dropping state at a recent control level, if one exists.
> 
> The comment to the pseudo-code on page 20:
> 
> 
>            // If min went above target close to when it last went
>            // below, assume that the drop rate that controlled the
>            // queue on the last cycle is a good starting point to
>            // control it now. ('drop_next' will be at most 'interval'
>            // later than the time of the last drop so 'now - drop_next'
>            // is a good approximation of the time from the last drop
>            // until now.) *Implementations vary slightly here; this is // the Linux version, which
> is more widely deployed and // tested.
> 
> *
> 
> 
> Or summarized: this issue is not addressed in text, which instead just
> mentions that the issue exists, there are several versions of this logic
> and here is one of them.
> 
> 
> As it reads now, this "reentering" should be a tiny insignificant part
> of the algorithm that (1) doesn't happen in "most" of the cases, and (2)
> when it does happen, it will only "slightly" affect "performance" (with
> proper definitions of "most", "slightly", and "performance").
> 
> 
> Is it really so? 
> 
> 
> Based on our evaluations, with pure CoDel (without FQ-CoDel),
> "reentering" is actually a common case. I think Dave and Toke should
> have more experimental results to answer this question. (I included Dave
> in CC)


We also studied this issue about one or two years ago. The algorithm
controlling the reentering of the dropping mode may have a significant
impact on control variables, drop rates, drop pattern depending on the
traffic. It can also impact utilization. Most interesting is probably
Fig. 6 on page 7 in
https://atlas.informatik.uni-tuebingen.de/~menth/papers/Menth16e.pdf

Kind regards,

Michael



> 
> 
> If it is so, I still think that the "common case" should be at the very
> least explained in text too.
> 
> 
> *Issue 2:* The "initial drop spacing" is inconsistent
> 
> 
> Regarding the "initial drop spacing" there are following lines in the
> document.
> 
> 
> between pages 11 and 12:
> 
>>and so the initial drop spacing 
>>SHOULD be set to the estimator's *interval plus twice the target*
>>(i.e., initial drop spacing = 1.1 * interval)
> 
> page 12:
> 
>>the initial next
>>drop spacing is intended to be long enough to give the endpoints time
>>to react to the single drop so SHOULD be set to a value *of 1.1 times >the interval*
> 
> page 16:
> 
> 
>>and the initial drop
>>spacing is also set to *interval*.
> 
> and it is also set to*one interval* in pseudo-code, which I believe is
> this line:
> 
> 
>> drop_next_ = control_law(now, count_);
> 
> 
> Note: I think there was an email on CoDel mailing list
> (https://lists.bufferbloat.net/pipermail/codel/) about this issue.
> 
> 
> 
> ------------------------------------------------------------------------
> *From:* Andrew Mcgregor <[e-mail]>
> *Sent:* Friday, June 3, 2016 3:50 AM
> *To:* Goltsman, Polina
> *Cc:* Wesley Eddy; [e-mail]; Jana Iyengar
> *Subject:* Re: [aqm] I-D Action: draft-ietf-aqm-codel-03.txt
>  
> Thank you Polina.
> 
> We have incorporated most of these (sometimes with slightly different
> solutions than you suggested), and a new draft will be out very
> shortly.  I felt that by far the most important was the different way
> the Linux implementation addressed re-entry to the dropping state, and
> the pseudocode now does exactly that.
> 
> Andrew
> 
> On 22 March 2016 at 08:10, Polina Goltsman
> <[e-mail]
> <mailto:[e-mail]>> wrote:
> 
>     Dear Wesley, Dear All,
> 
>     First of all our feedback regarding different "re-entering dropping
>     state" in the document and in the Linux implementation
>     (http://www.ietf.org/mail-archive/web/aqm/current/msg01686.html) was
>     not addressed.
> 
>     As FQ-CoDel  relies on CoDel, this issue is also (partly) relevant
>     for the FQ-CoDel document. In the introduction FQ-CoDel references
>     ns-3 and Linux implementations where the first one uses the
>     re-entering logic from the CoDel document while the second from
>     CoDel Linux implementation. The algorithm that has seen widespread
>     testing according to Section 7 is (I suppose) the Linux version. Is
>     this situation acceptable for an algorithm specification?
> 
>     /[since this comment was supposed to be sent before the end of 2015,
>     feel free to (silently) ignore it]/
>     Second, unlike Rasool Al-Saadi (see
>     http://www.ietf.org/mail-archive/web/aqm/current/msg01693.html) I do
>     not like the document. Although I agree that the pseudocode is
>     sufficient to create a working implementation, however, in my
>     opinion, the rest of the document makes implementing CoDeL more
>     confusing (at least without reading [CODEL2012] first). Is it normal
>     for a RFC, which, as I assume, should primarily contain an algorithm
>     specification to contain the algorithm specification ONLY in form of
>     pseudocode?  
>     These are two items that I found the most confusing:
>     (1) section 3 introduces estimator, setpoint, and control loop which
>     are not clearly distinguishable in pseudocode. It would be nice if
>     section 4 explained how the three entities transition into the
>     routines in pseudocode.
>     (2) IMHO, there is some missing explanations. For example, the
>     document never says how exactly "bad"/persistent queue is
>     determined. The document says in Section 3.1 that the minimum is
>     tracked, but it never says that persistent queue is when the minimum
>     is above setpoint/target for at least an interval. As another
>     example, Section 3.3 could say how the controller looks like exactly.
> 
>     I also have some small comments and nits regarding the latest
>     version (-03):
> 
>     ---
> 
>     in the whole document there are several places of using "we" to
>     (supposedly) refer to the authors. According to Bob Briscoe's
>     message
>     (http://www.ietf.org/mail-archive/web/aqm/current/msg01805.html),
>     "we" do not necessary refer to the authors in a rfc. Are these "we"s ok?
> 
>     ---
> 
>     abstract:
> 
>     This document describes a general *framework* called CoDel 
> 
>     IMHO the word "framework" is too overloaded and too broad. It might
>     be better to call CoDel an AQM or something that has "queue" in it.
>     Other ways (e.g., http://queue.acm.org/detail.cfm?id=2839461)
>     probably also use it to control a queue.
> 
>     ---
> 
>     section 1:
> 
>     (nit)
>        Despite this
>        awareness, the problem has only gotten worse as Moore's Law growth in
>        memory density fueled an exponential increase in buffer pool size.
>        Efforts to deploy AQM have been frustrated by difficult configuration
>        and negative impact on network utilization.  *This problem,* recently
>        christened "bufferbloat", [TSV2011] [BB2011] has become increasingly
>        important throughout the Internet but particularly at the consumer
>        edge.
> 
>     it reads like "this problem" refers to "efforts to deploy AQM" and
>     not to "increase in buffer pool size"
> 
>     ---
> 
>     (nit)
> 
>        To understand
>        queue management, it is critical to understand the difference between
>        the necessary, useful *"good" queue*, and the counterproductive *"bad" queue*.
> 
>     strictly speaking "good" and "bad" queue were not defined either
>     before or in this sentence (it is explained in section 3 or in
>     [CODEL2012])
> 
>     ---
> 
>     (nit)
> 
>        o  treat "good queue" and "bad queue" differently, that is, keep
>           delay low while permitting necessary bursts of traffic
> 
>     (1) see above
>     (2) I would say that the goal is "to keep delay low while ..." and
>     to treat good/bad queue differently is a mechanism used to achieve
>     the goal
> 
>     ---
> 
>     section 2:
> 
>     (nit)
>           In this document, *the characters ">>" preceding an indented
>     line(s)*
>        indicates a compliance requirement statement using the key words
>        listed above.  This convention aids reviewers in quickly identifying
>        or finding the explicit compliance requirements of this RFC.
> 
>     I maybe missing something, but I can't find any use of "characters
>     ">>"" anywhere except this line in any (txt,html, pdf) of the versions.
> 
>     ---
> 
>     section 3:
> 
>     generally IMHO it would be nice to introduce /interval/ and /target/
>     so it is more obvious that these are parameters (or some other kind
>     of variables). Interval is more or less obvious (although imho
>     should be more explicit). The target is first introduced at the end
>     of section 3.2 as:
> 
>        This results in a particularly simple form for the
>        setpoint: the ideal range for the permitted standing queue is between
>        5% and 10% of the TCP connection's RTT.  Thus *target* is simply 5% of
>        the interval of section 3.1
>     <https://tools.ietf.org/html/draft-ietf-aqm-codel-03#section-3.1>.
> 
>     It was not previously defined that /target/ is a parameter with a
>     meaning of /setpoint/. I don't think that this is clear for people
>     who do not know what target is in advance.
> 
>     [It seems that it was intended to introduce /interval/ and /target/
>     parameters in section 4]
> 
>     ---
> 
>        Instead of averages
>        *we recommend* tracking the minimum sojourn time, then, 
> 
>     "we recommend" or "CoDeL does"? According to the last paragraph in
>     Section 3.1, tracking minimum is a core part of CoDeL:
> 
>        These two innovations, use of sojourn time as observed values *and the local minimum as the statistic to monitor queue congestion* are key to
>        CoDel's estimator building block.
> 
>     ---
> 
>        Since the peak queue delay*is simply f r*, power is solely a function
> 
>     is it f/r ?
> 
>     ---
>     (nit)
> 
>        The only remaining building block needed for a basic AQM is a
>        'control loop' algorithm to effectively drive the queueing system
>        from any *'persistent queue above target'* state to a state where the
>        *persistent queue is below target*.
> 
>     It would be nice to explicitly state that the "persistent queue
>     above target" is indication of "bad" queue/congestion somewhere
>     above or in this line. I think that these terms are used in
>     [codel2012] but were not used before in the document. See also the
>     next item.
> 
>     ---
>     In section 3.3 there are three "states":
> 
>       *
> 
>         'persistent queue above target' state
> 
>       *
> 
>         'has persistent queue' state
> 
>       *
> 
>          dropping state
> 
>     It would be nice to a) use the same one b) explain how CoDeL gets
>     in/out of there
> 
>     ---
> 
>     (probably nit)
> 
>        When
>        *the minimum sojourn time first crosses the target* and *CoDel drops a packet*, the earliest the controller could see the effect of the drop
> 
>     from current sentence it could be inferred that the two events occur
>     at the same time which is not the case. Maybe " when persistent
>     queue above target is detected" would be better.
> 
>     ---
> 
>        variation is less than the target, and so the *initial drop spacing SHOULD be set to the estimator's interval plus
>     twice the target (i.e., initial drop spacing = 1.1 * interval)* to ensure that the
> 
>     this contradicts the pseudocode:
> 
>        dodequeue_result codel_queue_t::dodequeue(time_t now)
> 
>     	...
>     	first_above_time_ = now + interval_;
> 
>     section 4:
> 
>        To keep from
>        making drops when it would starve the output link, CoDel makes
>        another check before dropping to see if at least an MTU worth of
>        bytes remains in the buffer. If not, the packet SHOULD NOT be
>        dropped and, *currently*, CoDel exits the drop state. 
> 
>     what is the meaning of "currently" ? in the current experimental
>     version?
> 
>     ---
>     (nit)
> 
>        In practice, this value is not known or measured *(*though see
>        Section 6.2
>     <https://tools.ietf.org/html/draft-ietf-aqm-codel-03#section-6.2> for an application where interval is measured.
> 
>     missing closing bracket
> 
>     Section 5 / pseudocode: (all nits)
> 
>     The variables are introduced as first_time_above, dropping, but are
>     used as first_time_above_, dropping_ (with underscores at end). 
> 
>     ---
> 
>     *Packet** CoDelQueue::dequeue()
> 
>     a pointer to current packet was introduced as packet_t
> 
>     *--- double* now = clock();;
> 
>     time units were supposed to be of type time_t
> 
>     *--- dodequeueResult* r = dodequeue(now);
> 
>     compare with: typedef struct { ... } *dodequeue_result* below
>     (present in two routines)
> 
>     ---
> 
>     dodequeueResult r = {*NULL, queue_t::dequeue() *};
> 
>     shouldn't it be {queue_t::dequeue(), false} ?
> 
> 
>     Best Regards,
>     Polina
> 
> 
>     * note the reply by Dave Taht:
>     http://www.ietf.org/mail-archive/web/aqm/current/msg01687.html
> 
>     On 03/20/2016 04:35 PM, Wesley Eddy wrote:
>>     It looks like the WGLC feedback on the document body is
>>     incorporated, so this is good.
>>
>>     Is there a reason to stay with Informational and not Experimental
>>     like we've done with PIE an d FQ-CoDel?
>>
>>     Also, idnits has some problems with the references that should be
>>     fixed (e.g. "NATAL2010" is probably supposed to be "NETAL2010"):
>>     https://tools.ietf.org/idnits?url=https://tools.ietf.org/id/draft-ietf-aqm-codel-03.txt
> 
> 
>     _______________________________________________
>     aqm mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/aqm
> 
> 
> 
> 
> -- 
> Andrew McGregor | SRE | [e-mail]
> <mailto:[e-mail]> | [phone]
> 
> 
> _______________________________________________
> aqm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/aqm
> 

-- 
Prof. Dr. habil. Michael Menth
University of Tuebingen
Faculty of Science
Department of Computer Science
Chair of Communication Networks
Sand 13, 72076 Tuebingen, Germany
phone: [phone]
fax: [phone]
mailto:[e-mail]
http://kn.inf.uni-tuebingen.de