Commit 1ee5fa1e authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

sch_red: fix red_change

Le mercredi 30 novembre 2011 à 14:36 -0800, Stephen Hemminger a écrit :

> (Almost) nobody uses RED because they can't figure it out.
> According to Wikipedia, VJ says that:
>  "there are not one, but two bugs in classic RED."

RED is useful for high throughput routers, I doubt many linux machines
act as such devices.

I was considering adding Adaptative RED (Sally Floyd, Ramakrishna
Gummadi, Scott Shender), August 2001

In this version, maxp is dynamic (from 1% to 50%), and user only have to
setup min_th (target average queue size)
(max_th and wq (burst in linux RED) are automatically setup)

By the way it seems we have a small bug in red_change()

if (skb_queue_empty(&sch->q))
	red_end_of_idle_period(&q->parms);

First, if queue is empty, we should call
red_start_of_idle_period(&q->parms);

Second, since we dont use anymore sch->q, but q->qdisc, the test is
meaningless.

Oh well...

[PATCH] sch_red: fix red_change()

Now RED is classful, we must check q->qdisc->q.qlen, and if queue is empty,
we start an idle period, not end it.
Signed-off-by: default avatarEric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 59c2cdae
...@@ -209,8 +209,8 @@ static int red_change(struct Qdisc *sch, struct nlattr *opt) ...@@ -209,8 +209,8 @@ static int red_change(struct Qdisc *sch, struct nlattr *opt)
ctl->Plog, ctl->Scell_log, ctl->Plog, ctl->Scell_log,
nla_data(tb[TCA_RED_STAB])); nla_data(tb[TCA_RED_STAB]));
if (skb_queue_empty(&sch->q)) if (!q->qdisc->q.qlen)
red_end_of_idle_period(&q->parms); red_start_of_idle_period(&q->parms);
sch_tree_unlock(sch); sch_tree_unlock(sch);
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment