All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: Patrick McHardy <kaber@trash.net>,
	netdev <netdev@vger.kernel.org>,
	Jarek Poplawski <jarkao2@gmail.com>
Subject: [PATCH] net_sched: sch_sfq: fix allot handling
Date: Wed, 15 Dec 2010 15:03:03 +0100	[thread overview]
Message-ID: <1292421783.3427.232.camel@edumazet-laptop> (raw)

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



             reply	other threads:[~2010-12-15 14:03 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-15 14:03 Eric Dumazet [this message]
2010-12-15 16:03 ` [PATCH] net_sched: sch_sfq: fix allot handling Patrick McHardy
2010-12-15 16:27   ` Eric Dumazet
2010-12-15 16:40     ` [PATCH v2] " Eric Dumazet
2010-12-15 16:43       ` Patrick McHardy
2010-12-15 16:55         ` Eric Dumazet
2010-12-15 17:03           ` Patrick McHardy
2010-12-15 17:09             ` Eric Dumazet
2010-12-15 17:21               ` Patrick McHardy
2010-12-15 17:30                 ` [PATCH v3] " Eric Dumazet
2010-12-15 18:18                   ` [PATCH net-next-2.6] net_sched: sch_sfq: add backlog info in sfq_dump_class_stats() Eric Dumazet
2010-12-15 19:10                     ` Eric Dumazet
2010-12-16  8:16                     ` Jarek Poplawski
2010-12-16 10:18                       ` [PATCH v2 " Eric Dumazet
2010-12-16 11:03                       ` [PATCH " Eric Dumazet
2010-12-16 13:09                         ` Jarek Poplawski
2010-12-20 21:14                     ` David Miller
2010-12-20 21:18                   ` [PATCH v3] net_sched: sch_sfq: fix allot handling David Miller
2010-12-16 13:08             ` [PATCH v2] " Eric Dumazet
2010-12-17 16:52               ` [RFC PATCH] net_sched: sch_sfq: better struct layouts Eric Dumazet
2010-12-19 21:22                 ` Jarek Poplawski
2010-12-20 17:02                   ` [PATCH v2] " Eric Dumazet
2010-12-20 21:33                     ` David Miller
2010-12-20 21:42                       ` Eric Dumazet
2010-12-20 22:54                         ` [PATCH v3 net-next-2.6] " Eric Dumazet
2010-12-21  5:33                           ` David Miller
2010-12-20 22:55                     ` [PATCH v2] " Jarek Poplawski
2010-12-20 23:16                     ` [PATCH net-next-2.6] sch_sfq: allow big packets and be fair Eric Dumazet
2010-12-21 10:15                       ` Jarek Poplawski
2010-12-21 10:30                         ` Jarek Poplawski
2010-12-21 10:44                           ` Eric Dumazet
2010-12-21 10:56                             ` Jarek Poplawski
2010-12-21 10:57                         ` Eric Dumazet
2010-12-21 11:39                           ` Jarek Poplawski
2010-12-21 12:17                             ` Jarek Poplawski
2010-12-21 13:04                               ` [PATCH v2 " Eric Dumazet
2010-12-21 13:47                                 ` Jarek Poplawski
2010-12-28 21:46                                 ` David Miller
2010-12-29  7:53                                   ` [PATCH v3 " Eric Dumazet
2010-12-31 20:48                                     ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1292421783.3427.232.camel@edumazet-laptop \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=jarkao2@gmail.com \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.