netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Gao feng <gaofeng@cn.fujitsu.com>
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	serge.hallyn@canonical.com, ebiederm@xmission.com,
	dlezcano@fr.ibm.com
Subject: Re: [PATCH 05/17] netfilter: add namespace support for l4proto_tcp
Date: Fri, 25 May 2012 05:00:15 +0200	[thread overview]
Message-ID: <20120525030015.GB21076@1984> (raw)
In-Reply-To: <1336985547-31960-6-git-send-email-gaofeng@cn.fujitsu.com>

Hi Gao,

While having a look at this again, I have two new requests:

On Mon, May 14, 2012 at 04:52:15PM +0800, Gao feng wrote:
[...]
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 4dfbfa8..dd19350 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
[...]
> @@ -1549,10 +1532,80 @@ static struct ctl_table tcp_compat_sysctl_table[] = {
>  #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
>  #endif /* CONFIG_SYSCTL */
>
> +static int tcp_init_net(struct net *net, u_int8_t compat)
> +{
> +	int i;
> +	struct nf_tcp_net *tn = tcp_pernet(net);
> +	struct nf_proto_net *pn = (struct nf_proto_net *)tn;
> +#ifdef CONFIG_SYSCTL
> +#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> +	if (compat) {
> +		pn->ctl_compat_table = kmemdup(tcp_compat_sysctl_table,
> +					       sizeof(tcp_compat_sysctl_table),
> +					       GFP_KERNEL);
> +		if (!pn->ctl_compat_table)
> +			return -ENOMEM;
> +
> +		pn->ctl_compat_table[0].data = &tn->timeouts[TCP_CONNTRACK_SYN_SENT];
> +		pn->ctl_compat_table[1].data = &tn->timeouts[TCP_CONNTRACK_SYN_SENT2];
> +		pn->ctl_compat_table[2].data = &tn->timeouts[TCP_CONNTRACK_SYN_RECV];
> +		pn->ctl_compat_table[3].data = &tn->timeouts[TCP_CONNTRACK_ESTABLISHED];
> +		pn->ctl_compat_table[4].data = &tn->timeouts[TCP_CONNTRACK_FIN_WAIT];
> +		pn->ctl_compat_table[5].data = &tn->timeouts[TCP_CONNTRACK_CLOSE_WAIT];
> +		pn->ctl_compat_table[6].data = &tn->timeouts[TCP_CONNTRACK_LAST_ACK];
> +		pn->ctl_compat_table[7].data = &tn->timeouts[TCP_CONNTRACK_TIME_WAIT];
> +		pn->ctl_compat_table[8].data = &tn->timeouts[TCP_CONNTRACK_CLOSE];
> +		pn->ctl_compat_table[9].data = &tn->timeouts[TCP_CONNTRACK_RETRANS];
> +		pn->ctl_compat_table[10].data = &tn->tcp_loose;
> +		pn->ctl_compat_table[11].data = &tn->tcp_be_liberal;
> +		pn->ctl_compat_table[12].data = &tn->tcp_max_retrans;

You can make a generic function to set the ctl_data that you can
reuse for this code above and the one below.

> +	}
> +#endif
> +	if (!pn->ctl_table) {
> +#else
> +	if (!pn->user++) {
> +#endif
> +		for (i = 0; i < TCP_CONNTRACK_TIMEOUT_MAX; i++)
> +			tn->timeouts[i] = tcp_timeouts[i];
> +		tn->tcp_loose = nf_ct_tcp_loose;
> +		tn->tcp_be_liberal = nf_ct_tcp_be_liberal;
> +		tn->tcp_max_retrans = nf_ct_tcp_max_retrans;
> +#ifdef CONFIG_SYSCTL
> +		pn->ctl_table = kmemdup(tcp_sysctl_table,
> +					sizeof(tcp_sysctl_table),
> +					GFP_KERNEL);
> +		if (!pn->ctl_table) {
> +#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> +			if (compat) {
> +				kfree(pn->ctl_compat_table);
> +				pn->ctl_compat_table = NULL;
> +			}
> +#endif
> +			return -ENOMEM;
> +		}
> +		pn->ctl_table[0].data = &tn->timeouts[TCP_CONNTRACK_SYN_SENT];
> +		pn->ctl_table[1].data = &tn->timeouts[TCP_CONNTRACK_SYN_RECV];
> +		pn->ctl_table[2].data = &tn->timeouts[TCP_CONNTRACK_ESTABLISHED];
> +		pn->ctl_table[3].data = &tn->timeouts[TCP_CONNTRACK_FIN_WAIT];
> +		pn->ctl_table[4].data = &tn->timeouts[TCP_CONNTRACK_CLOSE_WAIT];
> +		pn->ctl_table[5].data = &tn->timeouts[TCP_CONNTRACK_LAST_ACK];
> +		pn->ctl_table[6].data = &tn->timeouts[TCP_CONNTRACK_TIME_WAIT];
> +		pn->ctl_table[7].data = &tn->timeouts[TCP_CONNTRACK_CLOSE];
> +		pn->ctl_table[8].data = &tn->timeouts[TCP_CONNTRACK_RETRANS];
> +		pn->ctl_table[9].data = &tn->timeouts[TCP_CONNTRACK_UNACK];
> +		pn->ctl_table[10].data = &tn->tcp_loose;
> +		pn->ctl_table[11].data = &tn->tcp_be_liberal;
> +		pn->ctl_table[12].data = &tn->tcp_max_retrans;
> +#endif

I have bad experience with code that has lots of #ifdef's.

Please, split all *_init_net into smaller functions.

  reply	other threads:[~2012-05-25  3:00 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-14  8:52 [PATCH v3 00/17] netfilter: add namespace support for netfilter protos Gao feng
2012-05-14  8:52 ` [PATCH 01/17] netfilter: add struct nf_proto_net for register l4proto sysctl Gao feng
2012-05-23 10:12   ` Pablo Neira Ayuso
2012-05-24  1:35     ` Gao feng
2012-05-24  9:58       ` Pablo Neira Ayuso
2012-05-24 10:54         ` Gao feng
2012-05-24 14:38           ` Pablo Neira Ayuso
2012-05-25  1:05             ` Gao feng
2012-05-25  2:54               ` Pablo Neira Ayuso
2012-05-25  6:02                 ` Gao feng
2012-05-26  2:28                   ` Gao feng
2012-05-28  9:53                     ` Pablo Neira Ayuso
2012-05-14  8:52 ` [PATCH 02/17] netfilter: add namespace support for l4proto Gao feng
2012-05-23 10:25   ` Pablo Neira Ayuso
2012-05-24  1:52     ` Gao feng
2012-05-24 10:00       ` Pablo Neira Ayuso
2012-05-14  8:52 ` [PATCH 03/17] netfilter: add namespace support for l3proto Gao feng
2012-05-23 10:29   ` Pablo Neira Ayuso
2012-05-24  1:58     ` Gao feng
2012-05-24 10:04       ` Pablo Neira Ayuso
2012-05-24 10:57         ` Gao feng
2012-05-14  8:52 ` [PATCH 04/17] netfilter: add namespace support for l4proto_generic Gao feng
2012-05-23 10:32   ` Pablo Neira Ayuso
2012-05-24  1:13     ` Gao feng
2012-05-24  9:52       ` Pablo Neira Ayuso
2012-05-24 11:07         ` Gao feng
2012-05-24 14:40           ` Pablo Neira Ayuso
2012-05-26  2:36             ` Gao feng
2012-05-28  9:54               ` Pablo Neira Ayuso
2012-05-14  8:52 ` [PATCH 05/17] netfilter: add namespace support for l4proto_tcp Gao feng
2012-05-25  3:00   ` Pablo Neira Ayuso [this message]
2012-05-25  6:05     ` Gao feng
2012-05-14  8:52 ` [PATCH 06/17] netfilter: add namespace support for l4proto_udp Gao feng
2012-05-14  8:52 ` [PATCH 07/17] netfilter: add namespace support for l4proto_icmp Gao feng
2012-05-14  8:52 ` [PATCH 08/17] netfilter: add namespace support for l4proto_icmpv6 Gao feng
2012-05-14  8:52 ` [PATCH 09/17] netfilter: add namespace support for l3proto_ipv4 Gao feng
2012-05-14  8:52 ` [PATCH 10/17] netfilter: add namespace support for l3proto_ipv6 Gao feng
2012-05-14  8:52 ` [PATCH 11/17] netfilter: add namespace support for l4proto_sctp Gao feng
2012-05-14  8:52 ` [PATCH 12/17] netfilter: add namespace support for l4proto_udplite Gao feng
2012-05-14  8:52 ` [PATCH 13/17] netfilter: adjust l4proto_dccp to the nf_conntrack_l4proto_register Gao feng
2012-05-14  8:52 ` [PATCH 14/17] netfilter: adjust l4proto_gre4 " Gao feng
2012-05-14  8:52 ` [PATCH 15/17] netfilter: cleanup sysctl for l4proto and l3proto Gao feng
2012-05-23 10:38   ` Pablo Neira Ayuso
2012-05-24  0:59     ` Gao feng
2012-05-24  9:56       ` Pablo Neira Ayuso
2012-05-14  8:52 ` [PATCH 16/17] netfilter: add namespace support for cttimeout Gao feng
2012-05-23 10:41   ` Pablo Neira Ayuso
2012-05-24  1:04     ` Gao feng
2012-05-14  8:52 ` [PATCH 17/17] netfilter: cttimeout use pernet data of l4proto Gao feng
2012-05-21  8:52 ` [PATCH v3 00/17] netfilter: add namespace support for netfilter protos Gao feng
2012-05-23 10:42 ` Pablo Neira Ayuso
  -- strict thread matches above, loose matches on Subject: below --
2012-04-27  9:37 [PATCH v2 " Gao feng
2012-04-27  9:37 ` [PATCH 05/17] netfilter: add namespace support for l4proto_tcp Gao feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120525030015.GB21076@1984 \
    --to=pablo@netfilter.org \
    --cc=dlezcano@fr.ibm.com \
    --cc=ebiederm@xmission.com \
    --cc=gaofeng@cn.fujitsu.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=serge.hallyn@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).