[Devel] [PATCH rh7 2/4] net: Skip allocation of conntrack if there are no rules

[Kirill Tkhai]

On 08.09.2016 08:31, Andrei Vagin wrote:
> On Tue, Sep 06, 2016 at 09:31:33AM +0300, Kirill Tkhai wrote:
>> HW node may have many CTs, which are not having conntrack
>> users. Do not allocate them to save resources and decrease
>> performance penalty.
>>
>> Next two patches will actually mark conntrack users.
>>
>> https://jira.sw.ru/browse/PSBM-51050
>>
>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>> ---
>>  net/netfilter/nf_conntrack_core.c    |    7 ++++++-
>>  net/netfilter/nf_conntrack_netlink.c |    2 ++
>>  net/netfilter/nf_synproxy_core.c     |    2 ++
>>  3 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
>> index b5de5aa..c46bede 100644
>> --- a/net/netfilter/nf_conntrack_core.c
>> +++ b/net/netfilter/nf_conntrack_core.c
>> @@ -841,6 +841,11 @@ __nf_conntrack_alloc(struct net *net, u16 zone,
>>  	unsigned int ct_max = net->ct.max ? net->ct.max : init_net.ct.max;
>>  	struct nf_conn *ct;
>>  
>> +	if (!atomic_read(&net->ct.users)) {
>> +		/* No rules loaded */
>> +		return NULL;
>> +	}
>> +
>>  	if (unlikely(!nf_conntrack_hash_rnd)) {
>>  		init_nf_conntrack_hash_rnd();
>>  		/* recompute the hash as nf_conntrack_hash_rnd is initialized */
>> @@ -963,7 +968,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
>>  
>>  	ct = __nf_conntrack_alloc(net, zone, tuple, &repl_tuple, GFP_ATOMIC,
>>  				  hash);
>> -	if (IS_ERR(ct))
>> +	if (IS_ERR_OR_NULL(ct))
>>  		return (struct nf_conntrack_tuple_hash *)ct;
>>  
>>  	if (tmpl && nfct_synproxy(tmpl)) {
>> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
>> index d6b6465..87f5091 100644
>> --- a/net/netfilter/nf_conntrack_netlink.c
>> +++ b/net/netfilter/nf_conntrack_netlink.c
>> @@ -1617,7 +1617,9 @@ ctnetlink_create_conntrack(struct net *net, u16 zone,
>>  	struct nf_conntrack_helper *helper;
>>  	struct nf_conn_tstamp *tstamp;
>>  
>> +	inc_conntrack_users(net);
>>  	ct = nf_conntrack_alloc(net, zone, otuple, rtuple, GFP_ATOMIC);
>> +	dec_conntrack_users(net);
> 
> When I look at this, I'm starting thinking that we have to start
> tracking connections when someone tries to use them, but we should not
> stop tracking connections.

I didn't completely understand you point. Is it related to the hunk above?
If so, we don't stop tracking the conntracks there.
 
> We should not forget about the fact that conntrack can be used from
> user-space http://conntrack-tools.netfilter.org/manual.html

The above hunk is about this, isn't it?
 
>>  	if (IS_ERR(ct))
>>  		return ERR_PTR(-ENOMEM);
>>  
>> diff --git a/net/netfilter/nf_synproxy_core.c b/net/netfilter/nf_synproxy_core.c
>> index 52e20c9..7bef49e 100644
>> --- a/net/netfilter/nf_synproxy_core.c
>> +++ b/net/netfilter/nf_synproxy_core.c
>> @@ -353,7 +353,9 @@ static int __net_init synproxy_net_init(struct net *net)
>>  	int err = -ENOMEM;
>>  
>>  	memset(&t, 0, sizeof(t));
>> +	inc_conntrack_users(net);
>>  	ct = nf_conntrack_alloc(net, 0, &t, &t, GFP_KERNEL);
>> +	dec_conntrack_users(net);
>>  	if (IS_ERR(ct)) {
>>  		err = PTR_ERR(ct);
>>  		goto err1;
>>