[PATCH 1/3 nf-next] netfilter: nf_ct_helper: define pr_fmt()

[PATCH 1/3 nf-next] netfilter: nf_ct_helper: define pr_fmt()

[Pablo Neira Ayuso]

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_ftp.c  | 17 ++++++++---------
 net/netfilter/nf_conntrack_irc.c  |  7 ++++---
 net/netfilter/nf_conntrack_sane.c | 19 +++++++++----------
 net/netfilter/nf_conntrack_sip.c  |  5 +++--
 net/netfilter/nf_conntrack_tftp.c |  7 ++++---
 5 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index b666959..883c691 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -10,6 +10,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/netfilter.h>
@@ -505,11 +507,11 @@ skip_nl_seq:
 		   different IP address.  Simply don't record it for
 		   NAT. */
 		if (cmd.l3num == PF_INET) {
-			pr_debug("conntrack_ftp: NOT RECORDING: %pI4 != %pI4\n",
+			pr_debug("NOT RECORDING: %pI4 != %pI4\n",
 				 &cmd.u3.ip,
 				 &ct->tuplehash[dir].tuple.src.u3.ip);
 		} else {
-			pr_debug("conntrack_ftp: NOT RECORDING: %pI6 != %pI6\n",
+			pr_debug("NOT RECORDING: %pI6 != %pI6\n",
 				 cmd.u3.ip6,
 				 ct->tuplehash[dir].tuple.src.u3.ip6);
 		}
@@ -586,8 +588,7 @@ static void nf_conntrack_ftp_fini(void)
 			if (ftp[i][j].me == NULL)
 				continue;
 
-			pr_debug("nf_ct_ftp: unregistering helper for pf: %d "
-				 "port: %d\n",
+			pr_debug("unregistering helper for pf: %d port: %d\n",
 				 ftp[i][j].tuple.src.l3num, ports[i]);
 			nf_conntrack_helper_unregister(&ftp[i][j]);
 		}
@@ -625,14 +626,12 @@ static int __init nf_conntrack_ftp_init(void)
 			else
 				sprintf(ftp[i][j].name, "ftp-%d", ports[i]);
 
-			pr_debug("nf_ct_ftp: registering helper for pf: %d "
-				 "port: %d\n",
+			pr_debug("registering helper for pf: %d port: %d\n",
 				 ftp[i][j].tuple.src.l3num, ports[i]);
 			ret = nf_conntrack_helper_register(&ftp[i][j]);
 			if (ret) {
-				printk(KERN_ERR "nf_ct_ftp: failed to register"
-				       " helper for pf: %d port: %d\n",
-					ftp[i][j].tuple.src.l3num, ports[i]);
+				pr_err("failed to register helper for pf: %d port: %d\n",
+				       ftp[i][j].tuple.src.l3num, ports[i]);
 				nf_conntrack_ftp_fini();
 				return ret;
 			}
diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index 0fd2976..8b6da27 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -9,6 +9,8 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/skbuff.h>
@@ -237,7 +239,7 @@ static int __init nf_conntrack_irc_init(void)
 	int i, ret;
 
 	if (max_dcc_channels < 1) {
-		printk(KERN_ERR "nf_ct_irc: max_dcc_channels must not be zero\n");
+		pr_err("max_dcc_channels must not be zero\n");
 		return -EINVAL;
 	}
 
@@ -267,8 +269,7 @@ static int __init nf_conntrack_irc_init(void)
 
 		ret = nf_conntrack_helper_register(&irc[i]);
 		if (ret) {
-			printk(KERN_ERR "nf_ct_irc: failed to register helper "
-			       "for pf: %u port: %u\n",
+			pr_err("failed to register helper for pf: %u port: %u\n",
 			       irc[i].tuple.src.l3num, ports[i]);
 			nf_conntrack_irc_fini();
 			return ret;
diff --git a/net/netfilter/nf_conntrack_sane.c b/net/netfilter/nf_conntrack_sane.c
index 4a2134f..7523a57 100644
--- a/net/netfilter/nf_conntrack_sane.c
+++ b/net/netfilter/nf_conntrack_sane.c
@@ -17,6 +17,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/netfilter.h>
@@ -120,14 +122,14 @@ static int help(struct sk_buff *skb,
 	ct_sane_info->state = SANE_STATE_NORMAL;
 
 	if (datalen < sizeof(struct sane_reply_net_start)) {
-		pr_debug("nf_ct_sane: NET_START reply too short\n");
+		pr_debug("NET_START reply too short\n");
 		goto out;
 	}
 
 	reply = sb_ptr;
 	if (reply->status != htonl(SANE_STATUS_SUCCESS)) {
 		/* saned refused the command */
-		pr_debug("nf_ct_sane: unsuccessful SANE_STATUS = %u\n",
+		pr_debug("unsuccessful SANE_STATUS = %u\n",
 			 ntohl(reply->status));
 		goto out;
 	}
@@ -148,7 +150,7 @@ static int help(struct sk_buff *skb,
 			  &tuple->src.u3, &tuple->dst.u3,
 			  IPPROTO_TCP, NULL, &reply->port);
 
-	pr_debug("nf_ct_sane: expect: ");
+	pr_debug("expect: ");
 	nf_ct_dump_tuple(&exp->tuple);
 
 	/* Can't expect this?  Best to drop packet now. */
@@ -178,8 +180,7 @@ static void nf_conntrack_sane_fini(void)
 
 	for (i = 0; i < ports_c; i++) {
 		for (j = 0; j < 2; j++) {
-			pr_debug("nf_ct_sane: unregistering helper for pf: %d "
-				 "port: %d\n",
+			pr_debug("unregistering helper for pf: %d port: %d\n",
 				 sane[i][j].tuple.src.l3num, ports[i]);
 			nf_conntrack_helper_unregister(&sane[i][j]);
 		}
@@ -216,14 +217,12 @@ static int __init nf_conntrack_sane_init(void)
 			else
 				sprintf(sane[i][j].name, "sane-%d", ports[i]);
 
-			pr_debug("nf_ct_sane: registering helper for pf: %d "
-				 "port: %d\n",
+			pr_debug("registering helper for pf: %d port: %d\n",
 				 sane[i][j].tuple.src.l3num, ports[i]);
 			ret = nf_conntrack_helper_register(&sane[i][j]);
 			if (ret) {
-				printk(KERN_ERR "nf_ct_sane: failed to "
-				       "register helper for pf: %d port: %d\n",
-					sane[i][j].tuple.src.l3num, ports[i]);
+				pr_err("failed to register helper for pf: %d port: %d\n",
+				       sane[i][j].tuple.src.l3num, ports[i]);
 				nf_conntrack_sane_fini();
 				return ret;
 			}
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 885b4ab..3e06402 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -10,6 +10,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/ctype.h>
 #include <linux/skbuff.h>
@@ -1665,8 +1667,7 @@ static int __init nf_conntrack_sip_init(void)
 
 			ret = nf_conntrack_helper_register(&sip[i][j]);
 			if (ret) {
-				printk(KERN_ERR "nf_ct_sip: failed to register"
-				       " helper for pf: %u port: %u\n",
+				pr_err("failed to register helper for pf: %u port: %u\n",
 				       sip[i][j].tuple.src.l3num, ports[i]);
 				nf_conntrack_sip_fini();
 				return ret;
diff --git a/net/netfilter/nf_conntrack_tftp.c b/net/netfilter/nf_conntrack_tftp.c
index e68ab4f..36f9640 100644
--- a/net/netfilter/nf_conntrack_tftp.c
+++ b/net/netfilter/nf_conntrack_tftp.c
@@ -5,6 +5,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/in.h>
@@ -138,9 +140,8 @@ static int __init nf_conntrack_tftp_init(void)
 
 			ret = nf_conntrack_helper_register(&tftp[i][j]);
 			if (ret) {
-				printk(KERN_ERR "nf_ct_tftp: failed to register"
-				       " helper for pf: %u port: %u\n",
-					tftp[i][j].tuple.src.l3num, ports[i]);
+				pr_err("failed to register helper for pf: %u port: %u\n",
+				       tftp[i][j].tuple.src.l3num, ports[i]);
 				nf_conntrack_tftp_fini();
 				return ret;
 			}
-- 
2.1.4

[PATCH 2/3 nf-next] netfilter: nf_ct_helper: Add new function nf_ct_helper_init()

[Pablo Neira Ayuso]

From: Gao Feng <fgao@ikuai8.com>

This patch adds a new function to consolidate helper initialization.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
@Gao Feng: I'm respecting your original authorship on this. I have applied
coding style cleanups mainly and resolved some small leftovers.

Please, stick to the coding style we have in your follow up submissions.
That speeds up patch acceptance process, otherwise I have to find the spare
time to sweep on this to polish small details.

Compile tested-only.

 include/net/netfilter/nf_conntrack_helper.h | 11 ++++
 net/netfilter/nf_conntrack_ftp.c            | 47 ++++++++--------
 net/netfilter/nf_conntrack_helper.c         | 30 +++++++++++
 net/netfilter/nf_conntrack_irc.c            | 17 ++----
 net/netfilter/nf_conntrack_sane.c           | 46 ++++++++--------
 net/netfilter/nf_conntrack_sip.c            | 84 ++++++++++++++++-------------
 net/netfilter/nf_conntrack_tftp.c           | 42 +++++++--------
 7 files changed, 157 insertions(+), 120 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h
index 6cf614bc..b5e2d7d 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -58,6 +58,17 @@ struct nf_conntrack_helper *__nf_conntrack_helper_find(const char *name,
 struct nf_conntrack_helper *nf_conntrack_helper_try_module_get(const char *name,
 							       u16 l3num,
 							       u8 protonum);
+void nf_ct_helper_init(struct nf_conntrack_helper *helper,
+		       u16 l3num, u16 protonum, const char *name,
+		       u16 default_port, u16 spec_port,
+		       const struct nf_conntrack_expect_policy *exp_pol,
+		       u32 expect_class_max, u32 data_len,
+		       int (*help)(struct sk_buff *skb, unsigned int protoff,
+				   struct nf_conn *ct,
+				   enum ip_conntrack_info ctinfo),
+		       int (*from_nlattr)(struct nlattr *attr,
+					  struct nf_conn *ct),
+		       struct module *module);
 
 int nf_conntrack_helper_register(struct nf_conntrack_helper *);
 void nf_conntrack_helper_unregister(struct nf_conntrack_helper *);
diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index 883c691..80928c6 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -599,7 +599,7 @@ static void nf_conntrack_ftp_fini(void)
 
 static int __init nf_conntrack_ftp_init(void)
 {
-	int i, j = -1, ret = 0;
+	int i, ret = 0;
 
 	ftp_buffer = kmalloc(65536, GFP_KERNEL);
 	if (!ftp_buffer)
@@ -611,30 +611,27 @@ static int __init nf_conntrack_ftp_init(void)
 	/* FIXME should be configurable whether IPv4 and IPv6 FTP connections
 		 are tracked or not - YK */
 	for (i = 0; i < ports_c; i++) {
-		ftp[i][0].tuple.src.l3num = PF_INET;
-		ftp[i][1].tuple.src.l3num = PF_INET6;
-		for (j = 0; j < 2; j++) {
-			ftp[i][j].data_len = sizeof(struct nf_ct_ftp_master);
-			ftp[i][j].tuple.src.u.tcp.port = htons(ports[i]);
-			ftp[i][j].tuple.dst.protonum = IPPROTO_TCP;
-			ftp[i][j].expect_policy = &ftp_exp_policy;
-			ftp[i][j].me = THIS_MODULE;
-			ftp[i][j].help = help;
-			ftp[i][j].from_nlattr = nf_ct_ftp_from_nlattr;
-			if (ports[i] == FTP_PORT)
-				sprintf(ftp[i][j].name, "ftp");
-			else
-				sprintf(ftp[i][j].name, "ftp-%d", ports[i]);
-
-			pr_debug("registering helper for pf: %d port: %d\n",
-				 ftp[i][j].tuple.src.l3num, ports[i]);
-			ret = nf_conntrack_helper_register(&ftp[i][j]);
-			if (ret) {
-				pr_err("failed to register helper for pf: %d port: %d\n",
-				       ftp[i][j].tuple.src.l3num, ports[i]);
-				nf_conntrack_ftp_fini();
-				return ret;
-			}
+		nf_ct_helper_init(&ftp[i][0], AF_INET, IPPROTO_TCP, "ftp",
+				  FTP_PORT, ports[i], &ftp_exp_policy, 0,
+				  sizeof(struct nf_ct_ftp_master), help,
+				  nf_ct_ftp_from_nlattr, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&ftp[i][0]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %d port: %d\n",
+			       ftp[i][0].tuple.src.l3num, ports[i]);
+			nf_conntrack_ftp_fini();
+			return ret;
+		}
+		nf_ct_helper_init(&ftp[i][1], AF_INET6, IPPROTO_TCP, "ftp",
+				  FTP_PORT, ports[i], &ftp_exp_policy, 0,
+				  sizeof(struct nf_ct_ftp_master), help,
+				  nf_ct_ftp_from_nlattr, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&ftp[i][1]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %d port: %d\n",
+			       ftp[i][1].tuple.src.l3num, ports[i]);
+			nf_conntrack_ftp_fini();
+			return ret;
 		}
 	}
 
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index bd9d315..dddfefc 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -456,6 +456,36 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_helper_unregister);
 
+void nf_ct_helper_init(struct nf_conntrack_helper *helper,
+		       u16 l3num, u16 protonum, const char *name,
+		       u16 default_port, u16 spec_port,
+		       const struct nf_conntrack_expect_policy *exp_pol,
+		       u32 expect_class_max, u32 data_len,
+		       int (*help)(struct sk_buff *skb, unsigned int protoff,
+				   struct nf_conn *ct,
+				   enum ip_conntrack_info ctinfo),
+		       int (*from_nlattr)(struct nlattr *attr,
+					  struct nf_conn *ct),
+		       struct module *module)
+{
+	helper->tuple.src.l3num		= l3num;
+	helper->tuple.dst.protonum	= protonum;
+	helper->tuple.src.u.all		= htons(spec_port);
+	helper->expect_policy		= exp_pol;
+	helper->expect_class_max	= expect_class_max;
+	helper->data_len		= data_len;
+	helper->help			= help;
+	helper->from_nlattr		= from_nlattr;
+	helper->me			= module;
+
+	if (spec_port == default_port)
+		snprintf(helper->name, sizeof(helper->name), "%s", name);
+	else
+		snprintf(helper->name, sizeof(helper->name), "%s-%u", name,
+			 spec_port);
+}
+EXPORT_SYMBOL_GPL(nf_ct_helper_init);
+
 static struct nf_ct_ext_type helper_extend __read_mostly = {
 	.len	= sizeof(struct nf_conn_help),
 	.align	= __alignof__(struct nf_conn_help),
diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index 8b6da27..bc1a0dd 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -255,20 +255,11 @@ static int __init nf_conntrack_irc_init(void)
 		ports[ports_c++] = IRC_PORT;
 
 	for (i = 0; i < ports_c; i++) {
-		irc[i].tuple.src.l3num = AF_INET;
-		irc[i].tuple.src.u.tcp.port = htons(ports[i]);
-		irc[i].tuple.dst.protonum = IPPROTO_TCP;
-		irc[i].expect_policy = &irc_exp_policy;
-		irc[i].me = THIS_MODULE;
-		irc[i].help = help;
-
-		if (ports[i] == IRC_PORT)
-			sprintf(irc[i].name, "irc");
-		else
-			sprintf(irc[i].name, "irc-%u", i);
-
+		nf_ct_helper_init(&irc[i], AF_INET, IPPROTO_TCP, "irc",
+				  IRC_PORT, ports[i], &irc_exp_policy, 0, 0,
+				  help, NULL, THIS_MODULE);
 		ret = nf_conntrack_helper_register(&irc[i]);
-		if (ret) {
+		if (ret < 0) {
 			pr_err("failed to register helper for pf: %u port: %u\n",
 			       irc[i].tuple.src.l3num, ports[i]);
 			nf_conntrack_irc_fini();
diff --git a/net/netfilter/nf_conntrack_sane.c b/net/netfilter/nf_conntrack_sane.c
index 7523a57..d005b14 100644
--- a/net/netfilter/nf_conntrack_sane.c
+++ b/net/netfilter/nf_conntrack_sane.c
@@ -191,7 +191,7 @@ static void nf_conntrack_sane_fini(void)
 
 static int __init nf_conntrack_sane_init(void)
 {
-	int i, j = -1, ret = 0;
+	int i, ret = 0;
 
 	sane_buffer = kmalloc(65536, GFP_KERNEL);
 	if (!sane_buffer)
@@ -203,29 +203,27 @@ static int __init nf_conntrack_sane_init(void)
 	/* FIXME should be configurable whether IPv4 and IPv6 connections
 		 are tracked or not - YK */
 	for (i = 0; i < ports_c; i++) {
-		sane[i][0].tuple.src.l3num = PF_INET;
-		sane[i][1].tuple.src.l3num = PF_INET6;
-		for (j = 0; j < 2; j++) {
-			sane[i][j].data_len = sizeof(struct nf_ct_sane_master);
-			sane[i][j].tuple.src.u.tcp.port = htons(ports[i]);
-			sane[i][j].tuple.dst.protonum = IPPROTO_TCP;
-			sane[i][j].expect_policy = &sane_exp_policy;
-			sane[i][j].me = THIS_MODULE;
-			sane[i][j].help = help;
-			if (ports[i] == SANE_PORT)
-				sprintf(sane[i][j].name, "sane");
-			else
-				sprintf(sane[i][j].name, "sane-%d", ports[i]);
-
-			pr_debug("registering helper for pf: %d port: %d\n",
-				 sane[i][j].tuple.src.l3num, ports[i]);
-			ret = nf_conntrack_helper_register(&sane[i][j]);
-			if (ret) {
-				pr_err("failed to register helper for pf: %d port: %d\n",
-				       sane[i][j].tuple.src.l3num, ports[i]);
-				nf_conntrack_sane_fini();
-				return ret;
-			}
+		nf_ct_helper_init(&sane[i][0], AF_INET, IPPROTO_TCP, "sane",
+				  SANE_PORT, ports[i], &sane_exp_policy, 0,
+				  sizeof(struct nf_ct_sane_master), help, NULL,
+				  THIS_MODULE);
+		ret = nf_conntrack_helper_register(&sane[i][0]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %d port: %d\n",
+			       sane[i][0].tuple.src.l3num, ports[i]);
+			nf_conntrack_sane_fini();
+			return ret;
+		}
+		nf_ct_helper_init(&sane[i][1], AF_INET6, IPPROTO_TCP, "sane",
+				  SANE_PORT, ports[i], &sane_exp_policy, 0,
+				  sizeof(struct nf_ct_sane_master), help, NULL,
+				  THIS_MODULE);
+		ret = nf_conntrack_helper_register(&sane[i][1]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %d port: %d\n",
+			       sane[i][1].tuple.src.l3num, ports[i]);
+			nf_conntrack_sane_fini();
+			return ret;
 		}
 	}
 
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 3e06402..5951427 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1629,7 +1629,7 @@ static void nf_conntrack_sip_fini(void)
 
 static int __init nf_conntrack_sip_init(void)
 {
-	int i, j, ret;
+	int i, ret;
 
 	if (ports_c == 0)
 		ports[ports_c++] = SIP_PORT;
@@ -1637,41 +1637,53 @@ static int __init nf_conntrack_sip_init(void)
 	for (i = 0; i < ports_c; i++) {
 		memset(&sip[i], 0, sizeof(sip[i]));
 
-		sip[i][0].tuple.src.l3num = AF_INET;
-		sip[i][0].tuple.dst.protonum = IPPROTO_UDP;
-		sip[i][0].help = sip_help_udp;
-		sip[i][1].tuple.src.l3num = AF_INET;
-		sip[i][1].tuple.dst.protonum = IPPROTO_TCP;
-		sip[i][1].help = sip_help_tcp;
-
-		sip[i][2].tuple.src.l3num = AF_INET6;
-		sip[i][2].tuple.dst.protonum = IPPROTO_UDP;
-		sip[i][2].help = sip_help_udp;
-		sip[i][3].tuple.src.l3num = AF_INET6;
-		sip[i][3].tuple.dst.protonum = IPPROTO_TCP;
-		sip[i][3].help = sip_help_tcp;
-
-		for (j = 0; j < ARRAY_SIZE(sip[i]); j++) {
-			sip[i][j].data_len = sizeof(struct nf_ct_sip_master);
-			sip[i][j].tuple.src.u.udp.port = htons(ports[i]);
-			sip[i][j].expect_policy = sip_exp_policy;
-			sip[i][j].expect_class_max = SIP_EXPECT_MAX;
-			sip[i][j].me = THIS_MODULE;
-
-			if (ports[i] == SIP_PORT)
-				sprintf(sip[i][j].name, "sip");
-			else
-				sprintf(sip[i][j].name, "sip-%u", i);
-
-			pr_debug("port #%u: %u\n", i, ports[i]);
-
-			ret = nf_conntrack_helper_register(&sip[i][j]);
-			if (ret) {
-				pr_err("failed to register helper for pf: %u port: %u\n",
-				       sip[i][j].tuple.src.l3num, ports[i]);
-				nf_conntrack_sip_fini();
-				return ret;
-			}
+		nf_ct_helper_init(&sip[i][0], AF_INET, IPPROTO_UDP, "sip",
+				  SIP_PORT, ports[i], &sip_exp_policy[0],
+				  SIP_EXPECT_MAX,
+				  sizeof(struct nf_ct_sip_master), sip_help_udp,
+				  NULL, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&sip[i][0]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %u port: %u\n",
+			       sip[i][0].tuple.src.l3num, ports[i]);
+			nf_conntrack_sip_fini();
+			return ret;
+		}
+		nf_ct_helper_init(&sip[i][1], AF_INET, IPPROTO_TCP, "sip",
+				  SIP_PORT, ports[i], &sip_exp_policy[0],
+				  SIP_EXPECT_MAX,
+				  sizeof(struct nf_ct_sip_master), sip_help_tcp,
+				  NULL, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&sip[i][1]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %u port: %u\n",
+			       sip[i][1].tuple.src.l3num, ports[i]);
+			nf_conntrack_sip_fini();
+			return ret;
+		}
+		nf_ct_helper_init(&sip[i][2], AF_INET6, IPPROTO_UDP, "sip",
+				  SIP_PORT, ports[i], &sip_exp_policy[0],
+				  SIP_EXPECT_MAX,
+				  sizeof(struct nf_ct_sip_master), sip_help_udp,
+				  NULL, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&sip[i][2]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %u port: %u\n",
+			       sip[i][2].tuple.src.l3num, ports[i]);
+			nf_conntrack_sip_fini();
+			return ret;
+		}
+		nf_ct_helper_init(&sip[i][3], AF_INET6, IPPROTO_TCP, "sip",
+				  SIP_PORT, ports[i], &sip_exp_policy[0],
+				  SIP_EXPECT_MAX,
+				  sizeof(struct nf_ct_sip_master), sip_help_tcp,
+				  NULL, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&sip[i][3]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %u port: %u\n",
+			       sip[i][3].tuple.src.l3num, ports[i]);
+			nf_conntrack_sip_fini();
+			return ret;
 		}
 	}
 	return 0;
diff --git a/net/netfilter/nf_conntrack_tftp.c b/net/netfilter/nf_conntrack_tftp.c
index 36f9640..25776d0 100644
--- a/net/netfilter/nf_conntrack_tftp.c
+++ b/net/netfilter/nf_conntrack_tftp.c
@@ -116,7 +116,7 @@ static void nf_conntrack_tftp_fini(void)
 
 static int __init nf_conntrack_tftp_init(void)
 {
-	int i, j, ret;
+	int i, ret;
 
 	if (ports_c == 0)
 		ports[ports_c++] = TFTP_PORT;
@@ -124,27 +124,25 @@ static int __init nf_conntrack_tftp_init(void)
 	for (i = 0; i < ports_c; i++) {
 		memset(&tftp[i], 0, sizeof(tftp[i]));
 
-		tftp[i][0].tuple.src.l3num = AF_INET;
-		tftp[i][1].tuple.src.l3num = AF_INET6;
-		for (j = 0; j < 2; j++) {
-			tftp[i][j].tuple.dst.protonum = IPPROTO_UDP;
-			tftp[i][j].tuple.src.u.udp.port = htons(ports[i]);
-			tftp[i][j].expect_policy = &tftp_exp_policy;
-			tftp[i][j].me = THIS_MODULE;
-			tftp[i][j].help = tftp_help;
-
-			if (ports[i] == TFTP_PORT)
-				sprintf(tftp[i][j].name, "tftp");
-			else
-				sprintf(tftp[i][j].name, "tftp-%u", i);
-
-			ret = nf_conntrack_helper_register(&tftp[i][j]);
-			if (ret) {
-				pr_err("failed to register helper for pf: %u port: %u\n",
-				       tftp[i][j].tuple.src.l3num, ports[i]);
-				nf_conntrack_tftp_fini();
-				return ret;
-			}
+		nf_ct_helper_init(&tftp[i][0], AF_INET, IPPROTO_UDP, "tftp",
+				  TFTP_PORT, ports[i], &tftp_exp_policy, 0, 0,
+				  tftp_help, NULL, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&tftp[i][0]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %u port: %u\n",
+			       tftp[i][0].tuple.src.l3num, ports[i]);
+			nf_conntrack_tftp_fini();
+			return ret;
+		}
+		nf_ct_helper_init(&tftp[i][1], AF_INET6, IPPROTO_UDP, "tftp",
+				  TFTP_PORT, ports[i], &tftp_exp_policy, 0, 0,
+				  tftp_help, NULL, THIS_MODULE);
+		ret = nf_conntrack_helper_register(&tftp[i][1]);
+		if (ret < 0) {
+			pr_err("failed to register helper for pf: %u port: %u\n",
+			       tftp[i][1].tuple.src.l3num, ports[i]);
+			nf_conntrack_tftp_fini();
+			return ret;
 		}
 	}
 	return 0;
-- 
2.1.4

[PATCH 3/3 nf-next] netfilter: nf_ct_helper: Add nf_conntrack_helpers_register()

[Pablo Neira Ayuso]

From: Gao Feng <fgao@ikuai8.com>

This patch adds nf_conntrack_helpers_register() and
nf_conntrack_helpers_unregister() to consolidate the helper registration
code.

This patch also fixes a crash via "insmod nf_conntrack_ftp.ko ports=76,76".
This happens because the code tries to unregister the helper which was
not actually registered successfully.  Now add new function
nf_conntrack_helpers_register to fix this bug. It would unregister the
right helpers automatically if someone fails.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_helper.h |  3 ++
 net/netfilter/nf_conntrack_ftp.c            | 33 +++++----------------
 net/netfilter/nf_conntrack_helper.c         | 28 ++++++++++++++++++
 net/netfilter/nf_conntrack_irc.c            | 22 ++++++--------
 net/netfilter/nf_conntrack_sane.c           | 32 +++++---------------
 net/netfilter/nf_conntrack_sip.c            | 46 ++++++-----------------------
 net/netfilter/nf_conntrack_tftp.c           | 28 +++++-------------
 7 files changed, 72 insertions(+), 120 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h
index b5e2d7d..a92aed0 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -73,6 +73,9 @@ void nf_ct_helper_init(struct nf_conntrack_helper *helper,
 int nf_conntrack_helper_register(struct nf_conntrack_helper *);
 void nf_conntrack_helper_unregister(struct nf_conntrack_helper *);
 
+int nf_conntrack_helpers_register(struct nf_conntrack_helper *, unsigned int);
+void nf_conntrack_helpers_unregister(struct nf_conntrack_helper *, unsigned int);
+
 struct nf_conn_help *nf_ct_helper_ext_add(struct nf_conn *ct,
 					  struct nf_conntrack_helper *helper,
 					  gfp_t gfp);
diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index 80928c6..02fefb2 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -582,18 +582,7 @@ static const struct nf_conntrack_expect_policy ftp_exp_policy = {
 /* don't make this __exit, since it's called from __init ! */
 static void nf_conntrack_ftp_fini(void)
 {
-	int i, j;
-	for (i = 0; i < ports_c; i++) {
-		for (j = 0; j < 2; j++) {
-			if (ftp[i][j].me == NULL)
-				continue;
-
-			pr_debug("unregistering helper for pf: %d port: %d\n",
-				 ftp[i][j].tuple.src.l3num, ports[i]);
-			nf_conntrack_helper_unregister(&ftp[i][j]);
-		}
-	}
-
+	nf_conntrack_helpers_unregister(&ftp[0][0], ports_c * 2);
 	kfree(ftp_buffer);
 }
 
@@ -615,24 +604,16 @@ static int __init nf_conntrack_ftp_init(void)
 				  FTP_PORT, ports[i], &ftp_exp_policy, 0,
 				  sizeof(struct nf_ct_ftp_master), help,
 				  nf_ct_ftp_from_nlattr, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&ftp[i][0]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %d port: %d\n",
-			       ftp[i][0].tuple.src.l3num, ports[i]);
-			nf_conntrack_ftp_fini();
-			return ret;
-		}
 		nf_ct_helper_init(&ftp[i][1], AF_INET6, IPPROTO_TCP, "ftp",
 				  FTP_PORT, ports[i], &ftp_exp_policy, 0,
 				  sizeof(struct nf_ct_ftp_master), help,
 				  nf_ct_ftp_from_nlattr, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&ftp[i][1]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %d port: %d\n",
-			       ftp[i][1].tuple.src.l3num, ports[i]);
-			nf_conntrack_ftp_fini();
-			return ret;
-		}
+	}
+	ret = nf_conntrack_helpers_register(&ftp[0][0], ports_c * 2);
+	if (ret < 0) {
+		pr_err("failed to register helpers\n");
+		kfree(ftp_buffer);
+		return ret;
 	}
 
 	return 0;
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index dddfefc..057cf50 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -486,6 +486,34 @@ void nf_ct_helper_init(struct nf_conntrack_helper *helper,
 }
 EXPORT_SYMBOL_GPL(nf_ct_helper_init);
 
+int nf_conntrack_helpers_register(struct nf_conntrack_helper *helper,
+				  unsigned int n)
+{
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < n; i++) {
+		err = nf_conntrack_helper_register(&helper[i]);
+		if (err < 0)
+			goto err;
+	}
+
+	return err;
+err:
+	if (i > 0)
+		nf_conntrack_helpers_unregister(helper, i);
+	return err;
+}
+EXPORT_SYMBOL_GPL(nf_conntrack_helpers_register);
+
+void nf_conntrack_helpers_unregister(struct nf_conntrack_helper *helper,
+				     unsigned int n)
+{
+	while (n-- > 0)
+		nf_conntrack_helper_unregister(&helper[n]);
+}
+EXPORT_SYMBOL_GPL(nf_conntrack_helpers_unregister);
+
 static struct nf_ct_ext_type helper_extend __read_mostly = {
 	.len	= sizeof(struct nf_conn_help),
 	.align	= __alignof__(struct nf_conn_help),
diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index bc1a0dd..2a403db 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -232,8 +232,6 @@ static int help(struct sk_buff *skb, unsigned int protoff,
 static struct nf_conntrack_helper irc[MAX_PORTS] __read_mostly;
 static struct nf_conntrack_expect_policy irc_exp_policy;
 
-static void nf_conntrack_irc_fini(void);
-
 static int __init nf_conntrack_irc_init(void)
 {
 	int i, ret;
@@ -258,14 +256,15 @@ static int __init nf_conntrack_irc_init(void)
 		nf_ct_helper_init(&irc[i], AF_INET, IPPROTO_TCP, "irc",
 				  IRC_PORT, ports[i], &irc_exp_policy, 0, 0,
 				  help, NULL, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&irc[i]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %u port: %u\n",
-			       irc[i].tuple.src.l3num, ports[i]);
-			nf_conntrack_irc_fini();
-			return ret;
-		}
 	}
+
+	ret = nf_conntrack_helpers_register(&irc[0], ports_c);
+	if (ret < 0) {
+		pr_err("failed to register helpers\n");
+		kfree(irc_buffer);
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -273,10 +272,7 @@ static int __init nf_conntrack_irc_init(void)
  * it is needed by the init function */
 static void nf_conntrack_irc_fini(void)
 {
-	int i;
-
-	for (i = 0; i < ports_c; i++)
-		nf_conntrack_helper_unregister(&irc[i]);
+	nf_conntrack_helpers_unregister(&irc[0], ports_c);
 	kfree(irc_buffer);
 }
 
diff --git a/net/netfilter/nf_conntrack_sane.c b/net/netfilter/nf_conntrack_sane.c
index d005b14..c93bf8e 100644
--- a/net/netfilter/nf_conntrack_sane.c
+++ b/net/netfilter/nf_conntrack_sane.c
@@ -176,16 +176,7 @@ static const struct nf_conntrack_expect_policy sane_exp_policy = {
 /* don't make this __exit, since it's called from __init ! */
 static void nf_conntrack_sane_fini(void)
 {
-	int i, j;
-
-	for (i = 0; i < ports_c; i++) {
-		for (j = 0; j < 2; j++) {
-			pr_debug("unregistering helper for pf: %d port: %d\n",
-				 sane[i][j].tuple.src.l3num, ports[i]);
-			nf_conntrack_helper_unregister(&sane[i][j]);
-		}
-	}
-
+	nf_conntrack_helpers_unregister(&sane[0][0], ports_c * 2);
 	kfree(sane_buffer);
 }
 
@@ -207,24 +198,17 @@ static int __init nf_conntrack_sane_init(void)
 				  SANE_PORT, ports[i], &sane_exp_policy, 0,
 				  sizeof(struct nf_ct_sane_master), help, NULL,
 				  THIS_MODULE);
-		ret = nf_conntrack_helper_register(&sane[i][0]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %d port: %d\n",
-			       sane[i][0].tuple.src.l3num, ports[i]);
-			nf_conntrack_sane_fini();
-			return ret;
-		}
 		nf_ct_helper_init(&sane[i][1], AF_INET6, IPPROTO_TCP, "sane",
 				  SANE_PORT, ports[i], &sane_exp_policy, 0,
 				  sizeof(struct nf_ct_sane_master), help, NULL,
 				  THIS_MODULE);
-		ret = nf_conntrack_helper_register(&sane[i][1]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %d port: %d\n",
-			       sane[i][1].tuple.src.l3num, ports[i]);
-			nf_conntrack_sane_fini();
-			return ret;
-		}
+	}
+
+	ret = nf_conntrack_helpers_register(&sane[0][0], ports_c * 2);
+	if (ret < 0) {
+		pr_err("failed to register helpers\n");
+		kfree(sane_buffer);
+		return ret;
 	}
 
 	return 0;
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 5951427..c53be45 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1616,15 +1616,8 @@ static const struct nf_conntrack_expect_policy sip_exp_policy[SIP_EXPECT_MAX + 1
 
 static void nf_conntrack_sip_fini(void)
 {
-	int i, j;
-
-	for (i = 0; i < ports_c; i++) {
-		for (j = 0; j < ARRAY_SIZE(sip[i]); j++) {
-			if (sip[i][j].me == NULL)
-				continue;
-			nf_conntrack_helper_unregister(&sip[i][j]);
-		}
-	}
+	nf_conntrack_helpers_unregister(&sip[0][0],
+					ports_c * ARRAY_SIZE(sip[0]));
 }
 
 static int __init nf_conntrack_sip_init(void)
@@ -1642,49 +1635,28 @@ static int __init nf_conntrack_sip_init(void)
 				  SIP_EXPECT_MAX,
 				  sizeof(struct nf_ct_sip_master), sip_help_udp,
 				  NULL, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&sip[i][0]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %u port: %u\n",
-			       sip[i][0].tuple.src.l3num, ports[i]);
-			nf_conntrack_sip_fini();
-			return ret;
-		}
 		nf_ct_helper_init(&sip[i][1], AF_INET, IPPROTO_TCP, "sip",
 				  SIP_PORT, ports[i], &sip_exp_policy[0],
 				  SIP_EXPECT_MAX,
 				  sizeof(struct nf_ct_sip_master), sip_help_tcp,
 				  NULL, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&sip[i][1]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %u port: %u\n",
-			       sip[i][1].tuple.src.l3num, ports[i]);
-			nf_conntrack_sip_fini();
-			return ret;
-		}
 		nf_ct_helper_init(&sip[i][2], AF_INET6, IPPROTO_UDP, "sip",
 				  SIP_PORT, ports[i], &sip_exp_policy[0],
 				  SIP_EXPECT_MAX,
 				  sizeof(struct nf_ct_sip_master), sip_help_udp,
 				  NULL, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&sip[i][2]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %u port: %u\n",
-			       sip[i][2].tuple.src.l3num, ports[i]);
-			nf_conntrack_sip_fini();
-			return ret;
-		}
 		nf_ct_helper_init(&sip[i][3], AF_INET6, IPPROTO_TCP, "sip",
 				  SIP_PORT, ports[i], &sip_exp_policy[0],
 				  SIP_EXPECT_MAX,
 				  sizeof(struct nf_ct_sip_master), sip_help_tcp,
 				  NULL, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&sip[i][3]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %u port: %u\n",
-			       sip[i][3].tuple.src.l3num, ports[i]);
-			nf_conntrack_sip_fini();
-			return ret;
-		}
+	}
+
+	ret = nf_conntrack_helpers_register(&sip[0][0],
+					    ports_c * ARRAY_SIZE(sip[0]));
+	if (ret < 0) {
+		pr_err("failed to register helpers\n");
+		return ret;
 	}
 	return 0;
 }
diff --git a/net/netfilter/nf_conntrack_tftp.c b/net/netfilter/nf_conntrack_tftp.c
index 25776d0..d87c312 100644
--- a/net/netfilter/nf_conntrack_tftp.c
+++ b/net/netfilter/nf_conntrack_tftp.c
@@ -106,12 +106,7 @@ static const struct nf_conntrack_expect_policy tftp_exp_policy = {
 
 static void nf_conntrack_tftp_fini(void)
 {
-	int i, j;
-
-	for (i = 0; i < ports_c; i++) {
-		for (j = 0; j < 2; j++)
-			nf_conntrack_helper_unregister(&tftp[i][j]);
-	}
+	nf_conntrack_helpers_unregister(&tftp[0][0], ports_c * 2);
 }
 
 static int __init nf_conntrack_tftp_init(void)
@@ -127,24 +122,17 @@ static int __init nf_conntrack_tftp_init(void)
 		nf_ct_helper_init(&tftp[i][0], AF_INET, IPPROTO_UDP, "tftp",
 				  TFTP_PORT, ports[i], &tftp_exp_policy, 0, 0,
 				  tftp_help, NULL, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&tftp[i][0]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %u port: %u\n",
-			       tftp[i][0].tuple.src.l3num, ports[i]);
-			nf_conntrack_tftp_fini();
-			return ret;
-		}
 		nf_ct_helper_init(&tftp[i][1], AF_INET6, IPPROTO_UDP, "tftp",
 				  TFTP_PORT, ports[i], &tftp_exp_policy, 0, 0,
 				  tftp_help, NULL, THIS_MODULE);
-		ret = nf_conntrack_helper_register(&tftp[i][1]);
-		if (ret < 0) {
-			pr_err("failed to register helper for pf: %u port: %u\n",
-			       tftp[i][1].tuple.src.l3num, ports[i]);
-			nf_conntrack_tftp_fini();
-			return ret;
-		}
 	}
+
+	ret = nf_conntrack_helpers_register(&tftp[0][0], ports_c * 2);
+	if (ret < 0) {
+		pr_err("failed to register helpers\n");
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.1.4

Re: [PATCH 3/3 nf-next] netfilter: nf_ct_helper: Add nf_conntrack_helpers_register()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]

Hi Gao,

[auto build test WARNING on nf-next/master]

url:    https://github.com/0day-ci/linux/commits/Pablo-Neira-Ayuso/netfilter-nf_ct_helper-define-pr_fmt/20160104-195139
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next master
config: i386-randconfig-x008-01040616 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   net/netfilter/nf_conntrack_helper.c: In function 'nf_conntrack_helpers_register':
>> net/netfilter/nf_conntrack_helper.c:493:6: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]
     int err;
         ^

vim +/err +493 net/netfilter/nf_conntrack_helper.c

   477		helper->help			= help;
   478		helper->from_nlattr		= from_nlattr;
   479		helper->me			= module;
   480	
   481		if (spec_port == default_port)
   482			snprintf(helper->name, sizeof(helper->name), "%s", name);
   483		else
   484			snprintf(helper->name, sizeof(helper->name), "%s-%u", name,
   485				 spec_port);
   486	}
   487	EXPORT_SYMBOL_GPL(nf_ct_helper_init);
   488	
   489	int nf_conntrack_helpers_register(struct nf_conntrack_helper *helper,
   490					  unsigned int n)
   491	{
   492		unsigned int i;
 > 493		int err;
   494	
   495		for (i = 0; i < n; i++) {
   496			err = nf_conntrack_helper_register(&helper[i]);
   497			if (err < 0)
   498				goto err;
   499		}
   500	
   501		return err;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 25616 bytes --]

Re: 答复: [PATCH 3/3 nf-next] netfilter: nf_ct_helper: Add nf_conntrack_helpers_register()

[Pablo Neira Ayuso]

On Mon, Jan 04, 2016 at 10:14:40PM +0800, Feng Gao wrote:
> Hi Pablo,
> 
> I received the warning notification.
> But I just checked the patch file, and find I already initialized the "err"
> in the nf_conntrack_helpers_register.
> 
> http://www.spinics.net/lists/netfilter-devel/msg39270.html

Will fix this here, no problem.

Re: 答复: [PATCH 3/3 nf-next] netfilter: nf_ct_helper: Add nf_conntrack_helpers_register()

[Pablo Neira Ayuso]

On Mon, Jan 04, 2016 at 05:15:50PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Jan 04, 2016 at 10:14:40PM +0800, Feng Gao wrote:
> > Hi Pablo,
> > 
> > I received the warning notification.
> > But I just checked the patch file, and find I already initialized the "err"
> > in the nf_conntrack_helpers_register.
> > 
> > http://www.spinics.net/lists/netfilter-devel/msg39270.html
> 
> Will fix this here, no problem.

BTW, it would be good if you can send us a follow up patch on top of
this series to switch from the bidimensional to unidimensional arrays
to store the helper structures, so we can get rid of the &array[0][0]
thing.

Re: 答复: 答复: [PATCH 3/3 nf-next] netfilter: nf_ct_helper: Add nf_conntrack_helpers_register()

[Pablo Neira Ayuso]

On Fri, Jan 08, 2016 at 08:18:02AM +0800, Feng Gao wrote:
> Hi Pablo,
> 
> I know how to enhance it now, and it must be converted.
> Because there is one bug in the codes when using
> nf_conntrack_helpers_register.
> 
> Take the following codes as an example
>     for (i = 0; i < ports_c; i++) {
>         nf_ct_helper_init(&ftp[i][0], AF_INET, IPPROTO_TCP, "ftp",
>                   FTP_PORT, ports[i], &ftp_exp_policy, 0,
>                   sizeof(struct nf_ct_ftp_master), help,
>                   nf_ct_ftp_from_nlattr, THIS_MODULE);
>         nf_ct_helper_init(&ftp[i][1], AF_INET6, IPPROTO_TCP, "ftp",
>                   FTP_PORT, ports[i], &ftp_exp_policy, 0,
>                   sizeof(struct nf_ct_ftp_master), help,
>                   nf_ct_ftp_from_nlattr, THIS_MODULE);
>     }   
>     ret = nf_conntrack_helpers_register(&ftp[0][0], ports_c * 2);
> 
> When init the helper, we initialized the ftp[i][0], ftp[i][1], they are not
> continuous.
> So it bring one bug when use nf_conntrack_helpers_register(&ftp[0][0],
> ports_c * 2);
> 
> I will fix it later

Then, I going to keep these patchset back in this round, sorry.

Please, take over my 2/3 and 3/3 iteration, fix them and resubmit.

Thanks.

Re: 答复: 答复: 答复: [PATCH 3/3 nf-next] netfilter: nf_ct_helper: Add nf_conntrack_helpers_register()

[Pablo Neira Ayuso]

On Sat, Jan 09, 2016 at 08:09:57AM +0800, Feng Gao wrote:
> Hi Pablo,
> 
> These patches are ok, the memory is continuous. ----------------- I
> misunderstood it. It was deep night, so my status was bad.
> Because the array is helper[MAX_PORT][2], not [2][MAX_PORT].
> 
> I have tested them, and they works well.

Then, please resubmit them. We still have time to integrate them.

> So I only send another patch to enhance the multi-dimension array to
> unidimenssion.

You sent that as a follow up coming after the principal ones.