Re: xfrm_state locking regression...

["Timo Teräs" <timo.teras@iki.fi>]

David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Wed, 10 Sep 2008 14:01:07 +1000
> 
>> On Tue, Sep 09, 2008 at 08:38:08PM -0700, David Miller wrote:
>>> No problem.  It might be a little bit of a chore because this new
>>> walker design is intimately tied to the af_key non-atomic dump
>>> changes.
>> Actually I was mistaken as to how the original dump worked.  I'd
>> thought that it actually kept track of which bucket it was in and
>> resumed from that bucket.  However in reality it only had a global
>> counter and would always start walking from the beginning up until
>> the counted value.  So it isn't as easy as just copying the old
>> code across :)
> 
> Only AF_KEY gave an error, and this ended the dump.  This was
> one of Timo's goals, to make AF_KEY continue where it left
> off in subsequent dump calls done by the user, when we hit the
> socket limit.

Yes, this was the other goal. The other goal was to speed up
dumping in netlink from O(n^2) to O(n). And fixing the problem that
netlink might miss to dump some entries (if an entry was removed
from the beginning of the hash when dumping was ongoing).

> ipsec: Restore hash based xfrm_state dumping.
> 
> Get rid of ->all member of struct xfrm_state, and just use a hash
> iteration like we used before.
> 
> This shrinks the size of struct xfrm_state, and also restores the
> dump ordering of 2.6.25 and previous.

I think bad things will happen if the hash gets resized between
xfrm_state_walk() calls. Since it assumes that the walk->state
entry has remained in walk->chain bucket.

The dumping order shouldn't really make any difference. But
reducing struct xfrm_state is definitely good. The only downside
is that when an entry is inserted to the beginning of the hash
while dump is ongoing, it won't be dumped at all. But that is
not really a problem since you get a notification about that
entry separately.

Cheers,
  Timo