Am 24.01.2011 15:06, schrieb Eric Dumazet: > Le lundi 24 janvier 2011 Ã 14:37 +0100, Patrick McHardy a Ãcrit : >> On 24.01.2011 14:25, Pablo Neira Ayuso wrote: >>> On 24/01/11 14:12, Eric Dumazet wrote: >>>> Le lundi 24 janvier 2011 Ã 14:06 +0100, Pablo Neira Ayuso a Ãcrit : >>>> >>>>> Yes, we can use nf_conntrack_get (which does atomic_inc) instead. New >>>>> patch attached. >>>> >>>> I feel now a bit uncomfortable, sorry ;) >>>> >>>> Are we sure the refcount cannot reach 0 while we hold >>>> nf_conntrack_lock ? >>> >>> the ct deletion from the hash list is protected by spin lock, so >>> whatever deletion would wait until we have left the dump section. >>> >>> with this patch, the code looks like it was in 2.6.24 before the rcu stuff. >> >> Yeah, we definitely have a reference while the conntrack is contained >> in the hash table, and removal requires taking nf_conntrack_lock, >> therefor using the conntrack entry while holding the lock is valid. > > Yes, but to clarify my question, is the following possible ? > > CPU 0 CPU1 > dump() > spin_lock() > .... > if (not enough room in skb) > decrement last refcount > increment_ref_count() > spin_unlock(); > since refcount hit 0, > spin_lock(nf_conntrack) > remove ct from hashes > No, that can't happen. Before the last refcount is released, the entry is removed from the hash, which requires to take the lock. Each entry contained in the hash table has a refcount of at least 1. If there are no futher concerns, I'm going to apply Pablo's patch. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
