Re: [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init

[Gao feng <gaofeng@cn.fujitsu.com>]

于 2012年06月15日 19:44, Pablo Neira Ayuso 写道:
> On Thu, Jun 14, 2012 at 06:07:20PM +0800, Gao feng wrote:
>> merge tcpv4_net_init and tcpv6_net_init into tcp_net_init to
>> reduce the redundancy codes.
>>
>> and use nf_proto_net.users to identify if it's the first time
>> we use the nf_proto_net. when it's the first time,we will
>> initialized it.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>>  net/netfilter/nf_conntrack_proto_tcp.c |   57 ++++++++------------------------
>>  1 files changed, 14 insertions(+), 43 deletions(-)
>>
>> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
>> index 6db9d3c..e3d5427 100644
>> --- a/net/netfilter/nf_conntrack_proto_tcp.c
>> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
>> @@ -1593,18 +1593,14 @@ static int tcp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
>>  	return 0;
>>  }
>>  
>> -static int tcpv4_init_net(struct net *net, u_int16_t proto)
>> +static int tcp_init_net(struct net *net, u_int16_t proto)
>>  {
>> -	int i;
>>  	int ret = 0;
>>  	struct nf_tcp_net *tn = tcp_pernet(net);
>>  	struct nf_proto_net *pn = (struct nf_proto_net *)tn;
> 
> while at it, it would be fine if you use &tn->pn instead. I know this
> cast is making the trick, but what I propose is better practise.
> 

OK, I will change it.

>> -#ifdef CONFIG_SYSCTL
>> -	if (!pn->ctl_table) {
>> -#else
>> -	if (!pn->users++) {
>> -#endif
>> +	if (!pn->users) {
>> +		int i = 0;
>>  		for (i = 0; i < TCP_CONNTRACK_TIMEOUT_MAX; i++)
>>  			tn->timeouts[i] = tcp_timeouts[i];
>>  
>> @@ -1613,45 +1609,20 @@ static int tcpv4_init_net(struct net *net, u_int16_t proto)
>>  		tn->tcp_max_retrans = nf_ct_tcp_max_retrans;
>>  	}
>>  
>> -	ret = tcp_kmemdup_compat_sysctl_table(pn);
>> +	if (proto == AF_INET) {
>> +		ret = tcp_kmemdup_compat_sysctl_table(pn);
>> +		if (ret < 0)
>> +			return ret;
>>  
>> -	if (ret < 0)
>> -		return ret;
>> +		ret = tcp_kmemdup_sysctl_table(pn);
> 
> One thing I noticed: This kmemdup will happen twice, once for IPv4 and
> once for IPv6. I think this should happen only once, as both TCP
> tracker for IPv4 and IPv6 are sharing the same nf_proto_net.
> 
> So it should happen inside the if (!pn->users) thing.
> 
> AFAICS, then this should look like the following:
> 
> if (pn->users)
>         return 0;

maybe we register IPv6's l4proto first, it will only kmemdup the sysctl table.
if we return here, when we register Ipv4's l4proto,the compat sysctl table will
not be allocated, so the netfilter will have no compat sysctl entries.

> 
> /*
>  * here comes all per-net initialization
>  */
> 
--
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