[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4BA32C41.2020000@iki.fi>
Date: Fri, 19 Mar 2010 09:48:17 +0200
From: Timo Teräs <timo.teras@....fi>
To: Herbert Xu <herbert@...dor.apana.org.au>
CC: netdev@...r.kernel.org
Subject: Re: [PATCH] xfrm: cache bundle lookup results in flow cache
Herbert Xu wrote:
> On Mon, Mar 15, 2010 at 02:20:10PM +0200, Timo Teras wrote:
>> - policy = flow_cache_lookup(net, fl, dst_orig->ops->family,
>> - dir, xfrm_policy_lookup);
>> - err = PTR_ERR(policy);
>> - if (IS_ERR(policy)) {
>> - XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
>> - goto dropdst;
>> + fce = flow_cache_lookup(&net->xfrm.flow_cache,
>> + fl, family, dir);
>> + if (fce == NULL)
>> + goto no_cache;
>> +
>> + xf = container_of(fce, struct xfrm_flow_cache_entry, fce);
>> + xfrm_flow_cache_entry_validate(&net->xfrm.flow_cache, fce);
>
> This doesn't work.
>
> The flow cache operates without locking as it is a per-cpu cache.
> To make this work you must ensure that you stay on the same CPU
> or use some other form of synchronoisation if you write to the
> object returned.
>
> AFAICS there is no synchronisation here and you're writing to fce.
>
> So you'll need to disable preemption around the bit that touches
> fce.
But flow_cache_lookup disables pre-emption until _put is called.
So it should work. Would there be a cleaner way?
However, now I figured that we need to make walk.dead atomic
because it's read some times without taking the policy lock.
- Timo
--
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