[PATCH] net_sched: sch_sfq: fix allot handling

Eric Dumazet
2010-12-15T14:03:08Z

When deploying SFQ/IFB here at work, I found the allot management was
pretty wrong in sfq, even changing allot from short to int...

We should init allot for each new flow turn, not using a previous value,
or else small packets can easily make allot overflow.

Before patch, I saw burst of several packets per flow, apparently
denying the "allot 1514" limit I had on my SFQ class.

class sfq 11:1 parent 11:
(dropped 0, overlimits 0 requeues 0)
backlog 0b 7p requeues 0
allot 11546

class sfq 11:46 parent 11:
(dropped 0, overlimits 0 requeues 0)
backlog 0b 1p requeues 0
allot -23873

class sfq 11:78 parent 11:
(dropped 0, overlimits 0 requeues 0)
backlog 0b 5p requeues 0
allot 11393

After patch, better fairness among each flow, allot limit being
respected.

class sfq 11:52 parent 11:
(dropped 0, overlimits 0 requeues 0)
backlog 0b 4p requeues 0
allot 1514

class sfq 11:60 parent 11:
(dropped 0, overlimits 0 requeues 0)
backlog 0b 3p requeues 0
allot -586

class sfq 11:6b parent 11:
(dropped 0, overlimits 0 requeues 0)
backlog 0b 3p requeues 0
allot -586

class sfq 11:71 parent 11:
(dropped 0, overlimits 0 requeues 0)
backlog 0b 3p requeues 0
allot 1514

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/sched/sch_sfq.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 3cf478d..8c8a190 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -270,7 +270,7 @@ static unsigned int sfq_drop(struct Qdisc *sch)
/* It is difficult to believe, but ALL THE SLOTS HAVE LENGTH 1. */
d = q->next[q->tail];
q->next[q->tail] = q->next[d];
- q->allot[q->next[d]] += q->quantum;
+ q->allot[q->next[d]] = q->quantum;
skb = q->qs[d].prev;
len = qdisc_pkt_len(skb);
__skb_unlink(skb, &q->qs[d]);
@@ -321,14 +321,13 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
sfq_inc(q, x);
if (q->qs[x].qlen == 1) { /* The flow is new */
if (q->tail == SFQ_DEPTH) { /* It is the first flow */
- q->tail = x;
q->next[x] = x;
- q->allot[x] = q->quantum;
} else {
q->next[x] = q->next[q->tail];
q->next[q->tail] = x;
- q->tail = x;
}
+ q->tail = x;
+ q->allot[x] = q->quantum;
}
if (++sch->q.qlen <= q->limit) {
sch->bstats.bytes += qdisc_pkt_len(skb);
@@ -382,11 +381,11 @@ sfq_dequeue(struct Qdisc *sch)
return skb;
}
q->next[q->tail] = a;
- q->allot[a] += q->quantum;
+ q->allot[a] = q->quantum;
} else if ((q->allot[a] -= qdisc_pkt_len(skb)) <= 0) {
q->tail = a;
a = q->next[a];
- q->allot[a] += q->quantum;
+ q->allot[a] = q->quantum;
}
return skb;
}