[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48F732CB.8030704@cosmosbay.com>
Date: Thu, 16 Oct 2008 14:25:47 +0200
From: Eric Dumazet <dada1@...mosbay.com>
To: Neil Horman <nhorman@...driver.com>
Cc: David Miller <davem@...emloft.net>, billfink@...dspring.com,
netdev@...r.kernel.org, kuznet@....inr.ac.ru, pekkas@...core.fi,
jmorris@...ei.org, yoshfuji@...ux-ipv6.org, kaber@...sh.net,
johnpol@....mipt.ru
Subject: Re: [PATCH] net: implement emergency route cache rebulds when gc_elasticity
is exceeded
Neil Horman a écrit :
> On Wed, Oct 15, 2008 at 11:55:56PM -0700, David Miller wrote:
>> From: Neil Horman <nhorman@...driver.com>
>> Date: Mon, 13 Oct 2008 14:26:55 -0400
>>
>>> If this meets everyones approval I think we can follow up with a
>>> patch to remove the secret interval code entirely.
>> This patch looks pretty good to me.
>>
>> Just some minor coding style nits:
>>
>>> +static void rt_secret_rebuild_oneshot(struct net *net) {
>> Openning brace on new line please.
>>
>>> +static void rt_emergency_hash_rebuild(struct net *net) {
>> Likewise.
>>
>
> Thanks Dave, new patch, with those nits fixed up. I also cleaned up a few
> checkpatch errors (all trailing whitespace and 80 col errors)
>
> Best
> Neil
>
> Signed-off-by: Neil Horman <nhorman@...driver.com>
>
> +/*
> + * While freeing expired entries, we compute average chain length
> + * and standard deviation, using fixed-point arithmetic.
> + * This to have an estimation of rt_chain_length_max
> + * rt_chain_length_max = max(elasticity, AVG + 4*SD)
> + * We use 3 bits for frational part, and 29 (or 61) for magnitude.
> + */
> +
> +#define FRACT_BITS 3
> +#define ONE (1UL << FRACT_BITS)
> +
> static void rt_check_expire(void)
> {
> static unsigned int rover;
> unsigned int i = rover, goal;
> struct rtable *rth, **rthp;
> + unsigned long length;
> u64 mult;
>
> mult = ((u64)ip_rt_gc_interval) << rt_hash_log;
> @@ -784,11 +812,29 @@ static void rt_check_expire(void)
> if (time_before_eq(jiffies, rth->u.dst.expires)) {
> tmo >>= 1;
> rthp = &rth->u.dst.rt_next;
> + /*
> + * Only bump our length if the hash
> + * inputs on entries n and n+1 are not
> + * the same, we only count entries on
> + * a chain with equal hash inputs once
> + * so that entries for different QOS
> + * levels, and other non-hash input
> + * attributes don't unfairly skew
> + * the length computation
> + */
> + if (*rthp &&
> + !compare_hash_inputs(&(*rthp)->fl,
> + &rth->fl))
> + length += ONE;
> continue;
> }
> } else if (!rt_may_expire(rth, tmo, ip_rt_gc_timeout)) {
> tmo >>= 1;
> rthp = &rth->u.dst.rt_next;
> + if (*rthp &&
> + !compare_hash_inputs(&(*rthp)->fl,
> + &rth->fl))
> + length += ONE;
> continue;
> }
Incomplete patch ?
You added a 'length' variable, and update it but nowhere initialize and/or read it ?
Some way to change rt_chain_length_max is needed, sysctl or dynamically...
--
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