Patchwork [04/13] netfilter: regard users as refcount for l4proto's per-net data

login
register
mail settings
Submitter Gao feng
Date June 21, 2012, 2:36 p.m.
Message ID <1340289410-17642-4-git-send-email-gaofeng@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/166337/
State Accepted
Headers show

Comments

Gao feng - June 21, 2012, 2:36 p.m.
Now, nf_proto_net's users is confusing.
we should regard it as the refcount for l4proto's per-net data,
because maybe there are two l4protos use the same per-net data.

so increment pn->users when nf_conntrack_l4proto_register
success, and decrement it for nf_conntrack_l4_unregister case.

because nf_conntrack_l3proto_ipv[4|6] don't use the same per-net
data,so we don't need to add a refcnt for their per-net data.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/netfilter/nf_conntrack_proto.c |   76 ++++++++++++++++++++++--------------
 1 files changed, 46 insertions(+), 30 deletions(-)
Pablo Neira - June 25, 2012, 11:20 a.m.
On Thu, Jun 21, 2012 at 10:36:41PM +0800, Gao feng wrote:
> Now, nf_proto_net's users is confusing.
> we should regard it as the refcount for l4proto's per-net data,
> because maybe there are two l4protos use the same per-net data.
> 
> so increment pn->users when nf_conntrack_l4proto_register
> success, and decrement it for nf_conntrack_l4_unregister case.
> 
> because nf_conntrack_l3proto_ipv[4|6] don't use the same per-net
> data,so we don't need to add a refcnt for their per-net data.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  net/netfilter/nf_conntrack_proto.c |   76 ++++++++++++++++++++++--------------
>  1 files changed, 46 insertions(+), 30 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> index 9d6b6ab..63612e6 100644
> --- a/net/netfilter/nf_conntrack_proto.c
> +++ b/net/netfilter/nf_conntrack_proto.c
[...]
> @@ -458,23 +446,32 @@ int nf_conntrack_l4proto_register(struct net *net,
>  				  struct nf_conntrack_l4proto *l4proto)
>  {
>  	int ret = 0;
> +	struct nf_proto_net *pn = NULL;
>  
>  	if (l4proto->init_net) {
>  		ret = l4proto->init_net(net, l4proto->l3proto);
>  		if (ret < 0)
> -			return ret;
> +			goto out;
>  	}
>  
> -	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
> +	pn = nf_ct_l4proto_net(net, l4proto);
> +	if (pn == NULL)
> +		goto out;

Same thing here, we're leaking memory allocated by l4proto->init_net.

> +	ret = nf_ct_l4proto_register_sysctl(net, pn, l4proto);
>  	if (ret < 0)
> -		return ret;
> +		goto out;
>  
>  	if (net == &init_net) {
>  		ret = nf_conntrack_l4proto_register_net(l4proto);
> -		if (ret < 0)
> -			nf_ct_l4proto_unregister_sysctl(net, l4proto);
> +		if (ret < 0) {
> +			nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
> +			goto out;

Better replace the two lines above by:

goto out_register_net;

and then...

> +		}
>  	}
>  
> +	pn->users++;

out_register_net:
        nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);

> +out:
>  	return ret;

I think that this change is similar to patch 1/1, I think you should
send it as a separated patch.

>  }
>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
> @@ -499,10 +496,18 @@ nf_conntrack_l4proto_unregister_net(struct nf_conntrack_l4proto *l4proto)
>  void nf_conntrack_l4proto_unregister(struct net *net,
>  				     struct nf_conntrack_l4proto *l4proto)
>  {
> +	struct nf_proto_net *pn = NULL;
> +
>  	if (net == &init_net)
>  		nf_conntrack_l4proto_unregister_net(l4proto);
>  
> -	nf_ct_l4proto_unregister_sysctl(net, l4proto);
> +	pn = nf_ct_l4proto_net(net, l4proto);
> +	if (pn == NULL)
> +		return;
> +
> +	pn->users--;
> +	nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
> +
>  	/* Remove all contrack entries for this protocol */
>  	rtnl_lock();
>  	nf_ct_iterate_cleanup(net, kill_l4proto, l4proto);
> @@ -514,11 +519,15 @@ int nf_conntrack_proto_init(struct net *net)
>  {
>  	unsigned int i;
>  	int err;
> +	struct nf_proto_net *pn = nf_ct_l4proto_net(net,
> +					&nf_conntrack_l4proto_generic);
> +
>  	err = nf_conntrack_l4proto_generic.init_net(net,
>  					nf_conntrack_l4proto_generic.l3proto);
>  	if (err < 0)
>  		return err;
>  	err = nf_ct_l4proto_register_sysctl(net,
> +					    pn,
>  					    &nf_conntrack_l4proto_generic);
>  	if (err < 0)
>  		return err;
> @@ -528,13 +537,20 @@ int nf_conntrack_proto_init(struct net *net)
>  			rcu_assign_pointer(nf_ct_l3protos[i],
>  					   &nf_conntrack_l3proto_generic);
>  	}
> +
> +	pn->users++;
>  	return 0;
>  }
>  
>  void nf_conntrack_proto_fini(struct net *net)
>  {
>  	unsigned int i;
> +	struct nf_proto_net *pn = nf_ct_l4proto_net(net,
> +					&nf_conntrack_l4proto_generic);
> +
> +	pn->users--;
>  	nf_ct_l4proto_unregister_sysctl(net,
> +					pn,
>  					&nf_conntrack_l4proto_generic);
>  	if (net == &init_net) {
>  		/* free l3proto protocol tables */
> -- 
> 1.7.7.6
> 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gao feng - June 26, 2012, 3:58 a.m.
Hi Pablo:
于 2012年06月25日 19:20, Pablo Neira Ayuso 写道:
> On Thu, Jun 21, 2012 at 10:36:41PM +0800, Gao feng wrote:
>> Now, nf_proto_net's users is confusing.
>> we should regard it as the refcount for l4proto's per-net data,
>> because maybe there are two l4protos use the same per-net data.
>>
>> so increment pn->users when nf_conntrack_l4proto_register
>> success, and decrement it for nf_conntrack_l4_unregister case.
>>
>> because nf_conntrack_l3proto_ipv[4|6] don't use the same per-net
>> data,so we don't need to add a refcnt for their per-net data.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>>  net/netfilter/nf_conntrack_proto.c |   76 ++++++++++++++++++++++--------------
>>  1 files changed, 46 insertions(+), 30 deletions(-)
>>
>> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
>> index 9d6b6ab..63612e6 100644
>> --- a/net/netfilter/nf_conntrack_proto.c
>> +++ b/net/netfilter/nf_conntrack_proto.c
> [...]
>> @@ -458,23 +446,32 @@ int nf_conntrack_l4proto_register(struct net *net,
>>  				  struct nf_conntrack_l4proto *l4proto)
>>  {
>>  	int ret = 0;
>> +	struct nf_proto_net *pn = NULL;
>>  
>>  	if (l4proto->init_net) {
>>  		ret = l4proto->init_net(net, l4proto->l3proto);
>>  		if (ret < 0)
>> -			return ret;
>> +			goto out;
>>  	}
>>  
>> -	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
>> +	pn = nf_ct_l4proto_net(net, l4proto);
>> +	if (pn == NULL)
>> +		goto out;
> 
> Same thing here, we're leaking memory allocated by l4proto->init_net.

if pn is NULL,init_net can't allocate memory for pn->ctl_table.
So I think it's not memory leak here.

> 
>> +	ret = nf_ct_l4proto_register_sysctl(net, pn, l4proto);
>>  	if (ret < 0)
>> -		return ret;
>> +		goto out;
>>  
>>  	if (net == &init_net) {
>>  		ret = nf_conntrack_l4proto_register_net(l4proto);
>> -		if (ret < 0)
>> -			nf_ct_l4proto_unregister_sysctl(net, l4proto);
>> +		if (ret < 0) {
>> +			nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
>> +			goto out;
> 
> Better replace the two lines above by:
> 
> goto out_register_net;
> 
> and then...
> 
>> +		}
>>  	}
>>  
>> +	pn->users++;
> 
> out_register_net:
>         nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
> 
>> +out:
>>  	return ret;
> 
> I think that this change is similar to patch 1/1, I think you should
> send it as a separated patch.
> 

Yes, It looks better.
should I change this and rebase whole patchset or
maybe you just apply this patchset and then I send a cleanup patch to do this?

>>  }
>>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
>> @@ -499,10 +496,18 @@ nf_conntrack_l4proto_unregister_net(struct nf_conntrack_l4proto *l4proto)
>>  void nf_conntrack_l4proto_unregister(struct net *net,
>>  				     struct nf_conntrack_l4proto *l4proto)
>>  {
>> +	struct nf_proto_net *pn = NULL;
>> +
>>  	if (net == &init_net)
>>  		nf_conntrack_l4proto_unregister_net(l4proto);
>>  
>> -	nf_ct_l4proto_unregister_sysctl(net, l4proto);
>> +	pn = nf_ct_l4proto_net(net, l4proto);
>> +	if (pn == NULL)
>> +		return;
>> +
>> +	pn->users--;
>> +	nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
>> +
>>  	/* Remove all contrack entries for this protocol */
>>  	rtnl_lock();
>>  	nf_ct_iterate_cleanup(net, kill_l4proto, l4proto);
>> @@ -514,11 +519,15 @@ int nf_conntrack_proto_init(struct net *net)
>>  {
>>  	unsigned int i;
>>  	int err;
>> +	struct nf_proto_net *pn = nf_ct_l4proto_net(net,
>> +					&nf_conntrack_l4proto_generic);
>> +
>>  	err = nf_conntrack_l4proto_generic.init_net(net,
>>  					nf_conntrack_l4proto_generic.l3proto);
>>  	if (err < 0)
>>  		return err;
>>  	err = nf_ct_l4proto_register_sysctl(net,
>> +					    pn,
>>  					    &nf_conntrack_l4proto_generic);
>>  	if (err < 0)
>>  		return err;
>> @@ -528,13 +537,20 @@ int nf_conntrack_proto_init(struct net *net)
>>  			rcu_assign_pointer(nf_ct_l3protos[i],
>>  					   &nf_conntrack_l3proto_generic);
>>  	}
>> +
>> +	pn->users++;
>>  	return 0;
>>  }
>>  
>>  void nf_conntrack_proto_fini(struct net *net)
>>  {
>>  	unsigned int i;
>> +	struct nf_proto_net *pn = nf_ct_l4proto_net(net,
>> +					&nf_conntrack_l4proto_generic);
>> +
>> +	pn->users--;
>>  	nf_ct_l4proto_unregister_sysctl(net,
>> +					pn,
>>  					&nf_conntrack_l4proto_generic);
>>  	if (net == &init_net) {
>>  		/* free l3proto protocol tables */
>> -- 
>> 1.7.7.6
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - June 26, 2012, 2:47 p.m.
On Tue, Jun 26, 2012 at 11:58:45AM +0800, Gao feng wrote:
> Hi Pablo:
> 于 2012年06月25日 19:20, Pablo Neira Ayuso 写道:
> > On Thu, Jun 21, 2012 at 10:36:41PM +0800, Gao feng wrote:
> >> Now, nf_proto_net's users is confusing.
> >> we should regard it as the refcount for l4proto's per-net data,
> >> because maybe there are two l4protos use the same per-net data.
> >>
> >> so increment pn->users when nf_conntrack_l4proto_register
> >> success, and decrement it for nf_conntrack_l4_unregister case.
> >>
> >> because nf_conntrack_l3proto_ipv[4|6] don't use the same per-net
> >> data,so we don't need to add a refcnt for their per-net data.
> >>
> >> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> >> ---
> >>  net/netfilter/nf_conntrack_proto.c |   76 ++++++++++++++++++++++--------------
> >>  1 files changed, 46 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> >> index 9d6b6ab..63612e6 100644
> >> --- a/net/netfilter/nf_conntrack_proto.c
> >> +++ b/net/netfilter/nf_conntrack_proto.c
> > [...]
> >> @@ -458,23 +446,32 @@ int nf_conntrack_l4proto_register(struct net *net,
> >>  				  struct nf_conntrack_l4proto *l4proto)
> >>  {
> >>  	int ret = 0;
> >> +	struct nf_proto_net *pn = NULL;
> >>  
> >>  	if (l4proto->init_net) {
> >>  		ret = l4proto->init_net(net, l4proto->l3proto);
> >>  		if (ret < 0)
> >> -			return ret;
> >> +			goto out;
> >>  	}
> >>  
> >> -	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
> >> +	pn = nf_ct_l4proto_net(net, l4proto);
> >> +	if (pn == NULL)
> >> +		goto out;
> > 
> > Same thing here, we're leaking memory allocated by l4proto->init_net.
> 
> if pn is NULL,init_net can't allocate memory for pn->ctl_table.
> So I think it's not memory leak here.

Sorry, I meant to say the line below. But we've already clarified
this in patch 1/1.

> >> +	ret = nf_ct_l4proto_register_sysctl(net, pn, l4proto);
> >>  	if (ret < 0)
> >> -		return ret;
> >> +		goto out;
> >>  
> >>  	if (net == &init_net) {
> >>  		ret = nf_conntrack_l4proto_register_net(l4proto);
> >> -		if (ret < 0)
> >> -			nf_ct_l4proto_unregister_sysctl(net, l4proto);
> >> +		if (ret < 0) {
> >> +			nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
> >> +			goto out;
> > 
> > Better replace the two lines above by:
> > 
> > goto out_register_net;
> > 
> > and then...
> > 
> >> +		}
> >>  	}
> >>  
> >> +	pn->users++;
> > 
> > out_register_net:
> >         nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
> > 
> >> +out:
> >>  	return ret;
> > 
> > I think that this change is similar to patch 1/1, I think you should
> > send it as a separated patch.
> > 
> 
> Yes, It looks better.
> should I change this and rebase whole patchset or
> maybe you just apply this patchset and then I send a cleanup patch to do this?

This patch includes changes that are not included in the description,
so you have two choices:

1) You resend me this patch with appropriate description (including
the fact that you're fixing the same thing that patch 1/1 does). This
option still I don't like too much, since making two different things
in one single patch is nasty, but well if you push me...

2) you split the patch in two, with the appropriate descriptions each
and you'll make me happy.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gao feng - June 27, 2012, 1:34 a.m.
于 2012年06月26日 22:47, Pablo Neira Ayuso 写道:
> On Tue, Jun 26, 2012 at 11:58:45AM +0800, Gao feng wrote:
>> Hi Pablo:
>> 于 2012年06月25日 19:20, Pablo Neira Ayuso 写道:
>>> On Thu, Jun 21, 2012 at 10:36:41PM +0800, Gao feng wrote:
>>>> Now, nf_proto_net's users is confusing.
>>>> we should regard it as the refcount for l4proto's per-net data,
>>>> because maybe there are two l4protos use the same per-net data.
>>>>
>>>> so increment pn->users when nf_conntrack_l4proto_register
>>>> success, and decrement it for nf_conntrack_l4_unregister case.
>>>>
>>>> because nf_conntrack_l3proto_ipv[4|6] don't use the same per-net
>>>> data,so we don't need to add a refcnt for their per-net data.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>>> ---
>>>>  net/netfilter/nf_conntrack_proto.c |   76 ++++++++++++++++++++++--------------
>>>>  1 files changed, 46 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
>>>> index 9d6b6ab..63612e6 100644
>>>> --- a/net/netfilter/nf_conntrack_proto.c
>>>> +++ b/net/netfilter/nf_conntrack_proto.c
>>> [...]
>>>> @@ -458,23 +446,32 @@ int nf_conntrack_l4proto_register(struct net *net,
>>>>  				  struct nf_conntrack_l4proto *l4proto)
>>>>  {
>>>>  	int ret = 0;
>>>> +	struct nf_proto_net *pn = NULL;
>>>>  
>>>>  	if (l4proto->init_net) {
>>>>  		ret = l4proto->init_net(net, l4proto->l3proto);
>>>>  		if (ret < 0)
>>>> -			return ret;
>>>> +			goto out;
>>>>  	}
>>>>  
>>>> -	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
>>>> +	pn = nf_ct_l4proto_net(net, l4proto);
>>>> +	if (pn == NULL)
>>>> +		goto out;
>>>
>>> Same thing here, we're leaking memory allocated by l4proto->init_net.
>>
>> if pn is NULL,init_net can't allocate memory for pn->ctl_table.
>> So I think it's not memory leak here.
> 
> Sorry, I meant to say the line below. But we've already clarified
> this in patch 1/1.
> 
>>>> +	ret = nf_ct_l4proto_register_sysctl(net, pn, l4proto);
>>>>  	if (ret < 0)
>>>> -		return ret;
>>>> +		goto out;
>>>>  
>>>>  	if (net == &init_net) {
>>>>  		ret = nf_conntrack_l4proto_register_net(l4proto);
>>>> -		if (ret < 0)
>>>> -			nf_ct_l4proto_unregister_sysctl(net, l4proto);
>>>> +		if (ret < 0) {
>>>> +			nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
>>>> +			goto out;
>>>
>>> Better replace the two lines above by:
>>>
>>> goto out_register_net;
>>>
>>> and then...
>>>
>>>> +		}
>>>>  	}
>>>>  
>>>> +	pn->users++;
>>>
>>> out_register_net:
>>>         nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
>>>
>>>> +out:
>>>>  	return ret;
>>>
>>> I think that this change is similar to patch 1/1, I think you should
>>> send it as a separated patch.
>>>
>>
>> Yes, It looks better.
>> should I change this and rebase whole patchset or
>> maybe you just apply this patchset and then I send a cleanup patch to do this?
> 
> This patch includes changes that are not included in the description,
> so you have two choices:
> 
> 1) You resend me this patch with appropriate description (including
> the fact that you're fixing the same thing that patch 1/1 does). This
> option still I don't like too much, since making two different things
> in one single patch is nasty, but well if you push me...

Sorry, I don't know which the same thing I fixed in this patch and 1/13 patch.
the 1/13 patch only change the proto's registration order. and this patch doesn't
change the registration order.

This patch is used to try to make variable "users" clear.

> 
> 2) you split the patch in two, with the appropriate descriptions each
> and you'll make me happy.
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - June 27, 2012, 9:05 a.m.
On Wed, Jun 27, 2012 at 09:34:01AM +0800, Gao feng wrote:
[...]
> >>> I think that this change is similar to patch 1/1, I think you should
> >>> send it as a separated patch.
> >>>
> >>
> >> Yes, It looks better.
> >> should I change this and rebase whole patchset or
> >> maybe you just apply this patchset and then I send a cleanup patch to do this?
> > 
> > This patch includes changes that are not included in the description,
> > so you have two choices:
> > 
> > 1) You resend me this patch with appropriate description (including
> > the fact that you're fixing the same thing that patch 1/1 does). This
> > option still I don't like too much, since making two different things
> > in one single patch is nasty, but well if you push me...
> 
> Sorry, I don't know which the same thing I fixed in this patch and 1/13 patch.
> the 1/13 patch only change the proto's registration order. and this patch doesn't
> change the registration order.
> 
> This patch is used to try to make variable "users" clear.

Never mind, I'll take the patchset. Thanks Gao.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 9d6b6ab..63612e6 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -39,16 +39,13 @@  static int
 nf_ct_register_sysctl(struct net *net,
 		      struct ctl_table_header **header,
 		      const char *path,
-		      struct ctl_table *table,
-		      unsigned int *users)
+		      struct ctl_table *table)
 {
 	if (*header == NULL) {
 		*header = register_net_sysctl(net, path, table);
 		if (*header == NULL)
 			return -ENOMEM;
 	}
-	if (users != NULL)
-		(*users)++;
 
 	return 0;
 }
@@ -56,9 +53,9 @@  nf_ct_register_sysctl(struct net *net,
 static void
 nf_ct_unregister_sysctl(struct ctl_table_header **header,
 			struct ctl_table **table,
-			unsigned int *users)
+			unsigned int users)
 {
-	if (users != NULL && --*users > 0)
+	if (users > 0)
 		return;
 
 	unregister_net_sysctl_table(*header);
@@ -191,8 +188,7 @@  static int nf_ct_l3proto_register_sysctl(struct net *net,
 		err = nf_ct_register_sysctl(net,
 					    &in->ctl_table_header,
 					    l3proto->ctl_table_path,
-					    in->ctl_table,
-					    NULL);
+					    in->ctl_table);
 		if (err < 0) {
 			kfree(in->ctl_table);
 			in->ctl_table = NULL;
@@ -213,7 +209,7 @@  static void nf_ct_l3proto_unregister_sysctl(struct net *net,
 	if (in->ctl_table_header != NULL)
 		nf_ct_unregister_sysctl(&in->ctl_table_header,
 					&in->ctl_table,
-					NULL);
+					0);
 #endif
 }
 
@@ -329,20 +325,17 @@  static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,
 
 static
 int nf_ct_l4proto_register_sysctl(struct net *net,
+				  struct nf_proto_net *pn,
 				  struct nf_conntrack_l4proto *l4proto)
 {
 	int err = 0;
-	struct nf_proto_net *pn = nf_ct_l4proto_net(net, l4proto);
-	if (pn == NULL)
-		return 0;
 
 #ifdef CONFIG_SYSCTL
 	if (pn->ctl_table != NULL) {
 		err = nf_ct_register_sysctl(net,
 					    &pn->ctl_table_header,
 					    "net/netfilter",
-					    pn->ctl_table,
-					    &pn->users);
+					    pn->ctl_table);
 		if (err < 0) {
 			if (!pn->users) {
 				kfree(pn->ctl_table);
@@ -356,15 +349,14 @@  int nf_ct_l4proto_register_sysctl(struct net *net,
 		err = nf_ct_register_sysctl(net,
 					    &pn->ctl_compat_header,
 					    "net/ipv4/netfilter",
-					    pn->ctl_compat_table,
-					    NULL);
+					    pn->ctl_compat_table);
 		if (err == 0)
 			goto out;
 
 		nf_ct_kfree_compat_sysctl_table(pn);
 		nf_ct_unregister_sysctl(&pn->ctl_table_header,
 					&pn->ctl_table,
-					&pn->users);
+					pn->users);
 	}
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
 out:
@@ -374,25 +366,21 @@  out:
 
 static
 void nf_ct_l4proto_unregister_sysctl(struct net *net,
+				     struct nf_proto_net *pn,
 				     struct nf_conntrack_l4proto *l4proto)
 {
-	struct nf_proto_net *pn = nf_ct_l4proto_net(net, l4proto);
-	if (pn == NULL)
-		return;
 #ifdef CONFIG_SYSCTL
 	if (pn->ctl_table_header != NULL)
 		nf_ct_unregister_sysctl(&pn->ctl_table_header,
 					&pn->ctl_table,
-					&pn->users);
+					pn->users);
 
 #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
 	if (l4proto->l3proto != AF_INET6 && pn->ctl_compat_header != NULL)
 		nf_ct_unregister_sysctl(&pn->ctl_compat_header,
 					&pn->ctl_compat_table,
-					NULL);
+					0);
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
-#else
-	pn->users--;
 #endif /* CONFIG_SYSCTL */
 }
 
@@ -458,23 +446,32 @@  int nf_conntrack_l4proto_register(struct net *net,
 				  struct nf_conntrack_l4proto *l4proto)
 {
 	int ret = 0;
+	struct nf_proto_net *pn = NULL;
 
 	if (l4proto->init_net) {
 		ret = l4proto->init_net(net, l4proto->l3proto);
 		if (ret < 0)
-			return ret;
+			goto out;
 	}
 
-	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
+	pn = nf_ct_l4proto_net(net, l4proto);
+	if (pn == NULL)
+		goto out;
+
+	ret = nf_ct_l4proto_register_sysctl(net, pn, l4proto);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	if (net == &init_net) {
 		ret = nf_conntrack_l4proto_register_net(l4proto);
-		if (ret < 0)
-			nf_ct_l4proto_unregister_sysctl(net, l4proto);
+		if (ret < 0) {
+			nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
+			goto out;
+		}
 	}
 
+	pn->users++;
+out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
@@ -499,10 +496,18 @@  nf_conntrack_l4proto_unregister_net(struct nf_conntrack_l4proto *l4proto)
 void nf_conntrack_l4proto_unregister(struct net *net,
 				     struct nf_conntrack_l4proto *l4proto)
 {
+	struct nf_proto_net *pn = NULL;
+
 	if (net == &init_net)
 		nf_conntrack_l4proto_unregister_net(l4proto);
 
-	nf_ct_l4proto_unregister_sysctl(net, l4proto);
+	pn = nf_ct_l4proto_net(net, l4proto);
+	if (pn == NULL)
+		return;
+
+	pn->users--;
+	nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
+
 	/* Remove all contrack entries for this protocol */
 	rtnl_lock();
 	nf_ct_iterate_cleanup(net, kill_l4proto, l4proto);
@@ -514,11 +519,15 @@  int nf_conntrack_proto_init(struct net *net)
 {
 	unsigned int i;
 	int err;
+	struct nf_proto_net *pn = nf_ct_l4proto_net(net,
+					&nf_conntrack_l4proto_generic);
+
 	err = nf_conntrack_l4proto_generic.init_net(net,
 					nf_conntrack_l4proto_generic.l3proto);
 	if (err < 0)
 		return err;
 	err = nf_ct_l4proto_register_sysctl(net,
+					    pn,
 					    &nf_conntrack_l4proto_generic);
 	if (err < 0)
 		return err;
@@ -528,13 +537,20 @@  int nf_conntrack_proto_init(struct net *net)
 			rcu_assign_pointer(nf_ct_l3protos[i],
 					   &nf_conntrack_l3proto_generic);
 	}
+
+	pn->users++;
 	return 0;
 }
 
 void nf_conntrack_proto_fini(struct net *net)
 {
 	unsigned int i;
+	struct nf_proto_net *pn = nf_ct_l4proto_net(net,
+					&nf_conntrack_l4proto_generic);
+
+	pn->users--;
 	nf_ct_l4proto_unregister_sysctl(net,
+					pn,
 					&nf_conntrack_l4proto_generic);
 	if (net == &init_net) {
 		/* free l3proto protocol tables */