Subject: Re: [PATCH] conntrack: use SLAB_DESTROY_BY_RCU for nf_conn structs

From: Patrick McHardy <[email-masked]>
Date: Wed, 25 Mar 2009 20:41:08 +0100

Eric Dumazet wrote:
> Patrick McHardy a écrit :
>>>      NF_CT_ASSERT(ct);
>>> +    if (unlikely(!atomic_inc_not_zero(&ct->ct_general.use)))
>>> +        return 0;
>> Can we assume the next pointer still points to the next entry
>> in the same chain after the refcount dropped to zero?
>>
> 
> We are looking chain N.
> If we cannot atomic_inc() refcount, we got some deleted entry.
> If we could atomic_inc, we can meet an entry that just moved to another chain X
> 
> When hitting its end, we continue the search to the N+1 chain so we only 
> skip the end of previous chain (N). We can 'forget' some entries, we can print
> several time one given entry.
> 
> 
> We could solve this by :
> 
> 1) Checking hash value : if not one expected -> 
>    Going back to head of chain N, (potentially re-printing already handled entries)
>    So it is not a *perfect* solution.
> 
> 2) Use a locking to forbid writers (as done in UDP/TCP), but it is expensive and
> wont solve other problem :
> 
> We wont avoid emitting same entry several time anyway (this is a flaw of 
> current seq_file handling, since we 'count' entries to be skiped, and this is
> wrong if some entries were deleted or inserted meanwhile)
> 
> We have same problem on /proc/net/udp & /proc/net/tcp, I am not sure we should care...

I think double entries are not a problem, as you say, there
are already other cases where this can happen. But I think we
should try our best that every entry present at the start and
still present at the end of a dump is also contained in the
dump, otherwise the guantees seem to weak to still be useful.
Your first proposal would do exactly that, right?

> Also, current resizing code can give to a /proc/net/ip_conntrack reader a problem, since
> hash table can switch while its doing its dumping : many entries might be lost or regiven...

Thats true. But its a very rare operation, so I think its mainly
a documentation issue.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email-masked]
More majordomo info at  http://vger.kernel.org/majordomo-info.html