All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/10] netfilter: fix problem with proto register
@ 2012-06-14 10:07 Gao feng
  2012-06-14 10:07 ` [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net Gao feng
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-14 17:59   ` Pablo Neira Ayuso
  2012-06-14 10:07 ` [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear Gao feng
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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             |    6 ++++--
 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, 17 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..8fc0332 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -193,6 +193,7 @@ static int nf_ct_l3proto_register_sysctl(struct net *net,
 					    l3proto->ctl_table_path,
 					    in->ctl_table,
 					    NULL);
+
 		if (err < 0) {
 			kfree(in->ctl_table);
 			in->ctl_table = NULL;
@@ -460,7 +461,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 +515,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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
  2012-06-14 10:07 ` [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-14 18:06   ` Pablo Neira Ayuso
  2012-06-14 10:07 ` [PATCH 04/10] netfilter: regard users as refcount for l4proto's per-net data Gao feng
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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 |    8 ++++++++
 net/netfilter/nf_conntrack_proto.c           |    4 +---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 5dd60f2..889b717 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -132,6 +132,14 @@ extern int nf_ct_port_nlattr_to_tuple(struct nlattr *tb[],
 extern int nf_ct_port_nlattr_tuple_size(void);
 extern const struct nla_policy nf_ct_port_nla_policy[];
 
+static inline 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
+}
+
 #ifdef CONFIG_SYSCTL
 #ifdef DEBUG_INVALID_PACKETS
 #define LOG_INVALID(net, proto)				\
diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 8fc0332..c9df1b4 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -361,9 +361,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
 					    NULL);
 		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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 04/10] netfilter: regard users as refcount for l4proto's per-net data
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
  2012-06-14 10:07 ` [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net Gao feng
  2012-06-14 10:07 ` [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-14 18:03   ` Pablo Neira Ayuso
  2012-06-14 10:07 ` [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init Gao feng
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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 |   70 ++++++++++++++++++++++-------------
 1 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index c9df1b4..63f9430 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,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);
@@ -330,20 +326,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);
@@ -357,8 +350,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;
 		nf_ct_kfree_compat_sysctl_table(pn);
@@ -374,11 +366,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,
@@ -391,8 +381,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 */
 }
 
@@ -458,22 +446,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;
+		}
 	}
-
+	/* increase the nf_proto_net's refcnt */
+	pn->users++;
+out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
@@ -498,10 +497,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;
+
+	/* decrease the nf_proto_net's refcnt */
+	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);
@@ -513,11 +520,14 @@ 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;
@@ -527,13 +537,21 @@ 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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
                   ` (2 preceding siblings ...)
  2012-06-14 10:07 ` [PATCH 04/10] netfilter: regard users as refcount for l4proto's per-net data Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-15 11:44   ` Pablo Neira Ayuso
  2012-06-14 10:07 ` [PATCH 06/10] merge udpv[4,6]_net_init into udp_net_init Gao feng
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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 |   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;
 
-#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);
+		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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 06/10] merge udpv[4,6]_net_init into udp_net_init
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
                   ` (3 preceding siblings ...)
  2012-06-14 10:07 ` [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-14 18:13   ` Pablo Neira Ayuso
  2012-06-14 10:07 ` [PATCH 07/10] netfilter: nf_conntrack_l4proto_udplite[4,6] cleanup Gao feng
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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 |   60 ++++++++++++--------------------
 1 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 2b978e6..480126e 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -239,13 +239,16 @@ static int udp_kmemdup_sysctl_table(struct nf_proto_net *pn)
 {
 #ifdef CONFIG_SYSCTL
 	struct nf_udp_net *un = (struct nf_udp_net *)pn;
+
 	if (pn->ctl_table)
 		return 0;
+
 	pn->ctl_table = kmemdup(udp_sysctl_table,
 				sizeof(udp_sysctl_table),
 				GFP_KERNEL);
 	if (!pn->ctl_table)
 		return -ENOMEM;
+
 	pn->ctl_table[0].data = &un->timeouts[UDP_CT_UNREPLIED];
 	pn->ctl_table[1].data = &un->timeouts[UDP_CT_REPLIED];
 #endif
@@ -257,6 +260,7 @@ 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;
+
 	pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table,
 				       sizeof(udp_compat_sysctl_table),
 				       GFP_KERNEL);
@@ -270,52 +274,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 = 0;
+	struct nf_udp_net *un = udp_pernet(net);
+	struct nf_proto_net *pn = (struct nf_proto_net *)un;
+
+	if (pn->users) {
+		int i = 0;
 		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;
+	if (proto == AF_INET) {
+		ret = udp_kmemdup_compat_sysctl_table(pn);
+		if (ret < 0)
+			return ret;
 
-	udp_init_net_data(un);
+		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_compat_sysctl_table(pn);
-	if (ret < 0)
-		return ret;
-
-	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 +327,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 +358,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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 07/10] netfilter: nf_conntrack_l4proto_udplite[4,6] cleanup
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
                   ` (4 preceding siblings ...)
  2012-06-14 10:07 ` [PATCH 06/10] merge udpv[4,6]_net_init into udp_net_init Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-14 18:17   ` Pablo Neira Ayuso
  2012-06-14 10:07 ` [PATCH 08/10] netfilter: merge sctpv[4,6]_net_init into sctp_net_init Gao feng
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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 |   42 +++++++++++++++++----------
 1 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index d33e511..f6a789a 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -234,29 +234,39 @@ static struct ctl_table udplite_sysctl_table[] = {
 };
 #endif /* CONFIG_SYSCTL */
 
+static int udplite_kmemdup_sysctl_table(struct nf_proto_net *pn)
+{
+#ifdef CONFIG_SYSCTL
+	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)
 {
-	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++) {
-#endif
+
+	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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 08/10] netfilter: merge sctpv[4,6]_net_init into sctp_net_init
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
                   ` (5 preceding siblings ...)
  2012-06-14 10:07 ` [PATCH 07/10] netfilter: nf_conntrack_l4proto_udplite[4,6] cleanup Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-14 10:07 ` [PATCH 09/10] netfilter: nf_conntrack_l4proto_generic cleanup Gao feng
  2012-06-14 10:07 ` [PATCH 10/10] netfilter: nf_conntrack_l4proto_dccp[4,6] cleanup Gao feng
  8 siblings, 0 replies; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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 |   60 ++++++++++--------------------
 1 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 1e7836c..49ea3c0 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -707,23 +707,11 @@ 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
 	struct sctp_net *sn = (struct sctp_net *)pn;
+
 	if (pn->ctl_table)
 		return 0;
 
@@ -749,6 +737,7 @@ static int sctp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn)
 #ifdef CONFIG_SYSCTL
 #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
 	struct sctp_net *sn = (struct sctp_net *)pn;
+
 	pn->ctl_compat_table = kmemdup(sctp_compat_sysctl_table,
 				       sizeof(sctp_compat_sysctl_table),
 				       GFP_KERNEL);
@@ -767,41 +756,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;
+	int ret = 0;
 	struct sctp_net *sn = sctp_pernet(net);
 	struct nf_proto_net *pn = (struct nf_proto_net *)sn;
 
-	sctp_init_net_data(sn);
-
-	ret = sctp_kmemdup_compat_sysctl_table(pn);
-	if (ret < 0)
-		return ret;
+	if (!sn->pn.users) {
+		int i = 0;
+		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 +813,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 +847,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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 09/10] netfilter: nf_conntrack_l4proto_generic cleanup
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
                   ` (6 preceding siblings ...)
  2012-06-14 10:07 ` [PATCH 08/10] netfilter: merge sctpv[4,6]_net_init into sctp_net_init Gao feng
@ 2012-06-14 10:07 ` Gao feng
  2012-06-14 10:07 ` [PATCH 10/10] netfilter: nf_conntrack_l4proto_dccp[4,6] cleanup Gao feng
  8 siblings, 0 replies; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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..5e3c0ea 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 = 0;
+	struct nf_generic_net *gn = generic_pernet(net);
+	struct nf_proto_net *pn = (struct nf_proto_net *)gn;
+
+	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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 10/10] netfilter: nf_conntrack_l4proto_dccp[4,6] cleanup
  2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
                   ` (7 preceding siblings ...)
  2012-06-14 10:07 ` [PATCH 09/10] netfilter: nf_conntrack_l4proto_generic cleanup Gao feng
@ 2012-06-14 10:07 ` Gao feng
  8 siblings, 0 replies; 22+ messages in thread
From: Gao feng @ 2012-06-14 10:07 UTC (permalink / raw)
  To: pablo; +Cc: netdev, netfilter-devel, 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 |   51 +++++++++++++++++-------------
 1 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index 52da8f0..ef044ef 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -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;
 
-#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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net
  2012-06-14 10:07 ` [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net Gao feng
@ 2012-06-14 17:59   ` Pablo Neira Ayuso
  2012-06-15  1:38     ` Gao feng
  0 siblings, 1 reply; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-14 17:59 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

On Thu, Jun 14, 2012 at 06:07:17PM +0800, Gao feng wrote:
> 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             |    6 ++++--
>  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, 17 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..8fc0332 100644
> --- a/net/netfilter/nf_conntrack_proto.c
> +++ b/net/netfilter/nf_conntrack_proto.c
> @@ -193,6 +193,7 @@ static int nf_ct_l3proto_register_sysctl(struct net *net,
>  					    l3proto->ctl_table_path,
>  					    in->ctl_table,
>  					    NULL);
> +

This entire patchset contains many extra new lines. If you want to
provide some cleanup, it should come in some follow-up patch.

>  		if (err < 0) {
>  			kfree(in->ctl_table);
>  			in->ctl_table = NULL;
> @@ -460,7 +461,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 +515,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);

You have to make sure that lines break at 80-chars per column.

Something like this should be fine:

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 04/10] netfilter: regard users as refcount for l4proto's per-net data
  2012-06-14 10:07 ` [PATCH 04/10] netfilter: regard users as refcount for l4proto's per-net data Gao feng
@ 2012-06-14 18:03   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-14 18:03 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

On Thu, Jun 14, 2012 at 06:07:19PM +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 |   70 ++++++++++++++++++++++-------------
>  1 files changed, 44 insertions(+), 26 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> index c9df1b4..63f9430 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,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);
> @@ -330,20 +326,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);
> @@ -357,8 +350,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;
>  		nf_ct_kfree_compat_sysctl_table(pn);
> @@ -374,11 +366,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,
> @@ -391,8 +381,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 */
>  }
>  
> @@ -458,22 +446,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;
> +		}
>  	}
> -
> +	/* increase the nf_proto_net's refcnt */

this comment is superfluous, please remove it.

> +	pn->users++;
> +out:
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
> @@ -498,10 +497,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;
> +
> +	/* decrease the nf_proto_net's refcnt */

same thing.

> +	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);
> @@ -513,11 +520,14 @@ 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);

break lines at 80 chars per column.

>  	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;
> @@ -527,13 +537,21 @@ 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
> 
> --
> 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear
  2012-06-14 10:07 ` [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear Gao feng
@ 2012-06-14 18:06   ` Pablo Neira Ayuso
  2012-06-15  1:36     ` Gao feng
  0 siblings, 1 reply; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-14 18:06 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

On Thu, Jun 14, 2012 at 06:07:18PM +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 |    8 ++++++++
>  net/netfilter/nf_conntrack_proto.c           |    4 +---
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
> index 5dd60f2..889b717 100644
> --- a/include/net/netfilter/nf_conntrack_l4proto.h
> +++ b/include/net/netfilter/nf_conntrack_l4proto.h
> @@ -132,6 +132,14 @@ extern int nf_ct_port_nlattr_to_tuple(struct nlattr *tb[],
>  extern int nf_ct_port_nlattr_tuple_size(void);
>  extern const struct nla_policy nf_ct_port_nla_policy[];
>  
> +static inline 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
> +}
> +
>  #ifdef CONFIG_SYSCTL
>  #ifdef DEBUG_INVALID_PACKETS
>  #define LOG_INVALID(net, proto)				\
> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> index 8fc0332..c9df1b4 100644
> --- a/net/netfilter/nf_conntrack_proto.c
> +++ b/net/netfilter/nf_conntrack_proto.c
> @@ -361,9 +361,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
>  					    NULL);
>  		if (err == 0)
>  			goto out;
> -
> -		kfree(pn->ctl_compat_table);
> -		pn->ctl_compat_table = NULL;
> +		nf_ct_kfree_compat_sysctl_table(pn);

if this is the only client of this function, then make it static and
define it inside nf_conntrack_proto.c

>  		nf_ct_unregister_sysctl(&pn->ctl_table_header,
>  					&pn->ctl_table,
>  					&pn->users);
> -- 
> 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 06/10] merge udpv[4,6]_net_init into udp_net_init
  2012-06-14 10:07 ` [PATCH 06/10] merge udpv[4,6]_net_init into udp_net_init Gao feng
@ 2012-06-14 18:13   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-14 18:13 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

On Thu, Jun 14, 2012 at 06:07:21PM +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 |   60 ++++++++++++--------------------
>  1 files changed, 22 insertions(+), 38 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
> index 2b978e6..480126e 100644
> --- a/net/netfilter/nf_conntrack_proto_udp.c
> +++ b/net/netfilter/nf_conntrack_proto_udp.c
> @@ -239,13 +239,16 @@ static int udp_kmemdup_sysctl_table(struct nf_proto_net *pn)
>  {
>  #ifdef CONFIG_SYSCTL
>  	struct nf_udp_net *un = (struct nf_udp_net *)pn;
> +
>  	if (pn->ctl_table)
>  		return 0;
> +
>  	pn->ctl_table = kmemdup(udp_sysctl_table,
>  				sizeof(udp_sysctl_table),
>  				GFP_KERNEL);
>  	if (!pn->ctl_table)
>  		return -ENOMEM;
> +

I like this cleanup. But you have to make it in a separate patch.

>  	pn->ctl_table[0].data = &un->timeouts[UDP_CT_UNREPLIED];
>  	pn->ctl_table[1].data = &un->timeouts[UDP_CT_REPLIED];
>  #endif
> @@ -257,6 +260,7 @@ 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;
> +
>  	pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table,
>  				       sizeof(udp_compat_sysctl_table),
>  				       GFP_KERNEL);
> @@ -270,52 +274,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 = 0;
> +	struct nf_udp_net *un = udp_pernet(net);
> +	struct nf_proto_net *pn = (struct nf_proto_net *)un;
> +
> +	if (pn->users) {
> +		int i = 0;

redundant initialization of that i variable.

>  		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;
> +	if (proto == AF_INET) {
> +		ret = udp_kmemdup_compat_sysctl_table(pn);
> +		if (ret < 0)
> +			return ret;
>  
> -	udp_init_net_data(un);
> +		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_compat_sysctl_table(pn);
> -	if (ret < 0)
> -		return ret;
> -
> -	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 +327,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 +358,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
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 07/10] netfilter: nf_conntrack_l4proto_udplite[4,6] cleanup
  2012-06-14 10:07 ` [PATCH 07/10] netfilter: nf_conntrack_l4proto_udplite[4,6] cleanup Gao feng
@ 2012-06-14 18:17   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-14 18:17 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

On Thu, Jun 14, 2012 at 06:07:22PM +0800, Gao feng wrote:
> 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 |   42 +++++++++++++++++----------
>  1 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
> index d33e511..f6a789a 100644
> --- a/net/netfilter/nf_conntrack_proto_udplite.c
> +++ b/net/netfilter/nf_conntrack_proto_udplite.c
> @@ -234,29 +234,39 @@ static struct ctl_table udplite_sysctl_table[] = {
>  };
>  #endif /* CONFIG_SYSCTL */
>  
> +static int udplite_kmemdup_sysctl_table(struct nf_proto_net *pn)
> +{
> +#ifdef CONFIG_SYSCTL
> +	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;
> +}
> +
> +

Remove extra line.

>  static int udplite_init_net(struct net *net, u_int16_t proto)
>  {
> -	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++) {
> -#endif
> +
> +	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
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear
  2012-06-14 18:06   ` Pablo Neira Ayuso
@ 2012-06-15  1:36     ` Gao feng
  2012-06-15 10:08       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-15  1:36 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netdev, netfilter-devel

Hi Pablo:

于 2012年06月15日 02:06, Pablo Neira Ayuso 写道:
> On Thu, Jun 14, 2012 at 06:07:18PM +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 |    8 ++++++++
>>  net/netfilter/nf_conntrack_proto.c           |    4 +---
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
>> index 5dd60f2..889b717 100644
>> --- a/include/net/netfilter/nf_conntrack_l4proto.h
>> +++ b/include/net/netfilter/nf_conntrack_l4proto.h
>> @@ -132,6 +132,14 @@ extern int nf_ct_port_nlattr_to_tuple(struct nlattr *tb[],
>>  extern int nf_ct_port_nlattr_tuple_size(void);
>>  extern const struct nla_policy nf_ct_port_nla_policy[];
>>  
>> +static inline 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
>> +}
>> +
>>  #ifdef CONFIG_SYSCTL
>>  #ifdef DEBUG_INVALID_PACKETS
>>  #define LOG_INVALID(net, proto)				\
>> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
>> index 8fc0332..c9df1b4 100644
>> --- a/net/netfilter/nf_conntrack_proto.c
>> +++ b/net/netfilter/nf_conntrack_proto.c
>> @@ -361,9 +361,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
>>  					    NULL);
>>  		if (err == 0)
>>  			goto out;
>> -
>> -		kfree(pn->ctl_compat_table);
>> -		pn->ctl_compat_table = NULL;
>> +		nf_ct_kfree_compat_sysctl_table(pn);
> 
> if this is the only client of this function, then make it static and
> define it inside nf_conntrack_proto.c
> 

tcp_init_net,udp_init_net use this function too.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net
  2012-06-14 17:59   ` Pablo Neira Ayuso
@ 2012-06-15  1:38     ` Gao feng
  2012-06-15 10:10       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 22+ messages in thread
From: Gao feng @ 2012-06-15  1:38 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netdev, netfilter-devel

于 2012年06月15日 01:59, Pablo Neira Ayuso 写道:
> On Thu, Jun 14, 2012 at 06:07:17PM +0800, Gao feng wrote:
>> 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             |    6 ++++--
>>  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, 17 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..8fc0332 100644
>> --- a/net/netfilter/nf_conntrack_proto.c
>> +++ b/net/netfilter/nf_conntrack_proto.c
>> @@ -193,6 +193,7 @@ static int nf_ct_l3proto_register_sysctl(struct net *net,
>>  					    l3proto->ctl_table_path,
>>  					    in->ctl_table,
>>  					    NULL);
>> +
> 
> This entire patchset contains many extra new lines. If you want to
> provide some cleanup, it should come in some follow-up patch.

Ok, I will make a follow-up patch to do some cleanup.

> 
>>  		if (err < 0) {
>>  			kfree(in->ctl_table);
>>  			in->ctl_table = NULL;
>> @@ -460,7 +461,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 +515,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);
> 
> You have to make sure that lines break at 80-chars per column.
> 
> Something like this should be fine:
> 
>         err = nf_conntrack_l4proto_generic.init_net(net,
>                                         nf_conntrack_l4proto_generic.l3proto);
> 
> 

Get it, thanks.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear
  2012-06-15  1:36     ` Gao feng
@ 2012-06-15 10:08       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-15 10:08 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

On Fri, Jun 15, 2012 at 09:36:06AM +0800, Gao feng wrote:
> Hi Pablo:
> 
> 于 2012年06月15日 02:06, Pablo Neira Ayuso 写道:
> > On Thu, Jun 14, 2012 at 06:07:18PM +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 |    8 ++++++++
> >>  net/netfilter/nf_conntrack_proto.c           |    4 +---
> >>  2 files changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
> >> index 5dd60f2..889b717 100644
> >> --- a/include/net/netfilter/nf_conntrack_l4proto.h
> >> +++ b/include/net/netfilter/nf_conntrack_l4proto.h
> >> @@ -132,6 +132,14 @@ extern int nf_ct_port_nlattr_to_tuple(struct nlattr *tb[],
> >>  extern int nf_ct_port_nlattr_tuple_size(void);
> >>  extern const struct nla_policy nf_ct_port_nla_policy[];
> >>  
> >> +static inline 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
> >> +}

inline is usually reserved for fast path, not initialization path.

Although this function is fairly small, I think I prefer that this
function is defined in nf_conntrack_proto.c and exported.

> >> +
> >>  #ifdef CONFIG_SYSCTL
> >>  #ifdef DEBUG_INVALID_PACKETS
> >>  #define LOG_INVALID(net, proto)				\
> >> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> >> index 8fc0332..c9df1b4 100644
> >> --- a/net/netfilter/nf_conntrack_proto.c
> >> +++ b/net/netfilter/nf_conntrack_proto.c
> >> @@ -361,9 +361,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
> >>  					    NULL);
> >>  		if (err == 0)
> >>  			goto out;
> >> -
> >> -		kfree(pn->ctl_compat_table);
> >> -		pn->ctl_compat_table = NULL;
> >> +		nf_ct_kfree_compat_sysctl_table(pn);
> > 
> > if this is the only client of this function, then make it static and
> > define it inside nf_conntrack_proto.c
> > 
> 
> tcp_init_net,udp_init_net use this function too.
> 
> Thanks
> --
> 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net
  2012-06-15  1:38     ` Gao feng
@ 2012-06-15 10:10       ` Pablo Neira Ayuso
  2012-06-15 10:49         ` Gao feng
  0 siblings, 1 reply; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-15 10:10 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

On Fri, Jun 15, 2012 at 09:38:39AM +0800, Gao feng wrote:
> 于 2012年06月15日 01:59, Pablo Neira Ayuso 写道:
> > On Thu, Jun 14, 2012 at 06:07:17PM +0800, Gao feng wrote:
> >> 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             |    6 ++++--
> >>  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, 17 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..8fc0332 100644
> >> --- a/net/netfilter/nf_conntrack_proto.c
> >> +++ b/net/netfilter/nf_conntrack_proto.c
> >> @@ -193,6 +193,7 @@ static int nf_ct_l3proto_register_sysctl(struct net *net,
> >>  					    l3proto->ctl_table_path,
> >>  					    in->ctl_table,
> >>  					    NULL);
> >> +
> > 
> > This entire patchset contains many extra new lines. If you want to
> > provide some cleanup, it should come in some follow-up patch.
> 
> Ok, I will make a follow-up patch to do some cleanup.

I prefer if you wait a bit until I see how this looks like in the end.
Then, we can apply this little comestical changes.
--
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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net
  2012-06-15 10:10       ` Pablo Neira Ayuso
@ 2012-06-15 10:49         ` Gao feng
  0 siblings, 0 replies; 22+ messages in thread
From: Gao feng @ 2012-06-15 10:49 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netdev, netfilter-devel

于 2012年06月15日 18:10, Pablo Neira Ayuso 写道:
> On Fri, Jun 15, 2012 at 09:38:39AM +0800, Gao feng wrote:
>> 于 2012年06月15日 01:59, Pablo Neira Ayuso 写道:
>>> On Thu, Jun 14, 2012 at 06:07:17PM +0800, Gao feng wrote:
>>>> 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             |    6 ++++--
>>>>  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, 17 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..8fc0332 100644
>>>> --- a/net/netfilter/nf_conntrack_proto.c
>>>> +++ b/net/netfilter/nf_conntrack_proto.c
>>>> @@ -193,6 +193,7 @@ static int nf_ct_l3proto_register_sysctl(struct net *net,
>>>>  					    l3proto->ctl_table_path,
>>>>  					    in->ctl_table,
>>>>  					    NULL);
>>>> +
>>>
>>> This entire patchset contains many extra new lines. If you want to
>>> provide some cleanup, it should come in some follow-up patch.
>>
>> Ok, I will make a follow-up patch to do some cleanup.
> 
> I prefer if you wait a bit until I see how this looks like in the end.
> Then, we can apply this little comestical changes.

Ok, waiting for you ;)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init
  2012-06-14 10:07 ` [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init Gao feng
@ 2012-06-15 11:44   ` Pablo Neira Ayuso
  2012-06-15 12:30     ` Gao feng
  0 siblings, 1 reply; 22+ messages in thread
From: Pablo Neira Ayuso @ 2012-06-15 11:44 UTC (permalink / raw)
  To: Gao feng; +Cc: netdev, netfilter-devel

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.

> -#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;

/*
 * here comes all per-net initialization
 */

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init
  2012-06-15 11:44   ` Pablo Neira Ayuso
@ 2012-06-15 12:30     ` Gao feng
  0 siblings, 0 replies; 22+ messages in thread
From: Gao feng @ 2012-06-15 12:30 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netdev, netfilter-devel

于 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2012-06-15 12:30 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-14 10:07 [PATCH 01/10] netfilter: fix problem with proto register Gao feng
2012-06-14 10:07 ` [PATCH 02/10] netfilter: add parameter proto for l4proto.init_net Gao feng
2012-06-14 17:59   ` Pablo Neira Ayuso
2012-06-15  1:38     ` Gao feng
2012-06-15 10:10       ` Pablo Neira Ayuso
2012-06-15 10:49         ` Gao feng
2012-06-14 10:07 ` [PATCH 03/10] netfilter: add nf_ct_kfree_compat_sysctl_table to make codes clear Gao feng
2012-06-14 18:06   ` Pablo Neira Ayuso
2012-06-15  1:36     ` Gao feng
2012-06-15 10:08       ` Pablo Neira Ayuso
2012-06-14 10:07 ` [PATCH 04/10] netfilter: regard users as refcount for l4proto's per-net data Gao feng
2012-06-14 18:03   ` Pablo Neira Ayuso
2012-06-14 10:07 ` [PATCH 05/10] netfilter: merge tcpv[4,6]_net_init into tcp_net_init Gao feng
2012-06-15 11:44   ` Pablo Neira Ayuso
2012-06-15 12:30     ` Gao feng
2012-06-14 10:07 ` [PATCH 06/10] merge udpv[4,6]_net_init into udp_net_init Gao feng
2012-06-14 18:13   ` Pablo Neira Ayuso
2012-06-14 10:07 ` [PATCH 07/10] netfilter: nf_conntrack_l4proto_udplite[4,6] cleanup Gao feng
2012-06-14 18:17   ` Pablo Neira Ayuso
2012-06-14 10:07 ` [PATCH 08/10] netfilter: merge sctpv[4,6]_net_init into sctp_net_init Gao feng
2012-06-14 10:07 ` [PATCH 09/10] netfilter: nf_conntrack_l4proto_generic cleanup Gao feng
2012-06-14 10:07 ` [PATCH 10/10] netfilter: nf_conntrack_l4proto_dccp[4,6] cleanup Gao feng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.