Subject: Re: xfrm_state locking regression...

From: Timo Teräs <[email-masked]>
Date: Wed, 03 Sep 2008 10:14:02 +0300

David Miller wrote:
> Get creative, use a key of some sort to continue the walk, that's what
> other netlink'ish subsystems use.

Ok. ctnetlink_dump_table() seems to iterate the hash list and keeps
a unique id where it left. If that entry was deleted it starts again
from that hash bucket.

We could iterate SPD by idx. By SPI would be nice for SAD, but not
all entries have SPI. I guess we could use either of the bysrc or bydst
hash and memorize xfrm_id. Restart from first hash bucket entry if
the memorized entry gets deleted, and restart from the beginning
if hash gets resized.

This way we could avoid the additional locking and guarantee that all
entries are dumped in reasonable time.

>> Yes, but the dumping code produced crap. It could dump same entry
>> multiple times, miss entries and was dog slow. With it there was
>> no possibility to keep userland in sync with kernel SPD/SAD because
>> entries were lost.
> 
> I'd rather see an entry twice in a dump than have my IPSEC gateway
> lockup, or run slower because we take a lock twice as often as
> necessary during object destruction.

Seeing entry twice is not a problem. Not seeing an entry at all was
the real problem. Also listing SAD could take tens of seconds on
modern system. That's not nice either.

- Timo