Subject: Re: [RFT 3/4] Use mod_timer_noact to remove nf_conntrack_lock

From: Patrick McHardy <[email-masked]>
Date: Wed, 18 Feb 2009 15:27:58 +0100

Eric Dumazet wrote:
> Patrick McHardy a écrit :
>> Eric Dumazet wrote:
>>> Unfortunatly, this patch changes nothing, as most of the time, do_acct
>>> is true.
>>>
>>> We also need to fine lock the accounting part as well.
>>>
>>>     spin_lock_bh(&ct->some_lock);
>>>     acct = nf_conn_acct_find(ct);
>>>     if (acct) {
>>>         acct[CTINFO2DIR(ctinfo)].packets++;
>>>         acct[CTINFO2DIR(ctinfo)].bytes +=
>>>             skb->len - skb_network_offset(skb);
>>>     }
>>>     spin_unlock_bh(&ct->some_lock);
>>>
>> Its currently still enabled by default, but we intend to change that.
>> After that I guess almost nobody will have it enabled.
>> 
> Really ? I find this accounting stuff really useful and always enable it :)

You usually need extra userspace daemons to make something useful
out of the data and I doubt many people are running them. It doesn't
hurt to optimize it anyways of course :)

But I'm somewhat doubtful that we're actually having lock contention
here. One thing we could do with your lock hash change is to perform
the counter updates while holding those locks, that avoids taking
a different lock just for the counters. The only reason why its done
in nf_ct_refresh is that it was already taking the conntrack lock,
but if thats no longer the case, no reason to keep it there.
--
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