[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47015CD4.2040302@fatooh.org>
Date: Mon, 01 Oct 2007 13:47:16 -0700
From: Corey Hickey <bugfood-ml@...ooh.org>
To: Patrick McHardy <kaber@...sh.net>
CC: netdev@...r.kernel.org
Subject: Re: [PATCH 09/10] Change perturb_period to unsigned.
Patrick McHardy wrote:
> Corey Hickey wrote:
>> perturb_period is currently a signed integer, but I can't see any good
>> reason why this is so--a negative perturbation period will add a timer
>> that expires in the past, causing constant perturbation, which makes
>> hashing useless.
>>
>> if (q->perturb_period) {
>> q->perturb_timer.expires = jiffies + q->perturb_period;
>> add_timer(&q->perturb_timer);
>> }
>>
>> Strictly speaking, this will break binary compatibility with older
>> versions of tc, but that ought not to be a problem because (a) there's
>> no valid use for a negative perturb_period, and (b) negative values
>> will be seen as high values (> INT_MAX), which don't work anyway.
>>
>> If perturb_period is too large, (perturb_period * HZ) will overflow the
>> size of an unsigned int and wrap around. So, check for thet and reject
>> values that are too high.
>
>
> Sounds reasonable.
>
>> --- a/net/sched/sch_sfq.c
>> +++ b/net/sched/sch_sfq.c
>> @@ -74,6 +74,9 @@
>> typedef unsigned int sfq_index;
>> #define SFQ_MAX_DEPTH (UINT_MAX / 2 - 1)
>>
>> +/* We don't want perturb_period * HZ to overflow an unsigned int. */
>> +#define SFQ_MAX_PERTURB (UINT_MAX / HZ)
>
>
> jiffies are unsigned long.
Hmm. You're right. It looks like my previous patch obviated the need for
this part. I'll remove it.
-Corey
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists