* [PATCH] sch_qfq: fix overflow in qfq_update_start()
@ 2012-01-02 15:47 Eric Dumazet
2012-01-02 21:45 ` Eric Dumazet
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eric Dumazet @ 2012-01-02 15:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Stephen Hemminger, Dave Taht
grp->slot_shift is between 22 and 41, so using 32bit wide variables is
probably a typo.
This could explain QFQ hangs Dave reported to me, after 2^23 packets ?
(23 = 64 - 41)
Reported-by: Dave Taht <dave.taht@gmail.com>
CC: Stephen Hemminger <shemminger@vyatta.com>
CC: Dave Taht <dave.taht@gmail.com>
---
net/sched/sch_qfq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 1033434..7b03254 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -817,11 +817,11 @@ skip_unblock:
static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl)
{
unsigned long mask;
- uint32_t limit, roundedF;
+ u64 limit, roundedF;
int slot_shift = cl->grp->slot_shift;
roundedF = qfq_round_down(cl->F, slot_shift);
- limit = qfq_round_down(q->V, slot_shift) + (1UL << slot_shift);
+ limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift);
if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) {
/* timestamp was stale */
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sch_qfq: fix overflow in qfq_update_start()
2012-01-02 15:47 [PATCH] sch_qfq: fix overflow in qfq_update_start() Eric Dumazet
@ 2012-01-02 21:45 ` Eric Dumazet
2012-01-02 21:56 ` Dave Taht
2012-01-03 16:25 ` Stephen Hemminger
2012-01-03 17:59 ` David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2012-01-02 21:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Stephen Hemminger, Dave Taht
Le lundi 02 janvier 2012 à 16:47 +0100, Eric Dumazet a écrit :
> grp->slot_shift is between 22 and 41, so using 32bit wide variables is
> probably a typo.
>
> This could explain QFQ hangs Dave reported to me, after 2^23 packets ?
>
> (23 = 64 - 41)
>
> Reported-by: Dave Taht <dave.taht@gmail.com>
> CC: Stephen Hemminger <shemminger@vyatta.com>
> CC: Dave Taht <dave.taht@gmail.com>
Oh well, I forgot my SOB :(
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
David please just tell me if you prefer me to resend it
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sch_qfq: fix overflow in qfq_update_start()
2012-01-02 21:45 ` Eric Dumazet
@ 2012-01-02 21:56 ` Dave Taht
0 siblings, 0 replies; 6+ messages in thread
From: Dave Taht @ 2012-01-02 21:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Stephen Hemminger
On Mon, Jan 2, 2012 at 10:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 02 janvier 2012 à 16:47 +0100, Eric Dumazet a écrit :
>> grp->slot_shift is between 22 and 41, so using 32bit wide variables is
>> probably a typo.
>>
>> This could explain QFQ hangs Dave reported to me, after 2^23 packets ?
>>
>> (23 = 64 - 41)
>>
>> Reported-by: Dave Taht <dave.taht@gmail.com>
>> CC: Stephen Hemminger <shemminger@vyatta.com>
>> CC: Dave Taht <dave.taht@gmail.com>
>
> Oh well, I forgot my SOB :(
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> David please just tell me if you prefer me to resend it
>
>
I figure I'm the wrong david, but I'll be testing the QFQ patch overnight.
I have tested it for about an hour thus far.
Your SFQ patch, posted earlier today, does bring SFQ and QFQ back
into equivalence at 10 iperfs.
http://www.teklibre.com/~d/bloat/sfqnewvsqfq10iperfs.png
Well done.
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sch_qfq: fix overflow in qfq_update_start()
2012-01-02 15:47 [PATCH] sch_qfq: fix overflow in qfq_update_start() Eric Dumazet
2012-01-02 21:45 ` Eric Dumazet
@ 2012-01-03 16:25 ` Stephen Hemminger
2012-01-03 16:54 ` Luigi Rizzo
2012-01-03 17:59 ` David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2012-01-03 16:25 UTC (permalink / raw)
To: Eric Dumazet, Luigi Rizzo; +Cc: David Miller, netdev, Dave Taht
On Mon, 02 Jan 2012 16:47:57 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> grp->slot_shift is between 22 and 41, so using 32bit wide variables is
> probably a typo.
>
> This could explain QFQ hangs Dave reported to me, after 2^23 packets ?
>
> (23 = 64 - 41)
>
> Reported-by: Dave Taht <dave.taht@gmail.com>
> CC: Stephen Hemminger <shemminger@vyatta.com>
> CC: Dave Taht <dave.taht@gmail.com>
> ---
> net/sched/sch_qfq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 1033434..7b03254 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -817,11 +817,11 @@ skip_unblock:
> static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl)
> {
> unsigned long mask;
> - uint32_t limit, roundedF;
> + u64 limit, roundedF;
> int slot_shift = cl->grp->slot_shift;
>
> roundedF = qfq_round_down(cl->F, slot_shift);
> - limit = qfq_round_down(q->V, slot_shift) + (1UL << slot_shift);
> + limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift);
>
> if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) {
> /* timestamp was stale */
>
>
You need to copy the BSD developers on these patches since
most of the code came from them and FreeBSD probably still has same bug!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sch_qfq: fix overflow in qfq_update_start()
2012-01-03 16:25 ` Stephen Hemminger
@ 2012-01-03 16:54 ` Luigi Rizzo
0 siblings, 0 replies; 6+ messages in thread
From: Luigi Rizzo @ 2012-01-03 16:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Eric Dumazet, David Miller, netdev, Dave Taht
thanks, appreciated.
cheers
luigi
On Tue, Jan 03, 2012 at 08:25:57AM -0800, Stephen Hemminger wrote:
> On Mon, 02 Jan 2012 16:47:57 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > grp->slot_shift is between 22 and 41, so using 32bit wide variables is
> > probably a typo.
> >
> > This could explain QFQ hangs Dave reported to me, after 2^23 packets ?
> >
> > (23 = 64 - 41)
> >
> > Reported-by: Dave Taht <dave.taht@gmail.com>
> > CC: Stephen Hemminger <shemminger@vyatta.com>
> > CC: Dave Taht <dave.taht@gmail.com>
> > ---
> > net/sched/sch_qfq.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> > index 1033434..7b03254 100644
> > --- a/net/sched/sch_qfq.c
> > +++ b/net/sched/sch_qfq.c
> > @@ -817,11 +817,11 @@ skip_unblock:
> > static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl)
> > {
> > unsigned long mask;
> > - uint32_t limit, roundedF;
> > + u64 limit, roundedF;
> > int slot_shift = cl->grp->slot_shift;
> >
> > roundedF = qfq_round_down(cl->F, slot_shift);
> > - limit = qfq_round_down(q->V, slot_shift) + (1UL << slot_shift);
> > + limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift);
> >
> > if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) {
> > /* timestamp was stale */
> >
> >
>
>
> You need to copy the BSD developers on these patches since
> most of the code came from them and FreeBSD probably still has same bug!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sch_qfq: fix overflow in qfq_update_start()
2012-01-02 15:47 [PATCH] sch_qfq: fix overflow in qfq_update_start() Eric Dumazet
2012-01-02 21:45 ` Eric Dumazet
2012-01-03 16:25 ` Stephen Hemminger
@ 2012-01-03 17:59 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-01-03 17:59 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, shemminger, dave.taht
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Jan 2012 16:47:57 +0100
> grp->slot_shift is between 22 and 41, so using 32bit wide variables is
> probably a typo.
>
> This could explain QFQ hangs Dave reported to me, after 2^23 packets ?
>
> (23 = 64 - 41)
>
> Reported-by: Dave Taht <dave.taht@gmail.com>
> CC: Stephen Hemminger <shemminger@vyatta.com>
> CC: Dave Taht <dave.taht@gmail.com>
Applied, thanks Eric.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-03 17:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-02 15:47 [PATCH] sch_qfq: fix overflow in qfq_update_start() Eric Dumazet
2012-01-02 21:45 ` Eric Dumazet
2012-01-02 21:56 ` Dave Taht
2012-01-03 16:25 ` Stephen Hemminger
2012-01-03 16:54 ` Luigi Rizzo
2012-01-03 17:59 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox