[PATCH net-next v2 01/12] netfilter: fix problem with proto register

[PATCH net-next v2 01/12] netfilter: fix problem with proto register

[Gao feng]

commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448
(netfilter: nf_conntrack: prepare namespace support for
l4 protocol trackers) register proto before register sysctl.

it changes the behavior that when register sysctl failed, the
proto should not be registered too.

so change to register sysctl before register protos.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/netfilter/nf_conntrack_proto.c |   37 ++++++++++++++++++++++-------------
 1 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 1ea9194..a434dd7 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -253,18 +253,23 @@ int nf_conntrack_l3proto_register(struct net *net,
 {
 	int ret = 0;
 
-	if (net == &init_net)
-		ret = nf_conntrack_l3proto_register_net(proto);
+	if (proto->init_net) {
+		ret = proto->init_net(net);
+		if (ret < 0)
+			return ret;
+	}
 
+	ret = nf_ct_l3proto_register_sysctl(net, proto);
 	if (ret < 0)
 		return ret;
 
-	if (proto->init_net) {
-		ret = proto->init_net(net);
+	if (net == &init_net) {
+		ret = nf_conntrack_l3proto_register_net(proto);
 		if (ret < 0)
-			return ret;
+			nf_ct_l3proto_unregister_sysctl(net, proto);
 	}
-	return nf_ct_l3proto_register_sysctl(net, proto);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_l3proto_register);
 
@@ -454,19 +459,23 @@ int nf_conntrack_l4proto_register(struct net *net,
 				  struct nf_conntrack_l4proto *l4proto)
 {
 	int ret = 0;
-	if (net == &init_net)
-		ret = nf_conntrack_l4proto_register_net(l4proto);
-
-	if (ret < 0)
-		return ret;
-
-	if (l4proto->init_net)
+	if (l4proto->init_net) {
 		ret = l4proto->init_net(net);
+		if (ret < 0)
+			return ret;
+	}
 
+	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
 	if (ret < 0)
 		return ret;
 
-	return nf_ct_l4proto_register_sysctl(net, l4proto);
+	if (net == &init_net) {
+		ret = nf_conntrack_l4proto_register_net(l4proto);
+		if (ret < 0)
+			nf_ct_l4proto_unregister_sysctl(net, l4proto);
+	}
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
 
-- 
1.7.7.6

[PATCH net-next v2 02/12] netfilter: add parameter proto for l4proto.init_net

[Gao feng]

there are redundancy codes in l4proto's init_net functions.
we can use one init_net function and l3proto to impletment
the same thing.

So we should add l3proto as a parameter for init_net function.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/netfilter/nf_conntrack_l4proto.h   |    2 +-
 net/ipv4/netfilter/nf_conntrack_proto_icmp.c   |    2 +-
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |    2 +-
 net/netfilter/nf_conntrack_proto.c             |    5 +++--
 net/netfilter/nf_conntrack_proto_dccp.c        |    2 +-
 net/netfilter/nf_conntrack_proto_generic.c     |    2 +-
 net/netfilter/nf_conntrack_proto_gre.c         |    2 +-
 net/netfilter/nf_conntrack_proto_sctp.c        |    4 ++--
 net/netfilter/nf_conntrack_proto_tcp.c         |    4 ++--
 net/netfilter/nf_conntrack_proto_udp.c         |    4 ++--
 net/netfilter/nf_conntrack_proto_udplite.c     |    2 +-
 11 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 81c52b5..5dd60f2 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -97,7 +97,7 @@ struct nf_conntrack_l4proto {
 #endif
 	int	*net_id;
 	/* Init l4proto pernet data */
-	int (*init_net)(struct net *net);
+	int (*init_net)(struct net *net, u_int16_t proto);
 
 	/* Protocol name */
 	const char *name;
diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
index 041923c..76f7a2f 100644
--- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
@@ -337,7 +337,7 @@ static struct ctl_table icmp_compat_sysctl_table[] = {
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
 #endif /* CONFIG_SYSCTL */
 
-static int icmp_init_net(struct net *net)
+static int icmp_init_net(struct net *net, u_int16_t proto)
 {
 	struct nf_icmp_net *in = icmp_pernet(net);
 	struct nf_proto_net *pn = (struct nf_proto_net *)in;
diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 63ed012..807ae09 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -333,7 +333,7 @@ static struct ctl_table icmpv6_sysctl_table[] = {
 };
 #endif /* CONFIG_SYSCTL */
 
-static int icmpv6_init_net(struct net *net)
+static int icmpv6_init_net(struct net *net, u_int16_t proto)
 {
 	struct nf_icmp_net *in = icmpv6_pernet(net);
 	struct nf_proto_net *pn = (struct nf_proto_net *)in;
diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index a434dd7..5ea2d71 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -460,7 +460,7 @@ int nf_conntrack_l4proto_register(struct net *net,
 {
 	int ret = 0;
 	if (l4proto->init_net) {
-		ret = l4proto->init_net(net);
+		ret = l4proto->init_net(net, l4proto->l3proto);
 		if (ret < 0)
 			return ret;
 	}
@@ -514,7 +514,8 @@ int nf_conntrack_proto_init(struct net *net)
 {
 	unsigned int i;
 	int err;
-	err = nf_conntrack_l4proto_generic.init_net(net);
+	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,
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index c33f76a..52da8f0 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -815,7 +815,7 @@ static struct ctl_table dccp_sysctl_table[] = {
 };
 #endif /* CONFIG_SYSCTL */
 
-static int dccp_init_net(struct net *net)
+static int dccp_init_net(struct net *net, u_int16_t proto)
 {
 	struct dccp_net *dn = dccp_pernet(net);
 	struct nf_proto_net *pn = (struct nf_proto_net *)dn;
diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
index bb0e74f..d1ed7b4 100644
--- a/net/netfilter/nf_conntrack_proto_generic.c
+++ b/net/netfilter/nf_conntrack_proto_generic.c
@@ -135,7 +135,7 @@ static struct ctl_table generic_compat_sysctl_table[] = {
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
 #endif /* CONFIG_SYSCTL */
 
-static int generic_init_net(struct net *net)
+static int generic_init_net(struct net *net, u_int16_t proto)
 {
 	struct nf_generic_net *gn = generic_pernet(net);
 	struct nf_proto_net *pn = (struct nf_proto_net *)gn;
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index 25ba5a2..851b93b 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -348,7 +348,7 @@ gre_timeout_nla_policy[CTA_TIMEOUT_GRE_MAX+1] = {
 };
 #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
 
-static int gre_init_net(struct net *net)
+static int gre_init_net(struct net *net, u_int16_t proto)
 {
 	struct netns_proto_gre *net_gre = gre_pernet(net);
 	int i;
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 8fb0582..1e7836c 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -767,7 +767,7 @@ static int sctp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
 	return 0;
 }
 
-static int sctpv4_init_net(struct net *net)
+static int sctpv4_init_net(struct net *net, u_int16_t proto)
 {
 	int ret;
 	struct sctp_net *sn = sctp_pernet(net);
@@ -793,7 +793,7 @@ static int sctpv4_init_net(struct net *net)
 	return ret;
 }
 
-static int sctpv6_init_net(struct net *net)
+static int sctpv6_init_net(struct net *net, u_int16_t proto)
 {
 	struct sctp_net *sn = sctp_pernet(net);
 	struct nf_proto_net *pn = (struct nf_proto_net *)sn;
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 99caa13..6db9d3c 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -1593,7 +1593,7 @@ static int tcp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
 	return 0;
 }
 
-static int tcpv4_init_net(struct net *net)
+static int tcpv4_init_net(struct net *net, u_int16_t proto)
 {
 	int i;
 	int ret = 0;
@@ -1631,7 +1631,7 @@ static int tcpv4_init_net(struct net *net)
 	return ret;
 }
 
-static int tcpv6_init_net(struct net *net)
+static int tcpv6_init_net(struct net *net, u_int16_t proto)
 {
 	int i;
 	struct nf_tcp_net *tn = tcp_pernet(net);
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index a83cf93..2b978e6 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -283,7 +283,7 @@ static void udp_init_net_data(struct nf_udp_net *un)
 	}
 }
 
-static int udpv4_init_net(struct net *net)
+static int udpv4_init_net(struct net *net, u_int16_t proto)
 {
 	int ret;
 	struct nf_udp_net *un = udp_pernet(net);
@@ -307,7 +307,7 @@ static int udpv4_init_net(struct net *net)
 	return ret;
 }
 
-static int udpv6_init_net(struct net *net)
+static int udpv6_init_net(struct net *net, u_int16_t proto)
 {
 	struct nf_udp_net *un = udp_pernet(net);
 	struct nf_proto_net *pn = (struct nf_proto_net *)un;
diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index b32e700..d33e511 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -234,7 +234,7 @@ static struct ctl_table udplite_sysctl_table[] = {
 };
 #endif /* CONFIG_SYSCTL */
 
-static int udplite_init_net(struct net *net)
+static int udplite_init_net(struct net *net, u_int16_t proto)
 {
 	int i;
 	struct udplite_net *un = udplite_pernet(net);
-- 
1.7.7.6

[PATCH net-next v2 03/12] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear

[Gao feng]

add function nf_ct_kfree_compat_sysctl_table to kfree l4proto's
compat sysctl table and set the sysctl table point to NULL.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/netfilter/nf_conntrack_l4proto.h |    2 ++
 net/netfilter/nf_conntrack_proto.c           |   12 ++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 5dd60f2..11fcf27 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -124,6 +124,8 @@ extern int nf_conntrack_l4proto_register(struct net *net,
 extern void nf_conntrack_l4proto_unregister(struct net *net,
 					    struct nf_conntrack_l4proto *proto);
 
+extern void nf_ct_kfree_compat_sysctl_table(struct nf_proto_net *pn);
+
 /* Generic netlink helpers */
 extern int nf_ct_port_tuple_to_nlattr(struct sk_buff *skb,
 				      const struct nf_conntrack_tuple *tuple);
diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 5ea2d71..1033ee6 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -327,6 +327,15 @@ static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,
 	return NULL;
 }
 
+void nf_ct_kfree_compat_sysctl_table(struct nf_proto_net *pn)
+{
+#if defined(CONFIG_SYSCTL) && defined(CONFIG_NF_CONNTRACK_PROC_COMPAT)
+	kfree(pn->ctl_compat_table);
+	pn->ctl_compat_table = NULL;
+#endif
+}
+EXPORT_SYMBOL_GPL(nf_ct_kfree_compat_sysctl_table);
+
 static
 int nf_ct_l4proto_register_sysctl(struct net *net,
 				  struct nf_conntrack_l4proto *l4proto)
@@ -361,8 +370,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
 		if (err == 0)
 			goto out;
 
-		kfree(pn->ctl_compat_table);
-		pn->ctl_compat_table = NULL;
+		nf_ct_kfree_compat_sysctl_table(pn);
 		nf_ct_unregister_sysctl(&pn->ctl_table_header,
 					&pn->ctl_table,
 					&pn->users);
-- 
1.7.7.6

Re: [PATCH net-next v2 03/12] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear

[Pablo Neira Ayuso]

On Sat, Jun 16, 2012 at 11:41:14AM +0800, Gao feng wrote:
> add function nf_ct_kfree_compat_sysctl_table to kfree l4proto's
> compat sysctl table and set the sysctl table point to NULL.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  include/net/netfilter/nf_conntrack_l4proto.h |    2 ++
>  net/netfilter/nf_conntrack_proto.c           |   12 ++++++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
> index 5dd60f2..11fcf27 100644
> --- a/include/net/netfilter/nf_conntrack_l4proto.h
> +++ b/include/net/netfilter/nf_conntrack_l4proto.h
> @@ -124,6 +124,8 @@ extern int nf_conntrack_l4proto_register(struct net *net,
>  extern void nf_conntrack_l4proto_unregister(struct net *net,
>  					    struct nf_conntrack_l4proto *proto);
>  
> +extern void nf_ct_kfree_compat_sysctl_table(struct nf_proto_net *pn);
> +
>  /* Generic netlink helpers */
>  extern int nf_ct_port_tuple_to_nlattr(struct sk_buff *skb,
>  				      const struct nf_conntrack_tuple *tuple);
> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> index 5ea2d71..1033ee6 100644
> --- a/net/netfilter/nf_conntrack_proto.c
> +++ b/net/netfilter/nf_conntrack_proto.c
> @@ -327,6 +327,15 @@ static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,
>  	return NULL;
>  }
>  
> +void nf_ct_kfree_compat_sysctl_table(struct nf_proto_net *pn)
> +{
> +#if defined(CONFIG_SYSCTL) && defined(CONFIG_NF_CONNTRACK_PROC_COMPAT)
> +	kfree(pn->ctl_compat_table);
> +	pn->ctl_compat_table = NULL;
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(nf_ct_kfree_compat_sysctl_table);

I know, I told you to export this symbol instead of inlining it. But I
overlooked that this will be an empty symbol if systcl is disabled.

So, please recover the ancient patch that you sent that inlines this.

Sorry.

> +
>  static
>  int nf_ct_l4proto_register_sysctl(struct net *net,
>  				  struct nf_conntrack_l4proto *l4proto)
> @@ -361,8 +370,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
>  		if (err == 0)
>  			goto out;
>  
> -		kfree(pn->ctl_compat_table);
> -		pn->ctl_compat_table = NULL;
> +		nf_ct_kfree_compat_sysctl_table(pn);
>  		nf_ct_unregister_sysctl(&pn->ctl_table_header,
>  					&pn->ctl_table,
>  					&pn->users);
> -- 
> 1.7.7.6
> 

[PATCH net-next v2 04/12] netfilter: regard users as refcount for l4proto's per-net data

[Gao feng]

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 |   71 +++++++++++++++++++++++-------------
 1 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 1033ee6..86dbf9d 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;
 }
@@ -58,7 +55,7 @@ nf_ct_unregister_sysctl(struct ctl_table_header **header,
 			struct ctl_table **table,
 			unsigned int *users)
 {
-	if (users != NULL && --*users > 0)
+	if (users != NULL && *users > 0)
 		return;
 
 	unregister_net_sysctl_table(*header);
@@ -191,8 +188,8 @@ 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;
@@ -338,20 +335,17 @@ EXPORT_SYMBOL_GPL(nf_ct_kfree_compat_sysctl_table);
 
 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);
@@ -365,8 +359,7 @@ 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;
 
@@ -383,11 +376,9 @@ 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,
@@ -400,8 +391,6 @@ void nf_ct_l4proto_unregister_sysctl(struct net *net,
 					&pn->ctl_compat_table,
 					NULL);
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
-#else
-	pn->users--;
 #endif /* CONFIG_SYSCTL */
 }
 
@@ -467,22 +456,33 @@ 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);
@@ -507,10 +507,17 @@ 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);
@@ -522,11 +529,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;
@@ -536,13 +547,23 @@ int nf_conntrack_proto_init(struct net *net)
 			rcu_assign_pointer(nf_ct_l3protos[i],
 					   &nf_conntrack_l3proto_generic);
 	}
+
+	/* increase generic proto's nf_proto_net refcnt */
+	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);
+
+	/* decrease generic proto's nf_proto_net refcnt */
+	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

Re: [PATCH net-next v2 04/12] netfilter: regard users as refcount for l4proto's per-net data

[Pablo Neira Ayuso]

On Sat, Jun 16, 2012 at 11:41:15AM +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 |   71 +++++++++++++++++++++++-------------
>  1 files changed, 46 insertions(+), 25 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> index 1033ee6..86dbf9d 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;
>  }
> @@ -58,7 +55,7 @@ nf_ct_unregister_sysctl(struct ctl_table_header **header,
>  			struct ctl_table **table,
>  			unsigned int *users)
>  {
> -	if (users != NULL && --*users > 0)
> +	if (users != NULL && *users > 0)

We're not decrementing users anymore. Use unsigned int users instead.
Pass 0 for the layer 3 case to emulate the users refcnt.

>  		return;
>  
>  	unregister_net_sysctl_table(*header);
> @@ -191,8 +188,8 @@ 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;
> @@ -338,20 +335,17 @@ EXPORT_SYMBOL_GPL(nf_ct_kfree_compat_sysctl_table);
>  
>  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);
> @@ -365,8 +359,7 @@ 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;
>  
> @@ -383,11 +376,9 @@ 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,
> @@ -400,8 +391,6 @@ void nf_ct_l4proto_unregister_sysctl(struct net *net,
>  					&pn->ctl_compat_table,
>  					NULL);
>  #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
> -#else
> -	pn->users--;
>  #endif /* CONFIG_SYSCTL */
>  }
>  
> @@ -467,22 +456,33 @@ int nf_conntrack_l4proto_register(struct net *net,
>  				  struct nf_conntrack_l4proto *l4proto)
>  {
>  	int ret = 0;
> +

why this extra line above?

> +	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);
> @@ -507,10 +507,17 @@ 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);
> @@ -522,11 +529,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;
> @@ -536,13 +547,23 @@ int nf_conntrack_proto_init(struct net *net)
>  			rcu_assign_pointer(nf_ct_l3protos[i],
>  					   &nf_conntrack_l3proto_generic);
>  	}
> +
> +	/* increase generic proto's nf_proto_net refcnt */
> +	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);
> +
> +	/* decrease generic proto's nf_proto_net refcnt */

I asked you to remove this comment, it's superfluous.

> +	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
> 

[PATCH net-next v2 05/12] netfilter: merge tcpv[4,6]_net_init into tcp_net_init

[Gao feng]

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 |   61 ++++++++-----------------------
 1 files changed, 16 insertions(+), 45 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 6db9d3c..f2dda9e 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;
+	int ret;
 	struct nf_tcp_net *tn = tcp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)tn;
+	struct nf_proto_net *pn = &tn->pn;
 
-#ifdef CONFIG_SYSCTL
-	if (!pn->ctl_table) {
-#else
-	if (!pn->users++) {
-#endif
+	if (!pn->users) {
+		int i;
 		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);
+		if (ret < 0)
+			nf_ct_kfree_compat_sysctl_table(pn);
+	} else
+		ret = tcp_kmemdup_sysctl_table(pn);
 
-	ret = tcp_kmemdup_sysctl_table(pn);
-
-#ifdef CONFIG_SYSCTL
-#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
-	if (ret < 0) {
-		kfree(pn->ctl_compat_table);
-		pn->ctl_compat_table = NULL;
-	}
-#endif
-#endif
 	return ret;
 }
 
-static int tcpv6_init_net(struct net *net, u_int16_t proto)
-{
-	int i;
-	struct nf_tcp_net *tn = tcp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)tn;
-
-#ifdef CONFIG_SYSCTL
-	if (!pn->ctl_table) {
-#else
-	if (!pn->users++) {
-#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;
-	}
-
-	return tcp_kmemdup_sysctl_table(pn);
-}
-
 struct nf_conntrack_l4proto nf_conntrack_l4proto_tcp4 __read_mostly =
 {
 	.l3proto		= PF_INET,
@@ -1684,7 +1655,7 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_tcp4 __read_mostly =
 		.nla_policy	= tcp_timeout_nla_policy,
 	},
 #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
-	.init_net		= tcpv4_init_net,
+	.init_net		= tcp_init_net,
 };
 EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_tcp4);
 
@@ -1720,6 +1691,6 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_tcp6 __read_mostly =
 		.nla_policy	= tcp_timeout_nla_policy,
 	},
 #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
-	.init_net		= tcpv6_init_net,
+	.init_net		= tcp_init_net,
 };
 EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_tcp6);
-- 
1.7.7.6

Re: [PATCH net-next v2 05/12] netfilter: merge tcpv[4,6]_net_init into tcp_net_init

[Pablo Neira Ayuso]

On Sat, Jun 16, 2012 at 11:41:16AM +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 |   61 ++++++++-----------------------
>  1 files changed, 16 insertions(+), 45 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 6db9d3c..f2dda9e 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;
> +	int ret;
>  	struct nf_tcp_net *tn = tcp_pernet(net);
> -	struct nf_proto_net *pn = (struct nf_proto_net *)tn;
> +	struct nf_proto_net *pn = &tn->pn;
>  
> -#ifdef CONFIG_SYSCTL
> -	if (!pn->ctl_table) {
> -#else
> -	if (!pn->users++) {
> -#endif
> +	if (!pn->users) {
> +		int i;

Extra line missing after int i;

>  		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);
> +		if (ret < 0)
> +			nf_ct_kfree_compat_sysctl_table(pn);
> +	} else
> +		ret = tcp_kmemdup_sysctl_table(pn);
>  
> -	ret = tcp_kmemdup_sysctl_table(pn);
> -
> -#ifdef CONFIG_SYSCTL
> -#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> -	if (ret < 0) {
> -		kfree(pn->ctl_compat_table);
> -		pn->ctl_compat_table = NULL;
> -	}
> -#endif
> -#endif
>  	return ret;
>  }
>  
> -static int tcpv6_init_net(struct net *net, u_int16_t proto)
> -{
> -	int i;
> -	struct nf_tcp_net *tn = tcp_pernet(net);
> -	struct nf_proto_net *pn = (struct nf_proto_net *)tn;
> -
> -#ifdef CONFIG_SYSCTL
> -	if (!pn->ctl_table) {
> -#else
> -	if (!pn->users++) {
> -#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;
> -	}
> -
> -	return tcp_kmemdup_sysctl_table(pn);
> -}
> -
>  struct nf_conntrack_l4proto nf_conntrack_l4proto_tcp4 __read_mostly =
>  {
>  	.l3proto		= PF_INET,
> @@ -1684,7 +1655,7 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_tcp4 __read_mostly =
>  		.nla_policy	= tcp_timeout_nla_policy,
>  	},
>  #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
> -	.init_net		= tcpv4_init_net,
> +	.init_net		= tcp_init_net,
>  };
>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_tcp4);
>  
> @@ -1720,6 +1691,6 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_tcp6 __read_mostly =
>  		.nla_policy	= tcp_timeout_nla_policy,
>  	},
>  #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
> -	.init_net		= tcpv6_init_net,
> +	.init_net		= tcp_init_net,
>  };
>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_tcp6);
> -- 
> 1.7.7.6
> 

[PATCH net-next v2 06/12] netfilter: merge udpv[4,6]_net_init into udp_net_init

[Gao feng]

merge udpv4_net_init and udpv6_net_init into udp_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_udp.c |   56 ++++++++++---------------------
 1 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 2b978e6..61bca4f 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
 	return 0;
 }
 
-static void udp_init_net_data(struct nf_udp_net *un)
+static int udp_init_net(struct net *net, u_int16_t proto)
 {
-	int i;
-#ifdef CONFIG_SYSCTL
-	if (!un->pn.ctl_table) {
-#else
-	if (!un->pn.users++) {
-#endif
+	int ret;
+	struct nf_udp_net *un = udp_pernet(net);
+	struct nf_proto_net *pn = &un->pn;
+
+	if (!pn->users) {
+		int i;
 		for (i = 0; i < UDP_CT_MAX; i++)
 			un->timeouts[i] = udp_timeouts[i];
 	}
-}
-
-static int udpv4_init_net(struct net *net, u_int16_t proto)
-{
-	int ret;
-	struct nf_udp_net *un = udp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)un;
 
-	udp_init_net_data(un);
+	if (proto == AF_INET) {
+		ret = udp_kmemdup_compat_sysctl_table(pn);
+		if (ret < 0)
+			return ret;
 
-	ret = udp_kmemdup_compat_sysctl_table(pn);
-	if (ret < 0)
-		return ret;
+		ret = udp_kmemdup_sysctl_table(pn);
+		if (ret < 0)
+			nf_ct_kfree_compat_sysctl_table(pn);
+	} else
+		ret = udp_kmemdup_sysctl_table(pn);
 
-	ret = udp_kmemdup_sysctl_table(pn);
-#ifdef CONFIG_SYSCTL
-#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
-	if (ret < 0) {
-		kfree(pn->ctl_compat_table);
-		pn->ctl_compat_table = NULL;
-	}
-#endif
-#endif
 	return ret;
 }
 
-static int udpv6_init_net(struct net *net, u_int16_t proto)
-{
-	struct nf_udp_net *un = udp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)un;
-
-	udp_init_net_data(un);
-	return udp_kmemdup_sysctl_table(pn);
-}
-
 struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly =
 {
 	.l3proto		= PF_INET,
@@ -343,7 +323,7 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly =
 		.nla_policy	= udp_timeout_nla_policy,
 	},
 #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
-	.init_net		= udpv4_init_net,
+	.init_net		= udp_init_net,
 };
 EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp4);
 
@@ -374,6 +354,6 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp6 __read_mostly =
 		.nla_policy	= udp_timeout_nla_policy,
 	},
 #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
-	.init_net		= udpv6_init_net,
+	.init_net		= udp_init_net,
 };
 EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp6);
-- 
1.7.7.6

Re: [PATCH net-next v2 06/12] netfilter: merge udpv[4,6]_net_init into udp_net_init

[Pablo Neira Ayuso]

On Sat, Jun 16, 2012 at 11:41:17AM +0800, Gao feng wrote:
> merge udpv4_net_init and udpv6_net_init into udp_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_udp.c |   56 ++++++++++---------------------
>  1 files changed, 18 insertions(+), 38 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
> index 2b978e6..61bca4f 100644
> --- a/net/netfilter/nf_conntrack_proto_udp.c
> +++ b/net/netfilter/nf_conntrack_proto_udp.c
> @@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
>  	return 0;
>  }
>
> -static void udp_init_net_data(struct nf_udp_net *un)
> +static int udp_init_net(struct net *net, u_int16_t proto)
>  {
> -	int i;
> -#ifdef CONFIG_SYSCTL
> -	if (!un->pn.ctl_table) {
> -#else
> -	if (!un->pn.users++) {
> -#endif
> +	int ret;
> +	struct nf_udp_net *un = udp_pernet(net);
> +	struct nf_proto_net *pn = &un->pn;
> +
> +	if (!pn->users) {
> +		int i;
>  		for (i = 0; i < UDP_CT_MAX; i++)
>  			un->timeouts[i] = udp_timeouts[i];
>  	}
> -}
> -
> -static int udpv4_init_net(struct net *net, u_int16_t proto)
> -{
> -	int ret;
> -	struct nf_udp_net *un = udp_pernet(net);
> -	struct nf_proto_net *pn = (struct nf_proto_net *)un;
>
> -	udp_init_net_data(un);
> +	if (proto == AF_INET) {

I think we can remove that u_int16_t proto that I proposed to make
something like:

static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
{
#ifdef CONFIG_SYSCTL
#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
        struct nf_udp_net *un = (struct nf_udp_net *)pn;
+
+       if (pn->ctl_compat_table)
+               return 0;
+
        pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table,
                                       sizeof(udp_compat_sysctl_table),
                                       GFP_KERNEL);
        if (!pn->ctl_compat_table)
                return -ENOMEM;

That should be enough to ensure that the compat is registered once. No
matter if it's done by the IPv4 or IPv6 invocation of udp_init_net.

Thus, it will look consistent with udp_kmemdup_sysctl_table.

> +		ret = udp_kmemdup_compat_sysctl_table(pn);
> +		if (ret < 0)
> +			return ret;
>
> -	ret = udp_kmemdup_compat_sysctl_table(pn);
> -	if (ret < 0)
> -		return ret;
> +		ret = udp_kmemdup_sysctl_table(pn);
> +		if (ret < 0)
> +			nf_ct_kfree_compat_sysctl_table(pn);
> +	} else
> +		ret = udp_kmemdup_sysctl_table(pn);
>
> -	ret = udp_kmemdup_sysctl_table(pn);
> -#ifdef CONFIG_SYSCTL
> -#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> -	if (ret < 0) {
> -		kfree(pn->ctl_compat_table);
> -		pn->ctl_compat_table = NULL;
> -	}
> -#endif
> -#endif
>  	return ret;
>  }
>
> -static int udpv6_init_net(struct net *net, u_int16_t proto)
> -{
> -	struct nf_udp_net *un = udp_pernet(net);
> -	struct nf_proto_net *pn = (struct nf_proto_net *)un;
> -
> -	udp_init_net_data(un);
> -	return udp_kmemdup_sysctl_table(pn);
> -}
> -
>  struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly =
>  {
>  	.l3proto		= PF_INET,
> @@ -343,7 +323,7 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly =
>  		.nla_policy	= udp_timeout_nla_policy,
>  	},
>  #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
> -	.init_net		= udpv4_init_net,
> +	.init_net		= udp_init_net,
>  };
>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp4);
>
> @@ -374,6 +354,6 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp6 __read_mostly =
>  		.nla_policy	= udp_timeout_nla_policy,
>  	},
>  #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
> -	.init_net		= udpv6_init_net,
> +	.init_net		= udp_init_net,
>  };
>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp6);
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next v2 06/12] netfilter: merge udpv[4,6]_net_init into udp_net_init

[Gao feng]

于 2012年06月16日 19:22, Pablo Neira Ayuso 写道:
> On Sat, Jun 16, 2012 at 11:41:17AM +0800, Gao feng wrote:
>> merge udpv4_net_init and udpv6_net_init into udp_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_udp.c |   56 ++++++++++---------------------
>>  1 files changed, 18 insertions(+), 38 deletions(-)
>>
>> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
>> index 2b978e6..61bca4f 100644
>> --- a/net/netfilter/nf_conntrack_proto_udp.c
>> +++ b/net/netfilter/nf_conntrack_proto_udp.c
>> @@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
>>  	return 0;
>>  }
>>
>> -static void udp_init_net_data(struct nf_udp_net *un)
>> +static int udp_init_net(struct net *net, u_int16_t proto)
>>  {
>> -	int i;
>> -#ifdef CONFIG_SYSCTL
>> -	if (!un->pn.ctl_table) {
>> -#else
>> -	if (!un->pn.users++) {
>> -#endif
>> +	int ret;
>> +	struct nf_udp_net *un = udp_pernet(net);
>> +	struct nf_proto_net *pn = &un->pn;
>> +
>> +	if (!pn->users) {
>> +		int i;
>>  		for (i = 0; i < UDP_CT_MAX; i++)
>>  			un->timeouts[i] = udp_timeouts[i];
>>  	}
>> -}
>> -
>> -static int udpv4_init_net(struct net *net, u_int16_t proto)
>> -{
>> -	int ret;
>> -	struct nf_udp_net *un = udp_pernet(net);
>> -	struct nf_proto_net *pn = (struct nf_proto_net *)un;
>>
>> -	udp_init_net_data(un);
>> +	if (proto == AF_INET) {
> 
> I think we can remove that u_int16_t proto that I proposed to make
> something like:
> 
> static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
> {
> #ifdef CONFIG_SYSCTL
> #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
>         struct nf_udp_net *un = (struct nf_udp_net *)pn;
> +
> +       if (pn->ctl_compat_table)
> +               return 0;
> +
>         pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table,
>                                        sizeof(udp_compat_sysctl_table),
>                                        GFP_KERNEL);
>         if (!pn->ctl_compat_table)
>                 return -ENOMEM;
> 
> That should be enough to ensure that the compat is registered once. No
> matter if it's done by the IPv4 or IPv6 invocation of udp_init_net.
> 
> Thus, it will look consistent with udp_kmemdup_sysctl_table.


yes, but this will be very terrible to unregister compat sysctl
and free compat sysctl table.

thinking about, we may insmod nf_conntrack_ipv6 only, as your idea,
we will allocate compat_sysctl_table.so we have to free it when
rmmod nf_conntrack_ipv6.

in order to implement it, we have to change the logic of
nf_ct_l4proto_unregister_sysctl and nf_ct_unregister_sysctl. because we
only free the sysctl table when we unregister the proto.

Re: [PATCH net-next v2 06/12] netfilter: merge udpv[4,6]_net_init into udp_net_init

[Gao feng]

于 2012年06月19日 16:08, Gao feng 写道:
> 
> in order to implement it, we have to change the logic of
> nf_ct_l4proto_unregister_sysctl and nf_ct_unregister_sysctl. because we
> only free the sysctl table when we unregister the proto.

I means free the sysctl table when we register sysctl success
(pn->ctl_table_header != NULL).

Re: [PATCH net-next v2 06/12] netfilter: merge udpv[4,6]_net_init into udp_net_init

[Pablo Neira Ayuso]

On Tue, Jun 19, 2012 at 04:08:31PM +0800, Gao feng wrote:
> 于 2012年06月16日 19:22, Pablo Neira Ayuso 写道:
> > On Sat, Jun 16, 2012 at 11:41:17AM +0800, Gao feng wrote:
> >> merge udpv4_net_init and udpv6_net_init into udp_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_udp.c |   56 ++++++++++---------------------
> >>  1 files changed, 18 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
> >> index 2b978e6..61bca4f 100644
> >> --- a/net/netfilter/nf_conntrack_proto_udp.c
> >> +++ b/net/netfilter/nf_conntrack_proto_udp.c
> >> @@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
> >>  	return 0;
> >>  }
> >>
> >> -static void udp_init_net_data(struct nf_udp_net *un)
> >> +static int udp_init_net(struct net *net, u_int16_t proto)
> >>  {
> >> -	int i;
> >> -#ifdef CONFIG_SYSCTL
> >> -	if (!un->pn.ctl_table) {
> >> -#else
> >> -	if (!un->pn.users++) {
> >> -#endif
> >> +	int ret;
> >> +	struct nf_udp_net *un = udp_pernet(net);
> >> +	struct nf_proto_net *pn = &un->pn;
> >> +
> >> +	if (!pn->users) {
> >> +		int i;
> >>  		for (i = 0; i < UDP_CT_MAX; i++)
> >>  			un->timeouts[i] = udp_timeouts[i];
> >>  	}
> >> -}
> >> -
> >> -static int udpv4_init_net(struct net *net, u_int16_t proto)
> >> -{
> >> -	int ret;
> >> -	struct nf_udp_net *un = udp_pernet(net);
> >> -	struct nf_proto_net *pn = (struct nf_proto_net *)un;
> >>
> >> -	udp_init_net_data(un);
> >> +	if (proto == AF_INET) {
> > 
> > I think we can remove that u_int16_t proto that I proposed to make
> > something like:
> > 
> > static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
> > {
> > #ifdef CONFIG_SYSCTL
> > #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> >         struct nf_udp_net *un = (struct nf_udp_net *)pn;
> > +
> > +       if (pn->ctl_compat_table)
> > +               return 0;
> > +
> >         pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table,
> >                                        sizeof(udp_compat_sysctl_table),
> >                                        GFP_KERNEL);
> >         if (!pn->ctl_compat_table)
> >                 return -ENOMEM;
> > 
> > That should be enough to ensure that the compat is registered once. No
> > matter if it's done by the IPv4 or IPv6 invocation of udp_init_net.
> > 
> > Thus, it will look consistent with udp_kmemdup_sysctl_table.
> 
> 
> yes, but this will be very terrible to unregister compat sysctl
> and free compat sysctl table.
> 
> thinking about, we may insmod nf_conntrack_ipv6 only, as your idea,
> we will allocate compat_sysctl_table.so we have to free it when
> rmmod nf_conntrack_ipv6.

You're right.

> in order to implement it, we have to change the logic of
> nf_ct_l4proto_unregister_sysctl and nf_ct_unregister_sysctl. because we
> only free the sysctl table when we unregister the proto.

OK, let's stick to what we have then.

[PATCH net-next v2 07/12] netfilter: nf_conntrack_l4proto_udplite[4,6] cleanup

[Gao feng]

some cleanup for nf_conntrack_l4proto_udplite[4,6],
make codes more clearer and ready for moving the
sysctl code to nf_conntrack_proto_*_sysctl.c to
reduce the ifdef pollution.

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_udplite.c |   43 +++++++++++++++++-----------
 1 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index d33e511..00bb3f6 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -234,29 +234,38 @@ static struct ctl_table udplite_sysctl_table[] = {
 };
 #endif /* CONFIG_SYSCTL */
 
-static int udplite_init_net(struct net *net, u_int16_t proto)
+static int udplite_kmemdup_sysctl_table(struct nf_proto_net *pn)
 {
-	int i;
-	struct udplite_net *un = udplite_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)un;
 #ifdef CONFIG_SYSCTL
-	if (!pn->ctl_table) {
-#else
-	if (!pn->users++) {
+	struct udplite_net *un = (struct udplite_net *)pn;
+
+	if (pn->ctl_table)
+		return 0;
+
+	pn->ctl_table = kmemdup(udplite_sysctl_table,
+				sizeof(udplite_sysctl_table),
+				GFP_KERNEL);
+	if (!pn->ctl_table)
+		return -ENOMEM;
+
+	pn->ctl_table[0].data = &un->timeouts[UDPLITE_CT_UNREPLIED];
+	pn->ctl_table[1].data = &un->timeouts[UDPLITE_CT_REPLIED];
 #endif
+	return 0;
+}
+
+static int udplite_init_net(struct net *net, u_int16_t proto)
+{
+	struct udplite_net *un = udplite_pernet(net);
+	struct nf_proto_net *pn = &un->pn;
+
+	if (!pn->users) {
+		int i;
 		for (i = 0 ; i < UDPLITE_CT_MAX; i++)
 			un->timeouts[i] = udplite_timeouts[i];
-#ifdef CONFIG_SYSCTL
-		pn->ctl_table = kmemdup(udplite_sysctl_table,
-					sizeof(udplite_sysctl_table),
-					GFP_KERNEL);
-		if (!pn->ctl_table)
-			return -ENOMEM;
-		pn->ctl_table[0].data = &un->timeouts[UDPLITE_CT_UNREPLIED];
-		pn->ctl_table[1].data = &un->timeouts[UDPLITE_CT_REPLIED];
-#endif
 	}
-	return 0;
+
+	return udplite_kmemdup_sysctl_table(pn);
 }
 
 static struct nf_conntrack_l4proto nf_conntrack_l4proto_udplite4 __read_mostly =
-- 
1.7.7.6

[PATCH net-next v2 08/12] netfilter: merge sctpv[4,6]_net_init into sctp_net_init

[Gao feng]

merge sctpv4_net_init and sctpv6_net_init into sctp_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_sctp.c |   58 +++++++++---------------------
 1 files changed, 18 insertions(+), 40 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 1e7836c..dfad67e 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -707,19 +707,6 @@ static struct ctl_table sctp_compat_sysctl_table[] = {
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
 #endif
 
-static void sctp_init_net_data(struct sctp_net *sn)
-{
-	int i;
-#ifdef CONFIG_SYSCTL
-	if (!sn->pn.ctl_table) {
-#else
-	if (!sn->pn.users++) {
-#endif
-		for (i = 0; i < SCTP_CONNTRACK_MAX; i++)
-			sn->timeouts[i] = sctp_timeouts[i];
-	}
-}
-
 static int sctp_kmemdup_sysctl_table(struct nf_proto_net *pn)
 {
 #ifdef CONFIG_SYSCTL
@@ -767,41 +754,32 @@ static int sctp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
 	return 0;
 }
 
-static int sctpv4_init_net(struct net *net, u_int16_t proto)
+static int sctp_init_net(struct net *net, u_int16_t proto)
 {
 	int ret;
 	struct sctp_net *sn = sctp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)sn;
+	struct nf_proto_net *pn = &sn->pn;
 
-	sctp_init_net_data(sn);
-
-	ret = sctp_kmemdup_compat_sysctl_table(pn);
-	if (ret < 0)
-		return ret;
+	if (!pn->users) {
+		int i;
+		for (i = 0; i < SCTP_CONNTRACK_MAX; i++)
+			sn->timeouts[i] = sctp_timeouts[i];
+	}
 
-	ret = sctp_kmemdup_sysctl_table(pn);
+	if (proto == AF_INET) {
+		ret = sctp_kmemdup_compat_sysctl_table(pn);
+		if (ret < 0)
+			return ret;
 
-#ifdef CONFIG_SYSCTL
-#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
-	if (ret < 0) {
+		ret = sctp_kmemdup_sysctl_table(pn);
+		if (ret < 0)
+			nf_ct_kfree_compat_sysctl_table(pn);
+	} else
+		ret = sctp_kmemdup_sysctl_table(pn);
 
-		kfree(pn->ctl_compat_table);
-		pn->ctl_compat_table = NULL;
-	}
-#endif
-#endif
 	return ret;
 }
 
-static int sctpv6_init_net(struct net *net, u_int16_t proto)
-{
-	struct sctp_net *sn = sctp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)sn;
-
-	sctp_init_net_data(sn);
-	return sctp_kmemdup_sysctl_table(pn);
-}
-
 static struct nf_conntrack_l4proto nf_conntrack_l4proto_sctp4 __read_mostly = {
 	.l3proto		= PF_INET,
 	.l4proto 		= IPPROTO_SCTP,
@@ -833,7 +811,7 @@ static struct nf_conntrack_l4proto nf_conntrack_l4proto_sctp4 __read_mostly = {
 	},
 #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
 	.net_id			= &sctp_net_id,
-	.init_net		= sctpv4_init_net,
+	.init_net		= sctp_init_net,
 };
 
 static struct nf_conntrack_l4proto nf_conntrack_l4proto_sctp6 __read_mostly = {
@@ -867,7 +845,7 @@ static struct nf_conntrack_l4proto nf_conntrack_l4proto_sctp6 __read_mostly = {
 #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
 #endif
 	.net_id			= &sctp_net_id,
-	.init_net		= sctpv6_init_net,
+	.init_net		= sctp_init_net,
 };
 
 static int sctp_net_init(struct net *net)
-- 
1.7.7.6

[PATCH net-next v2 09/12] netfilter: nf_conntrack_l4proto_generic cleanup

[Gao feng]

some cleanup of nf_conntrack_l4proto_generic,
split the code to make it more clearer.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/netfilter/nf_conntrack_proto_generic.c |   41 ++++++++++++++++++++++-----
 1 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
index d1ed7b4..c8487d1 100644
--- a/net/netfilter/nf_conntrack_proto_generic.c
+++ b/net/netfilter/nf_conntrack_proto_generic.c
@@ -135,34 +135,59 @@ static struct ctl_table generic_compat_sysctl_table[] = {
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
 #endif /* CONFIG_SYSCTL */
 
-static int generic_init_net(struct net *net, u_int16_t proto)
+static int generic_kmemdup_sysctl_table(struct nf_proto_net *pn)
 {
-	struct nf_generic_net *gn = generic_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)gn;
-	gn->timeout = nf_ct_generic_timeout;
 #ifdef CONFIG_SYSCTL
+	struct nf_generic_net *gn = (struct nf_generic_net *)pn;
+
 	pn->ctl_table = kmemdup(generic_sysctl_table,
 				sizeof(generic_sysctl_table),
 				GFP_KERNEL);
 	if (!pn->ctl_table)
 		return -ENOMEM;
+
 	pn->ctl_table[0].data = &gn->timeout;
+#endif
+	return 0;
+}
 
+static int generic_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
+{
+#ifdef CONFIG_SYSCTL
 #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
+	struct nf_generic_net *gn = (struct nf_generic_net *)pn;
+
 	pn->ctl_compat_table = kmemdup(generic_compat_sysctl_table,
 				       sizeof(generic_compat_sysctl_table),
 				       GFP_KERNEL);
-	if (!pn->ctl_compat_table) {
-		kfree(pn->ctl_table);
-		pn->ctl_table = NULL;
+	if (!pn->ctl_compat_table)
 		return -ENOMEM;
-	}
+
 	pn->ctl_compat_table[0].data = &gn->timeout;
 #endif
 #endif
 	return 0;
 }
 
+static int generic_init_net(struct net *net, u_int16_t proto)
+{
+	int ret;
+	struct nf_generic_net *gn = generic_pernet(net);
+	struct nf_proto_net *pn = &gn->pn;
+
+	gn->timeout = nf_ct_generic_timeout;
+
+	ret = generic_kmemdup_compat_sysctl_table(pn);
+	if (ret < 0)
+		return ret;
+
+	ret = generic_kmemdup_sysctl_table(pn);
+	if (ret < 0)
+		nf_ct_kfree_compat_sysctl_table(pn);
+
+	return ret;
+}
+
 struct nf_conntrack_l4proto nf_conntrack_l4proto_generic __read_mostly =
 {
 	.l3proto		= PF_UNSPEC,
-- 
1.7.7.6

Re: [PATCH net-next v2 09/12] netfilter: nf_conntrack_l4proto_generic cleanup

[Pablo Neira Ayuso]

On Sat, Jun 16, 2012 at 11:41:20AM +0800, Gao feng wrote:
> some cleanup of nf_conntrack_l4proto_generic,
> split the code to make it more clearer.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  net/netfilter/nf_conntrack_proto_generic.c |   41 ++++++++++++++++++++++-----
>  1 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
> index d1ed7b4..c8487d1 100644
> --- a/net/netfilter/nf_conntrack_proto_generic.c
> +++ b/net/netfilter/nf_conntrack_proto_generic.c
> @@ -135,34 +135,59 @@ static struct ctl_table generic_compat_sysctl_table[] = {
>  #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
>  #endif /* CONFIG_SYSCTL */
>  
> -static int generic_init_net(struct net *net, u_int16_t proto)
> +static int generic_kmemdup_sysctl_table(struct nf_proto_net *pn)
>  {
> -	struct nf_generic_net *gn = generic_pernet(net);
> -	struct nf_proto_net *pn = (struct nf_proto_net *)gn;
> -	gn->timeout = nf_ct_generic_timeout;
>  #ifdef CONFIG_SYSCTL
> +	struct nf_generic_net *gn = (struct nf_generic_net *)pn;
> +
>  	pn->ctl_table = kmemdup(generic_sysctl_table,
>  				sizeof(generic_sysctl_table),
>  				GFP_KERNEL);
>  	if (!pn->ctl_table)
>  		return -ENOMEM;
> +
>  	pn->ctl_table[0].data = &gn->timeout;
> +#endif
> +	return 0;
> +}
>  
> +static int generic_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
> +{
> +#ifdef CONFIG_SYSCTL
>  #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> +	struct nf_generic_net *gn = (struct nf_generic_net *)pn;
> +
>  	pn->ctl_compat_table = kmemdup(generic_compat_sysctl_table,
>  				       sizeof(generic_compat_sysctl_table),
>  				       GFP_KERNEL);
> -	if (!pn->ctl_compat_table) {
> -		kfree(pn->ctl_table);
> -		pn->ctl_table = NULL;
> +	if (!pn->ctl_compat_table)
>  		return -ENOMEM;
> -	}
> +
>  	pn->ctl_compat_table[0].data = &gn->timeout;
>  #endif
>  #endif
>  	return 0;
>  }
>  
> +static int generic_init_net(struct net *net, u_int16_t proto)
> +{
> +	int ret;
> +	struct nf_generic_net *gn = generic_pernet(net);
> +	struct nf_proto_net *pn = &gn->pn;
> +
> +	gn->timeout = nf_ct_generic_timeout;
> +
> +	ret = generic_kmemdup_compat_sysctl_table(pn);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = generic_kmemdup_sysctl_table(pn);

better pass generic_kmemdup_sysctl(pn, gn) so we can avoid the casting
above in generic_kmemdup_compat_sysctl_table to obtain gn again.

> +	if (ret < 0)
> +		nf_ct_kfree_compat_sysctl_table(pn);
> +
> +	return ret;
> +}
> +
>  struct nf_conntrack_l4proto nf_conntrack_l4proto_generic __read_mostly =
>  {
>  	.l3proto		= PF_UNSPEC,
> -- 
> 1.7.7.6
> 

[PATCH net-next v2 10/12] netfilter: nf_conntrack_l4proto_dccp[4,6] cleanup

[Gao feng]

some cleanup of nf_conntrack_l4proto_dccp[4,6],
make codes more clearer and ready for moving the
sysctl code to nf_conntrack_proto_*_sysctl.c to
reduce the ifdef pollution.

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_dccp.c |   55 +++++++++++++++++-------------
 1 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index 52da8f0..962a145 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -387,7 +387,7 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 /* this module per-net specifics */
 static int dccp_net_id __read_mostly;
 struct dccp_net {
-	struct nf_proto_net np;
+	struct nf_proto_net pn;
 	int dccp_loose;
 	unsigned int dccp_timeout[CT_DCCP_MAX + 1];
 };
@@ -815,16 +815,38 @@ static struct ctl_table dccp_sysctl_table[] = {
 };
 #endif /* CONFIG_SYSCTL */
 
+static int dccp_kmemdup_sysctl_table(struct nf_proto_net *pn)
+{
+#ifdef CONFIG_SYSCTL
+	struct dccp_net *dn = (struct dccp_net *)pn;
+
+	if (pn->ctl_table)
+		return 0;
+
+	pn->ctl_table = kmemdup(dccp_sysctl_table,
+				sizeof(dccp_sysctl_table),
+				GFP_KERNEL);
+	if (!pn->ctl_table)
+		return -ENOMEM;
+
+	pn->ctl_table[0].data = &dn->dccp_timeout[CT_DCCP_REQUEST];
+	pn->ctl_table[1].data = &dn->dccp_timeout[CT_DCCP_RESPOND];
+	pn->ctl_table[2].data = &dn->dccp_timeout[CT_DCCP_PARTOPEN];
+	pn->ctl_table[3].data = &dn->dccp_timeout[CT_DCCP_OPEN];
+	pn->ctl_table[4].data = &dn->dccp_timeout[CT_DCCP_CLOSEREQ];
+	pn->ctl_table[5].data = &dn->dccp_timeout[CT_DCCP_CLOSING];
+	pn->ctl_table[6].data = &dn->dccp_timeout[CT_DCCP_TIMEWAIT];
+	pn->ctl_table[7].data = &dn->dccp_loose;
+#endif
+	return 0;
+}
+
 static int dccp_init_net(struct net *net, u_int16_t proto)
 {
 	struct dccp_net *dn = dccp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)dn;
+	struct nf_proto_net *pn = &dn->pn;
 
-#ifdef CONFIG_SYSCTL
-	if (!pn->ctl_table) {
-#else
-	if (!pn->users++) {
-#endif
+	if (!pn->users) {
 		/* default values */
 		dn->dccp_loose = 1;
 		dn->dccp_timeout[CT_DCCP_REQUEST]	= 2 * DCCP_MSL;
@@ -834,24 +856,9 @@ static int dccp_init_net(struct net *net, u_int16_t proto)
 		dn->dccp_timeout[CT_DCCP_CLOSEREQ]	= 64 * HZ;
 		dn->dccp_timeout[CT_DCCP_CLOSING]	= 64 * HZ;
 		dn->dccp_timeout[CT_DCCP_TIMEWAIT]	= 2 * DCCP_MSL;
-#ifdef CONFIG_SYSCTL
-		pn->ctl_table = kmemdup(dccp_sysctl_table,
-					sizeof(dccp_sysctl_table),
-					GFP_KERNEL);
-		if (!pn->ctl_table)
-			return -ENOMEM;
-
-		pn->ctl_table[0].data = &dn->dccp_timeout[CT_DCCP_REQUEST];
-		pn->ctl_table[1].data = &dn->dccp_timeout[CT_DCCP_RESPOND];
-		pn->ctl_table[2].data = &dn->dccp_timeout[CT_DCCP_PARTOPEN];
-		pn->ctl_table[3].data = &dn->dccp_timeout[CT_DCCP_OPEN];
-		pn->ctl_table[4].data = &dn->dccp_timeout[CT_DCCP_CLOSEREQ];
-		pn->ctl_table[5].data = &dn->dccp_timeout[CT_DCCP_CLOSING];
-		pn->ctl_table[6].data = &dn->dccp_timeout[CT_DCCP_TIMEWAIT];
-		pn->ctl_table[7].data = &dn->dccp_loose;
-#endif
 	}
-	return 0;
+
+	return dccp_kmemdup_sysctl_table(pn);
 }
 
 static struct nf_conntrack_l4proto dccp_proto4 __read_mostly = {
-- 
1.7.7.6

[PATCH net-next v2 11/12] netfilter: nf_conntrack_l4proto_icmp cleanup

[Gao feng]

add two functions icmp_kmemdup_sysctl_table and
icmp_kmemdup_compat_sysctl_table to make codes more
clearer.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/ipv4/netfilter/nf_conntrack_proto_icmp.c |   44 ++++++++++++++++++++-----
 1 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
index 76f7a2f..be7ae4c 100644
--- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
@@ -337,34 +337,60 @@ static struct ctl_table icmp_compat_sysctl_table[] = {
 #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
 #endif /* CONFIG_SYSCTL */
 
-static int icmp_init_net(struct net *net, u_int16_t proto)
+static int icmp_kmemdup_sysctl_table(struct nf_proto_net *pn)
 {
-	struct nf_icmp_net *in = icmp_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)in;
-	in->timeout = nf_ct_icmp_timeout;
-
 #ifdef CONFIG_SYSCTL
+	struct nf_icmp_net *in = (struct nf_icmp_net *)pn;
+
 	pn->ctl_table = kmemdup(icmp_sysctl_table,
 				sizeof(icmp_sysctl_table),
 				GFP_KERNEL);
 	if (!pn->ctl_table)
 		return -ENOMEM;
+
 	pn->ctl_table[0].data = &in->timeout;
+#endif
+	return 0;
+}
+
+static int icmp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
+{
+#ifdef CONFIG_SYSCTL
 #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
+	struct nf_icmp_net *in = (struct nf_icmp_net *)pn;
+
 	pn->ctl_compat_table = kmemdup(icmp_compat_sysctl_table,
 				       sizeof(icmp_compat_sysctl_table),
 				       GFP_KERNEL);
-	if (!pn->ctl_compat_table) {
-		kfree(pn->ctl_table);
-		pn->ctl_table = NULL;
+	if (!pn->ctl_compat_table)
 		return -ENOMEM;
-	}
+
 	pn->ctl_compat_table[0].data = &in->timeout;
 #endif
 #endif
 	return 0;
 }
 
+
+static int icmp_init_net(struct net *net, u_int16_t proto)
+{
+	int ret;
+	struct nf_icmp_net *in = icmp_pernet(net);
+	struct nf_proto_net *pn = &in->pn;
+
+	in->timeout = nf_ct_icmp_timeout;
+
+	ret = icmp_kmemdup_compat_sysctl_table(pn);
+	if (ret < 0)
+		return ret;
+
+	ret = icmp_kmemdup_sysctl_table(pn);
+	if (ret < 0)
+		nf_ct_kfree_compat_sysctl_table(pn);
+
+	return ret;
+}
+
 struct nf_conntrack_l4proto nf_conntrack_l4proto_icmp __read_mostly =
 {
 	.l3proto		= PF_INET,
-- 
1.7.7.6

Re: [PATCH net-next v2 11/12] netfilter: nf_conntrack_l4proto_icmp cleanup

[Pablo Neira Ayuso]

On Sat, Jun 16, 2012 at 11:41:22AM +0800, Gao feng wrote:
> add two functions icmp_kmemdup_sysctl_table and
> icmp_kmemdup_compat_sysctl_table to make codes more
> clearer.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  net/ipv4/netfilter/nf_conntrack_proto_icmp.c |   44 ++++++++++++++++++++-----
>  1 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> index 76f7a2f..be7ae4c 100644
> --- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> +++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> @@ -337,34 +337,60 @@ static struct ctl_table icmp_compat_sysctl_table[] = {
>  #endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
>  #endif /* CONFIG_SYSCTL */
>  
> -static int icmp_init_net(struct net *net, u_int16_t proto)
> +static int icmp_kmemdup_sysctl_table(struct nf_proto_net *pn)
>  {
> -	struct nf_icmp_net *in = icmp_pernet(net);
> -	struct nf_proto_net *pn = (struct nf_proto_net *)in;
> -	in->timeout = nf_ct_icmp_timeout;
> -
>  #ifdef CONFIG_SYSCTL
> +	struct nf_icmp_net *in = (struct nf_icmp_net *)pn;
> +
>  	pn->ctl_table = kmemdup(icmp_sysctl_table,
>  				sizeof(icmp_sysctl_table),
>  				GFP_KERNEL);
>  	if (!pn->ctl_table)
>  		return -ENOMEM;
> +
>  	pn->ctl_table[0].data = &in->timeout;
> +#endif
> +	return 0;
> +}
> +
> +static int icmp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
> +{
> +#ifdef CONFIG_SYSCTL
>  #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> +	struct nf_icmp_net *in = (struct nf_icmp_net *)pn;
> +
>  	pn->ctl_compat_table = kmemdup(icmp_compat_sysctl_table,
>  				       sizeof(icmp_compat_sysctl_table),
>  				       GFP_KERNEL);
> -	if (!pn->ctl_compat_table) {
> -		kfree(pn->ctl_table);
> -		pn->ctl_table = NULL;
> +	if (!pn->ctl_compat_table)
>  		return -ENOMEM;
> -	}
> +
>  	pn->ctl_compat_table[0].data = &in->timeout;
>  #endif
>  #endif
>  	return 0;
>  }
>  
> +

extra line unrequiered.

I'm sorry, I'm stressing a lot on this because I don't like abusing
follow-up patches that clean up extra lines / missing lines and that
sort of nitpicks...

> +static int icmp_init_net(struct net *net, u_int16_t proto)
> +{
> +	int ret;
> +	struct nf_icmp_net *in = icmp_pernet(net);
> +	struct nf_proto_net *pn = &in->pn;
> +
> +	in->timeout = nf_ct_icmp_timeout;
> +
> +	ret = icmp_kmemdup_compat_sysctl_table(pn);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = icmp_kmemdup_sysctl_table(pn);
> +	if (ret < 0)
> +		nf_ct_kfree_compat_sysctl_table(pn);
> +
> +	return ret;
> +}
> +
>  struct nf_conntrack_l4proto nf_conntrack_l4proto_icmp __read_mostly =
>  {
>  	.l3proto		= PF_INET,
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next v2 11/12] netfilter: nf_conntrack_l4proto_icmp cleanup

[Gao feng]

于 2012年06月16日 19:08, Pablo Neira Ayuso 写道:
> extra line unrequiered.
> 
> I'm sorry, I'm stressing a lot on this because I don't like abusing
> follow-up patches that clean up extra lines / missing lines and that
> sort of nitpicks...
> 

Sorry for this,I will double check this when I send the v3 patch.

Thanks.
--
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 net-next v2 12/12] netfilter: nf_conntrack_l4proto_icmpv6 cleanup

[Gao feng]

add function icmpv6_kmemdup_sysctl_table to make codes
more clearer.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 807ae09..5ef2c9d 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -333,22 +333,32 @@ static struct ctl_table icmpv6_sysctl_table[] = {
 };
 #endif /* CONFIG_SYSCTL */
 
-static int icmpv6_init_net(struct net *net, u_int16_t proto)
+static int icmpv6_kmemdup_sysctl_table(struct nf_proto_net *pn)
 {
-	struct nf_icmp_net *in = icmpv6_pernet(net);
-	struct nf_proto_net *pn = (struct nf_proto_net *)in;
-	in->timeout = nf_ct_icmpv6_timeout;
 #ifdef CONFIG_SYSCTL
+	struct nf_icmp_net *in = (struct nf_icmp_net *)pn;
+
 	pn->ctl_table = kmemdup(icmpv6_sysctl_table,
 				sizeof(icmpv6_sysctl_table),
 				GFP_KERNEL);
 	if (!pn->ctl_table)
 		return -ENOMEM;
+
 	pn->ctl_table[0].data = &in->timeout;
 #endif
 	return 0;
 }
 
+static int icmpv6_init_net(struct net *net, u_int16_t proto)
+{
+	struct nf_icmp_net *in = icmpv6_pernet(net);
+	struct nf_proto_net *pn = &in->pn;
+
+	in->timeout = nf_ct_icmpv6_timeout;
+
+	return icmpv6_kmemdup_sysctl_table(pn);
+}
+
 struct nf_conntrack_l4proto nf_conntrack_l4proto_icmpv6 __read_mostly =
 {
 	.l3proto		= PF_INET6,
-- 
1.7.7.6

Re: [PATCH net-next v2 01/12] netfilter: fix problem with proto register

[Pablo Neira Ayuso]

On Sat, Jun 16, 2012 at 11:41:12AM +0800, Gao feng wrote:
> commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448
> (netfilter: nf_conntrack: prepare namespace support for
> l4 protocol trackers) register proto before register sysctl.
> 
> it changes the behavior that when register sysctl failed, the
> proto should not be registered too.
> 
> so change to register sysctl before register protos.

Could you explain why we need to change the order in the registration?
ie. now first proto->init_net then sysctl things.

> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  net/netfilter/nf_conntrack_proto.c |   37 ++++++++++++++++++++++-------------
>  1 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> index 1ea9194..a434dd7 100644
> --- a/net/netfilter/nf_conntrack_proto.c
> +++ b/net/netfilter/nf_conntrack_proto.c
> @@ -253,18 +253,23 @@ int nf_conntrack_l3proto_register(struct net *net,
>  {
>  	int ret = 0;
>  
> -	if (net == &init_net)
> -		ret = nf_conntrack_l3proto_register_net(proto);
> +	if (proto->init_net) {
> +		ret = proto->init_net(net);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
> +	ret = nf_ct_l3proto_register_sysctl(net, proto);
>  	if (ret < 0)
>  		return ret;
>  
> -	if (proto->init_net) {
> -		ret = proto->init_net(net);
> +	if (net == &init_net) {
> +		ret = nf_conntrack_l3proto_register_net(proto);
>  		if (ret < 0)
> -			return ret;
> +			nf_ct_l3proto_unregister_sysctl(net, proto);
>  	}
> -	return nf_ct_l3proto_register_sysctl(net, proto);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(nf_conntrack_l3proto_register);
>  
> @@ -454,19 +459,23 @@ int nf_conntrack_l4proto_register(struct net *net,
>  				  struct nf_conntrack_l4proto *l4proto)
>  {
>  	int ret = 0;
> -	if (net == &init_net)
> -		ret = nf_conntrack_l4proto_register_net(l4proto);
> -
> -	if (ret < 0)
> -		return ret;
> -
> -	if (l4proto->init_net)
> +	if (l4proto->init_net) {
>  		ret = l4proto->init_net(net);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
> +	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
>  	if (ret < 0)
>  		return ret;
>  
> -	return nf_ct_l4proto_register_sysctl(net, l4proto);
> +	if (net == &init_net) {
> +		ret = nf_conntrack_l4proto_register_net(l4proto);
> +		if (ret < 0)
> +			nf_ct_l4proto_unregister_sysctl(net, l4proto);
> +	}
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
>  
> -- 
> 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

Re: [PATCH net-next v2 01/12] netfilter: fix problem with proto register

[Gao feng]

于 2012年06月16日 18:50, Pablo Neira Ayuso 写道:
> On Sat, Jun 16, 2012 at 11:41:12AM +0800, Gao feng wrote:
>> commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448
>> (netfilter: nf_conntrack: prepare namespace support for
>> l4 protocol trackers) register proto before register sysctl.
>>
>> it changes the behavior that when register sysctl failed, the
>> proto should not be registered too.
>>
>> so change to register sysctl before register protos.
> 
> Could you explain why we need to change the order in the registration?
> ie. now first proto->init_net then sysctl things.

before commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448, we register sysctl before
register protos, so if sysctl is registered faild, the protos will not be registered.

but now, we register protos first, and when register sysctl failed, we can use protos
too, it's different from before.

> 
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>>  net/netfilter/nf_conntrack_proto.c |   37 ++++++++++++++++++++++-------------
>>  1 files changed, 23 insertions(+), 14 deletions(-)
>>
>> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
>> index 1ea9194..a434dd7 100644
>> --- a/net/netfilter/nf_conntrack_proto.c
>> +++ b/net/netfilter/nf_conntrack_proto.c
>> @@ -253,18 +253,23 @@ int nf_conntrack_l3proto_register(struct net *net,
>>  {
>>  	int ret = 0;
>>  
>> -	if (net == &init_net)
>> -		ret = nf_conntrack_l3proto_register_net(proto);
>> +	if (proto->init_net) {
>> +		ret = proto->init_net(net);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>>  
>> +	ret = nf_ct_l3proto_register_sysctl(net, proto);
>>  	if (ret < 0)
>>  		return ret;
>>  
>> -	if (proto->init_net) {
>> -		ret = proto->init_net(net);
>> +	if (net == &init_net) {
>> +		ret = nf_conntrack_l3proto_register_net(proto);
>>  		if (ret < 0)
>> -			return ret;
>> +			nf_ct_l3proto_unregister_sysctl(net, proto);
>>  	}
>> -	return nf_ct_l3proto_register_sysctl(net, proto);
>> +
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(nf_conntrack_l3proto_register);
>>  
>> @@ -454,19 +459,23 @@ int nf_conntrack_l4proto_register(struct net *net,
>>  				  struct nf_conntrack_l4proto *l4proto)
>>  {
>>  	int ret = 0;
>> -	if (net == &init_net)
>> -		ret = nf_conntrack_l4proto_register_net(l4proto);
>> -
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	if (l4proto->init_net)
>> +	if (l4proto->init_net) {
>>  		ret = l4proto->init_net(net);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>>  
>> +	ret = nf_ct_l4proto_register_sysctl(net, l4proto);
>>  	if (ret < 0)
>>  		return ret;
>>  
>> -	return nf_ct_l4proto_register_sysctl(net, l4proto);
>> +	if (net == &init_net) {
>> +		ret = nf_conntrack_l4proto_register_net(l4proto);
>> +		if (ret < 0)
>> +			nf_ct_l4proto_unregister_sysctl(net, l4proto);
>> +	}
>> +
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
>>  
>> -- 
>> 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
> --
> 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

Re: [PATCH net-next v2 01/12] netfilter: fix problem with proto register

[Pablo Neira Ayuso]

On Mon, Jun 18, 2012 at 08:59:11AM +0800, Gao feng wrote:
> 于 2012年06月16日 18:50, Pablo Neira Ayuso 写道:
> > On Sat, Jun 16, 2012 at 11:41:12AM +0800, Gao feng wrote:
> >> commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448
> >> (netfilter: nf_conntrack: prepare namespace support for
> >> l4 protocol trackers) register proto before register sysctl.
> >>
> >> it changes the behavior that when register sysctl failed, the
> >> proto should not be registered too.
> >>
> >> so change to register sysctl before register protos.
> > 
> > Could you explain why we need to change the order in the registration?
> > ie. now first proto->init_net then sysctl things.
> 
> before commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448, we register sysctl before
> register protos, so if sysctl is registered faild, the protos will not be registered.
> 
> but now, we register protos first, and when register sysctl failed, we can use protos
> too, it's different from before.

That makes sense.

IMO, this is the thing that should be included in the description.