Herbert Xu wrote:
quoted text
> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html