On Tue, Jun 1, 2010 at 1:05 PM, Eric Dumazet [off-list ref] wrote:

Le mardi 01 juin 2010 à 08:28 +0800, Changli Gao a écrit :

quoted

On Tue, Jun 1, 2010 at 5:21 AM, Eric Dumazet [off-list ref] wrote:

quoted

I had a look at current conntrack and found the 'unconfirmed' list was
maybe a candidate for a potential blackhole.

That is, if a reader happens to hit an entry that is moved from regular
hash table slot 'hash' to unconfirmed list,

Sorry, but I can't find where we do this things. unconfirmed list is
used to track the unconfirmed cts, whose corresponding skbs are still
in path from the first to the last netfilter hooks. As soon as the
skbs end their travel in netfilter, the corresponding cts will be
confirmed(moving ct from unconfirmed list to regular hash table).

So netfilter is a monolithic thing.

When a packet begins its travel into netfilter, you guarantee that no
other packet can also begin its travel and find an unconfirmed
conntrack ?

I wonder why we use atomic ops then to track the confirmed bit :)

seems no need.

quoted

unconfirmed list should be small, as networking receiving is in BH.

So according to you, netfilter/ct runs only in input path ?

No. there are another entrances: local out and nf_reinject. If there
isn't any packet queued, as netfilter is in atomic context, the nubmer
of unconfirmed cts should be small( at most, 2 * nr_cpu?).

So I assume a packet is handled by CPU X, creates a new conntrack
(possibly early droping an old entry that was previously in a standard
hash chain), inserted in unconfirmed list.

Oh, Thanks, I got it.

_You_ guarantee another CPU
Y, handling another packet, possibly sent by a hacker reading your
netdev mails, cannot find the conntrack that was early dropped ?

quoted

How about implementing unconfirmed list as a per cpu variable?

I first implemented such a patch to reduce cache line contention, then I
asked to myself : What is exactly an unconfirmed conntrack ? Can their
number be unbounded ? If yes, we have a problem, even on a two cpus
machine. Using two lists instead of one wont solve the fundamental
problem.

The real question is, why do we need this unconfirmed 'list' in the
first place. Is it really a private per cpu thing ?

No, it isn't really private. But most of time, it is accessed locally,
if it is implemented as a per cpu var.

Can you prove this,
in respect of lockless lookups, and things like NFQUEUE ?

Each conntrack object has two list anchors. One for IP_CT_DIR_ORIGINAL,
one for IP_CT_DIR_REPLY.

Unconfirmed list use the first anchor. This means another cpu can
definitely find an unconfirmed item in a regular hash chain, since we
dont respect an RCU grace period before re-using an object.

If memory was not a problem, we probably would use a third anchor to
avoid this, or regular RCU instead of SLAB_DESTROY_BY_RCU variant.

thanks for your explaining.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)