conntrack locking

[Rusty Russell]

In message <Pine.LNX.4.10.10002091527300.2916-100000@blackhole.kfki.hu> you wri
te:
> Hello,

Hi Josef!

> I'm thinking on how to improve the locking in conntrack. First it seemed
> to be easy: just add locks to the ip_conntrack_hash per entry and that's
> 'all'. However the generation count (ip_conntrack_gen) prevents to do it
> easily: it would behave as a global lock in conntrack again.

First you'd have to provide evidence that conntrack locking needs
improvement, no?

Every connection has two entries in the hash table.  This makes life
tricky, otherwise we could simply lock one hash chain.  At the moment
we do:

find_conntrack_get:
	1) Get read lock.
	2) Look up connection
	3) If found, increment use count.
	4) Drop read lock.
init_conntrack:
	5) If not found, create conntrack.
	6) Get write lock.
	7) Look up connection.
	8) If found, we lost race.  Drop packet and destroy created
	   conntrack.
	9) Otherwise insert in hash table.
	10) Drop write lock.

Note that this optimises for the common case (part of a connection).
I'd like something better, so don't let me stop you...

> So I suggest to eliminate the generation count. Put the logic into
> the netfilter device  by adding the connection expiration time
> to nfdev_head. Then when a packet is returned from the userspace, one can
> make sure in nf_write that the pointer to the conntrack hash is still
> valid.

That's not the problem.  The problem is that an skb still has a
pointer to the conntrack (in nf_mark).  When that is interrogated,
that conntrack must still exist.  Without a way of tracking skbs
(which we don't have), a generation count is the only way.

I hate generation counts: the code grates.  I plan on trying to add
two #ifdef CONFIG_NETFILTER fields to skbuff.h: basically ctinfo and
struct conntrack *, and inserting the hooks to destroy them (and carry
them across fragmentation).  Have to battle Alexey 8-)

Rusty.
--
Hacking time.