All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf v6 1/3] netfilter: helper: Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper
@ 2017-05-05  0:55 gfree.wind
  2017-05-05  0:55 ` [PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module gfree.wind
  2017-05-05  0:55 ` [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded gfree.wind
  0 siblings, 2 replies; 6+ messages in thread
From: gfree.wind @ 2017-05-05  0:55 UTC (permalink / raw)
  To: pablo, kadlec, fw, netfilter-devel; +Cc: Gao Feng

From: Gao Feng <gfree.wind@vip.163.com>

Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper, and rename
other functions or variables which refer to it.
The new name is better than the old one.

Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
---
 v6: Rename the helper name of ftp, tftp.. to ftp-nat-follow-master, per Pablo
 v5: Register one nat_helper for every nat module, per Pablo
 v4: Cover the nat_module assignment in dataplane, per Pablo
 v3: Rename the nf_ct_helper_expectfn, func, and member, per Pablo
 v2: Use the module as the identifier when flush expect
 v1: initial version

 include/net/netfilter/nf_conntrack_helper.h | 14 ++++++-------
 net/ipv4/netfilter/nf_nat_h323.c            | 12 +++++------
 net/netfilter/nf_conntrack_helper.c         | 32 ++++++++++++++---------------
 net/netfilter/nf_conntrack_netlink.c        | 16 +++++++--------
 net/netfilter/nf_nat_core.c                 |  6 +++---
 net/netfilter/nf_nat_sip.c                  |  6 +++---
 6 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h
index 1eaac1f..d14fe493 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -111,7 +111,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb, unsigned int protoff,
 				enum ip_conntrack_info ctinfo,
 				unsigned int timeout);
 
-struct nf_ct_helper_expectfn {
+struct nf_ct_nat_helper {
 	struct list_head head;
 	const char *name;
 	void (*expectfn)(struct nf_conn *ct, struct nf_conntrack_expect *exp);
@@ -121,12 +121,12 @@ struct nf_ct_helper_expectfn {
 void nf_ct_helper_log(struct sk_buff *skb, const struct nf_conn *ct,
 		      const char *fmt, ...);
 
-void nf_ct_helper_expectfn_register(struct nf_ct_helper_expectfn *n);
-void nf_ct_helper_expectfn_unregister(struct nf_ct_helper_expectfn *n);
-struct nf_ct_helper_expectfn *
-nf_ct_helper_expectfn_find_by_name(const char *name);
-struct nf_ct_helper_expectfn *
-nf_ct_helper_expectfn_find_by_symbol(const void *symbol);
+void nf_ct_nat_helper_register(struct nf_ct_nat_helper *n);
+void nf_ct_nat_helper_unregister(struct nf_ct_nat_helper *n);
+struct nf_ct_nat_helper *
+nf_ct_nat_helper_find_by_name(const char *name);
+struct nf_ct_nat_helper *
+nf_ct_nat_helper_find_by_symbol(const void *symbol);
 
 extern struct hlist_head *nf_ct_helper_hash;
 extern unsigned int nf_ct_helper_hsize;
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index 574f7eb..346e764 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -567,12 +567,12 @@ static int nat_callforwarding(struct sk_buff *skb, struct nf_conn *ct,
 	return 0;
 }
 
-static struct nf_ct_helper_expectfn q931_nat = {
+static struct nf_ct_nat_helper q931_nat = {
 	.name		= "Q.931",
 	.expectfn	= ip_nat_q931_expect,
 };
 
-static struct nf_ct_helper_expectfn callforwarding_nat = {
+static struct nf_ct_nat_helper callforwarding_nat = {
 	.name		= "callforwarding",
 	.expectfn	= ip_nat_callforwarding_expect,
 };
@@ -599,8 +599,8 @@ static int __init init(void)
 	RCU_INIT_POINTER(nat_h245_hook, nat_h245);
 	RCU_INIT_POINTER(nat_callforwarding_hook, nat_callforwarding);
 	RCU_INIT_POINTER(nat_q931_hook, nat_q931);
-	nf_ct_helper_expectfn_register(&q931_nat);
-	nf_ct_helper_expectfn_register(&callforwarding_nat);
+	nf_ct_nat_helper_register(&q931_nat);
+	nf_ct_nat_helper_register(&callforwarding_nat);
 	return 0;
 }
 
@@ -616,8 +616,8 @@ static void __exit fini(void)
 	RCU_INIT_POINTER(nat_h245_hook, NULL);
 	RCU_INIT_POINTER(nat_callforwarding_hook, NULL);
 	RCU_INIT_POINTER(nat_q931_hook, NULL);
-	nf_ct_helper_expectfn_unregister(&q931_nat);
-	nf_ct_helper_expectfn_unregister(&callforwarding_nat);
+	nf_ct_nat_helper_unregister(&q931_nat);
+	nf_ct_nat_helper_unregister(&callforwarding_nat);
 	synchronize_rcu();
 }
 
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 99bcd44..1fd739c 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -302,32 +302,32 @@ void nf_ct_helper_destroy(struct nf_conn *ct)
 	}
 }
 
-static LIST_HEAD(nf_ct_helper_expectfn_list);
+static LIST_HEAD(nf_ct_nat_helper_list);
 
-void nf_ct_helper_expectfn_register(struct nf_ct_helper_expectfn *n)
+void nf_ct_nat_helper_register(struct nf_ct_nat_helper *n)
 {
 	spin_lock_bh(&nf_conntrack_expect_lock);
-	list_add_rcu(&n->head, &nf_ct_helper_expectfn_list);
+	list_add_rcu(&n->head, &nf_ct_nat_helper_list);
 	spin_unlock_bh(&nf_conntrack_expect_lock);
 }
-EXPORT_SYMBOL_GPL(nf_ct_helper_expectfn_register);
+EXPORT_SYMBOL_GPL(nf_ct_nat_helper_register);
 
-void nf_ct_helper_expectfn_unregister(struct nf_ct_helper_expectfn *n)
+void nf_ct_nat_helper_unregister(struct nf_ct_nat_helper *n)
 {
 	spin_lock_bh(&nf_conntrack_expect_lock);
 	list_del_rcu(&n->head);
 	spin_unlock_bh(&nf_conntrack_expect_lock);
 }
-EXPORT_SYMBOL_GPL(nf_ct_helper_expectfn_unregister);
+EXPORT_SYMBOL_GPL(nf_ct_nat_helper_unregister);
 
 /* Caller should hold the rcu lock */
-struct nf_ct_helper_expectfn *
-nf_ct_helper_expectfn_find_by_name(const char *name)
+struct nf_ct_nat_helper *
+nf_ct_nat_helper_find_by_name(const char *name)
 {
-	struct nf_ct_helper_expectfn *cur;
+	struct nf_ct_nat_helper *cur;
 	bool found = false;
 
-	list_for_each_entry_rcu(cur, &nf_ct_helper_expectfn_list, head) {
+	list_for_each_entry_rcu(cur, &nf_ct_nat_helper_list, head) {
 		if (!strcmp(cur->name, name)) {
 			found = true;
 			break;
@@ -335,16 +335,16 @@ struct nf_ct_helper_expectfn *
 	}
 	return found ? cur : NULL;
 }
-EXPORT_SYMBOL_GPL(nf_ct_helper_expectfn_find_by_name);
+EXPORT_SYMBOL_GPL(nf_ct_nat_helper_find_by_name);
 
 /* Caller should hold the rcu lock */
-struct nf_ct_helper_expectfn *
-nf_ct_helper_expectfn_find_by_symbol(const void *symbol)
+struct nf_ct_nat_helper *
+nf_ct_nat_helper_find_by_symbol(const void *symbol)
 {
-	struct nf_ct_helper_expectfn *cur;
+	struct nf_ct_nat_helper *cur;
 	bool found = false;
 
-	list_for_each_entry_rcu(cur, &nf_ct_helper_expectfn_list, head) {
+	list_for_each_entry_rcu(cur, &nf_ct_nat_helper_list, head) {
 		if (cur->expectfn == symbol) {
 			found = true;
 			break;
@@ -352,7 +352,7 @@ struct nf_ct_helper_expectfn *
 	}
 	return found ? cur : NULL;
 }
-EXPORT_SYMBOL_GPL(nf_ct_helper_expectfn_find_by_symbol);
+EXPORT_SYMBOL_GPL(nf_ct_nat_helper_find_by_symbol);
 
 __printf(3, 4)
 void nf_ct_helper_log(struct sk_buff *skb, const struct nf_conn *ct,
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 78f8c9a..0e3d1af 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2522,7 +2522,7 @@ static int ctnetlink_exp_dump_mask(struct sk_buff *skb,
 	struct nlattr *nest_parms;
 	struct nf_conntrack_tuple nat_tuple = {};
 #endif
-	struct nf_ct_helper_expectfn *expfn;
+	struct nf_ct_nat_helper *nat_helper;
 
 	if (timeout < 0)
 		timeout = 0;
@@ -2571,9 +2571,9 @@ static int ctnetlink_exp_dump_mask(struct sk_buff *skb,
 		    nla_put_string(skb, CTA_EXPECT_HELP_NAME, helper->name))
 			goto nla_put_failure;
 	}
-	expfn = nf_ct_helper_expectfn_find_by_symbol(exp->expectfn);
-	if (expfn != NULL &&
-	    nla_put_string(skb, CTA_EXPECT_FN, expfn->name))
+	nat_helper = nf_ct_nat_helper_find_by_symbol(exp->expectfn);
+	if (!nat_helper &&
+	    nla_put_string(skb, CTA_EXPECT_FN, nat_helper->name))
 		goto nla_put_failure;
 
 	return 0;
@@ -3090,14 +3090,14 @@ static int ctnetlink_del_expect(struct net *net, struct sock *ctnl,
 	}
 	if (cda[CTA_EXPECT_FN]) {
 		const char *name = nla_data(cda[CTA_EXPECT_FN]);
-		struct nf_ct_helper_expectfn *expfn;
+		struct nf_ct_nat_helper *nat_helper;
 
-		expfn = nf_ct_helper_expectfn_find_by_name(name);
-		if (expfn == NULL) {
+		nat_helper = nf_ct_nat_helper_find_by_name(name);
+		if (!nat_helper) {
 			err = -EINVAL;
 			goto err_out;
 		}
-		exp->expectfn = expfn->expectfn;
+		exp->expectfn = nat_helper->expectfn;
 	} else
 		exp->expectfn = NULL;
 
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 82802e4..2897bd4 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -848,7 +848,7 @@ static void __net_exit nf_nat_net_exit(struct net *net)
 	.exit = nf_nat_net_exit,
 };
 
-static struct nf_ct_helper_expectfn follow_master_nat = {
+static struct nf_ct_nat_helper follow_master_nat = {
 	.name		= "nat-follow-master",
 	.expectfn	= nf_nat_follow_master,
 };
@@ -872,7 +872,7 @@ static int __init nf_nat_init(void)
 	if (ret < 0)
 		goto cleanup_extend;
 
-	nf_ct_helper_expectfn_register(&follow_master_nat);
+	nf_ct_nat_helper_register(&follow_master_nat);
 
 	/* Initialize fake conntrack so that NAT will skip it */
 	nf_ct_untracked_status_or(IPS_NAT_DONE_MASK);
@@ -898,7 +898,7 @@ static void __exit nf_nat_cleanup(void)
 
 	unregister_pernet_subsys(&nf_nat_net_ops);
 	nf_ct_extend_unregister(&nat_extend);
-	nf_ct_helper_expectfn_unregister(&follow_master_nat);
+	nf_ct_nat_helper_unregister(&follow_master_nat);
 	RCU_INIT_POINTER(nfnetlink_parse_nat_setup_hook, NULL);
 #ifdef CONFIG_XFRM
 	RCU_INIT_POINTER(nf_nat_decode_session_hook, NULL);
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index 791fac4..d27c5a2 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -618,7 +618,7 @@ static unsigned int nf_nat_sdp_media(struct sk_buff *skb, unsigned int protoff,
 	return NF_DROP;
 }
 
-static struct nf_ct_helper_expectfn sip_nat = {
+static struct nf_ct_nat_helper sip_nat = {
 	.name		= "sip",
 	.expectfn	= nf_nat_sip_expected,
 };
@@ -627,7 +627,7 @@ static void __exit nf_nat_sip_fini(void)
 {
 	RCU_INIT_POINTER(nf_nat_sip_hooks, NULL);
 
-	nf_ct_helper_expectfn_unregister(&sip_nat);
+	nf_ct_nat_helper_unregister(&sip_nat);
 	synchronize_rcu();
 }
 
@@ -645,7 +645,7 @@ static int __init nf_nat_sip_init(void)
 {
 	BUG_ON(nf_nat_sip_hooks != NULL);
 	RCU_INIT_POINTER(nf_nat_sip_hooks, &sip_hooks);
-	nf_ct_helper_expectfn_register(&sip_nat);
+	nf_ct_nat_helper_register(&sip_nat);
 	return 0;
 }
 
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module
  2017-05-05  0:55 [PATCH nf v6 1/3] netfilter: helper: Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper gfree.wind
@ 2017-05-05  0:55 ` gfree.wind
  2017-05-15 17:20   ` Pablo Neira Ayuso
  2017-05-05  0:55 ` [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded gfree.wind
  1 sibling, 1 reply; 6+ messages in thread
From: gfree.wind @ 2017-05-05  0:55 UTC (permalink / raw)
  To: pablo, kadlec, fw, netfilter-devel; +Cc: Gao Feng

From: Gao Feng <gfree.wind@vip.163.com>

There are multiple proto nat modules which depend on the follow_master_nat
in the nf_nat_core. When this module is gone, all modules which refers to
it could not work well.

Now register one struct nf_ct_nat_helper in every proto nat module, it makes
sure every nat module could not effect each other, and it is helpful to
maintain the codes. The new nf_ct_nat_helper could be found by ctlink with
its name too, while the original "nat-follow-master" helper is not effected.
ctlink still could specify the "nat-follow-master" for ftp, but recommend
use the ftp's nat_helper "ftp-nat-follow-master" as the ftp nat_helper.

The following are all modifications.
1. Every proto nat module registers one nat_helper at least;
2. Replace the expectfn with nat_helper in the nf_conntrack_expect;
It is used to the latter commit which add the nat_helper module check.
3. Remove nf_ct_nat_helper_find_by_symbol, it is useless now.

Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
---
 v6: Rename the helper name of ftp, tftp.. to ftp-nat-follow-master, per Pablo
 v5: Register one nat_helper for every nat module, per Pablo
 v4: Cover the nat_module assignment in dataplane, per Pablo
 v3: Rename the nf_ct_helper_expectfn, func, and member, per Pablo
 v2: Use the module as the identifier when flush expect
 v1: initial version

 include/net/netfilter/nf_conntrack_expect.h |  5 ++--
 include/net/netfilter/nf_conntrack_helper.h |  2 --
 net/ipv4/netfilter/nf_nat_h323.c            | 44 ++++++++++++++++++-----------
 net/netfilter/ipvs/ip_vs_nfct.c             |  7 ++++-
 net/netfilter/nf_conntrack_broadcast.c      |  2 +-
 net/netfilter/nf_conntrack_core.c           |  4 +--
 net/netfilter/nf_conntrack_expect.c         |  2 +-
 net/netfilter/nf_conntrack_netlink.c        | 14 ++++-----
 net/netfilter/nf_conntrack_pptp.c           | 14 +++++++--
 net/netfilter/nf_nat_amanda.c               |  9 +++++-
 net/netfilter/nf_nat_ftp.c                  |  9 +++++-
 net/netfilter/nf_nat_irc.c                  |  9 +++++-
 net/netfilter/nf_nat_sip.c                  | 18 ++++++------
 net/netfilter/nf_nat_tftp.c                 |  9 +++++-
 14 files changed, 101 insertions(+), 47 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
index 5ed33ea..f665a6b 100644
--- a/include/net/netfilter/nf_conntrack_expect.h
+++ b/include/net/netfilter/nf_conntrack_expect.h
@@ -23,9 +23,8 @@ struct nf_conntrack_expect {
 	struct nf_conntrack_tuple tuple;
 	struct nf_conntrack_tuple_mask mask;
 
-	/* Function to call after setup and insertion */
-	void (*expectfn)(struct nf_conn *new,
-			 struct nf_conntrack_expect *this);
+	/* Expectation function owner */
+	struct nf_ct_nat_helper *nat_helper;
 
 	/* Helper to assign to new connection */
 	struct nf_conntrack_helper *helper;
diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h
index d14fe493..e8d31ca 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -125,8 +125,6 @@ void nf_ct_helper_log(struct sk_buff *skb, const struct nf_conn *ct,
 void nf_ct_nat_helper_unregister(struct nf_ct_nat_helper *n);
 struct nf_ct_nat_helper *
 nf_ct_nat_helper_find_by_name(const char *name);
-struct nf_ct_nat_helper *
-nf_ct_nat_helper_find_by_symbol(const void *symbol);
 
 extern struct hlist_head *nf_ct_helper_hash;
 extern unsigned int nf_ct_helper_hsize;
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index 346e764..ce2095c 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -21,6 +21,26 @@
 #include <linux/netfilter/nf_conntrack_h323.h>
 
 /****************************************************************************/
+static void ip_nat_q931_expect(struct nf_conn *new,
+			       struct nf_conntrack_expect *this);
+static void ip_nat_callforwarding_expect(struct nf_conn *new,
+					 struct nf_conntrack_expect *this);
+
+static struct nf_ct_nat_helper q931_nat = {
+	.name		= "Q.931",
+	.expectfn	= ip_nat_q931_expect,
+};
+
+static struct nf_ct_nat_helper callforwarding_nat = {
+	.name		= "callforwarding",
+	.expectfn	= ip_nat_callforwarding_expect,
+};
+
+static struct nf_ct_nat_helper follow_master_nat = {
+	.name		= "h323-nat-follow-master",
+	.expectfn	= nf_nat_follow_master,
+};
+
 static int set_addr(struct sk_buff *skb, unsigned int protoff,
 		    unsigned char **data, int dataoff,
 		    unsigned int addroff, __be32 ip, __be16 port)
@@ -187,10 +207,10 @@ static int nat_rtp_rtcp(struct sk_buff *skb, struct nf_conn *ct,
 
 	/* Set expectations for NAT */
 	rtp_exp->saved_proto.udp.port = rtp_exp->tuple.dst.u.udp.port;
-	rtp_exp->expectfn = nf_nat_follow_master;
+	rtp_exp->nat_helper = &follow_master_nat;
 	rtp_exp->dir = !dir;
 	rtcp_exp->saved_proto.udp.port = rtcp_exp->tuple.dst.u.udp.port;
-	rtcp_exp->expectfn = nf_nat_follow_master;
+	rtcp_exp->nat_helper = &follow_master_nat;
 	rtcp_exp->dir = !dir;
 
 	/* Lookup existing expects */
@@ -289,7 +309,7 @@ static int nat_t120(struct sk_buff *skb, struct nf_conn *ct,
 
 	/* Set expectations for NAT */
 	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
-	exp->expectfn = nf_nat_follow_master;
+	exp->nat_helper = &follow_master_nat;
 	exp->dir = !dir;
 
 	/* Try to get same port: if not, try to change it. */
@@ -341,7 +361,7 @@ static int nat_h245(struct sk_buff *skb, struct nf_conn *ct,
 
 	/* Set expectations for NAT */
 	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
-	exp->expectfn = nf_nat_follow_master;
+	exp->nat_helper = &follow_master_nat;
 	exp->dir = !dir;
 
 	/* Check existing expects */
@@ -433,7 +453,7 @@ static int nat_q931(struct sk_buff *skb, struct nf_conn *ct,
 
 	/* Set expectations for NAT */
 	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
-	exp->expectfn = ip_nat_q931_expect;
+	exp->nat_helper = &q931_nat;
 	exp->dir = !dir;
 
 	/* Check existing expects */
@@ -527,7 +547,7 @@ static int nat_callforwarding(struct sk_buff *skb, struct nf_conn *ct,
 	exp->saved_addr = exp->tuple.dst.u3;
 	exp->tuple.dst.u3.ip = ct->tuplehash[!dir].tuple.dst.u3.ip;
 	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
-	exp->expectfn = ip_nat_callforwarding_expect;
+	exp->nat_helper = &callforwarding_nat;
 	exp->dir = !dir;
 
 	/* Try to get same port: if not, try to change it. */
@@ -567,16 +587,6 @@ static int nat_callforwarding(struct sk_buff *skb, struct nf_conn *ct,
 	return 0;
 }
 
-static struct nf_ct_nat_helper q931_nat = {
-	.name		= "Q.931",
-	.expectfn	= ip_nat_q931_expect,
-};
-
-static struct nf_ct_nat_helper callforwarding_nat = {
-	.name		= "callforwarding",
-	.expectfn	= ip_nat_callforwarding_expect,
-};
-
 /****************************************************************************/
 static int __init init(void)
 {
@@ -601,6 +611,7 @@ static int __init init(void)
 	RCU_INIT_POINTER(nat_q931_hook, nat_q931);
 	nf_ct_nat_helper_register(&q931_nat);
 	nf_ct_nat_helper_register(&callforwarding_nat);
+	nf_ct_nat_helper_register(&follow_master_nat);
 	return 0;
 }
 
@@ -618,6 +629,7 @@ static void __exit fini(void)
 	RCU_INIT_POINTER(nat_q931_hook, NULL);
 	nf_ct_nat_helper_unregister(&q931_nat);
 	nf_ct_nat_helper_unregister(&callforwarding_nat);
+	nf_ct_nat_helper_unregister(&follow_master_nat);
 	synchronize_rcu();
 }
 
diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
index fc230d9..fc5af4f 100644
--- a/net/netfilter/ipvs/ip_vs_nfct.c
+++ b/net/netfilter/ipvs/ip_vs_nfct.c
@@ -220,6 +220,11 @@ static void ip_vs_nfct_expect_callback(struct nf_conn *ct,
 	return;
 }
 
+static struct nf_ct_nat_helper ip_vs_nat = {
+	.name = "ip-vs-nat",
+	.expectfn = ip_vs_nfct_expect_callback,
+};
+
 /*
  * Create NF conntrack expectation with wildcard (optional) source port.
  * Then the default callback function will alter the reply and will confirm
@@ -245,7 +250,7 @@ void ip_vs_nfct_expect_related(struct sk_buff *skb, struct nf_conn *ct,
 			proto, port ? &port : NULL,
 			from_rs ? &cp->cport : &cp->vport);
 
-	exp->expectfn = ip_vs_nfct_expect_callback;
+	exp->nat_helper = &ip_vs_nat;
 
 	IP_VS_DBG(7, "%s: ct=%p, expect tuple=" FMT_TUPLE "\n",
 		__func__, ct, ARG_TUPLE(&exp->tuple));
diff --git a/net/netfilter/nf_conntrack_broadcast.c b/net/netfilter/nf_conntrack_broadcast.c
index 4e99cca..114e042 100644
--- a/net/netfilter/nf_conntrack_broadcast.c
+++ b/net/netfilter/nf_conntrack_broadcast.c
@@ -65,7 +65,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
 	exp->mask.src.u3.ip       = mask;
 	exp->mask.src.u.udp.port  = htons(0xFFFF);
 
-	exp->expectfn             = NULL;
+	exp->nat_helper           = NULL;
 	exp->flags                = NF_CT_EXPECT_PERMANENT;
 	exp->class		  = NF_CT_EXPECT_CLASS_DEFAULT;
 	exp->helper               = NULL;
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ffb78e5..ae61513 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1233,8 +1233,8 @@ void nf_conntrack_free(struct nf_conn *ct)
 	local_bh_enable();
 
 	if (exp) {
-		if (exp->expectfn)
-			exp->expectfn(ct, exp);
+		if (exp->nat_helper)
+			exp->nat_helper->expectfn(ct, exp);
 		nf_ct_expect_put(exp);
 	}
 
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index d800730..aef8e18 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -295,7 +295,7 @@ void nf_ct_expect_init(struct nf_conntrack_expect *exp, unsigned int class,
 
 	exp->flags = 0;
 	exp->class = class;
-	exp->expectfn = NULL;
+	exp->nat_helper = NULL;
 	exp->helper = NULL;
 	exp->tuple.src.l3num = family;
 	exp->tuple.dst.protonum = proto;
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 0e3d1af..2a7ade6 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2522,7 +2522,6 @@ static int ctnetlink_exp_dump_mask(struct sk_buff *skb,
 	struct nlattr *nest_parms;
 	struct nf_conntrack_tuple nat_tuple = {};
 #endif
-	struct nf_ct_nat_helper *nat_helper;
 
 	if (timeout < 0)
 		timeout = 0;
@@ -2571,9 +2570,9 @@ static int ctnetlink_exp_dump_mask(struct sk_buff *skb,
 		    nla_put_string(skb, CTA_EXPECT_HELP_NAME, helper->name))
 			goto nla_put_failure;
 	}
-	nat_helper = nf_ct_nat_helper_find_by_symbol(exp->expectfn);
-	if (!nat_helper &&
-	    nla_put_string(skb, CTA_EXPECT_FN, nat_helper->name))
+
+	if (!exp->nat_helper &&
+	    nla_put_string(skb, CTA_EXPECT_FN, exp->nat_helper->name))
 		goto nla_put_failure;
 
 	return 0;
@@ -3097,9 +3096,10 @@ static int ctnetlink_del_expect(struct net *net, struct sock *ctnl,
 			err = -EINVAL;
 			goto err_out;
 		}
-		exp->expectfn = nat_helper->expectfn;
-	} else
-		exp->expectfn = NULL;
+		exp->nat_helper = nat_helper;
+	} else {
+		exp->nat_helper = NULL;
+	}
 
 	exp->class = class;
 	exp->master = ct;
diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c
index f60a475..896b154 100644
--- a/net/netfilter/nf_conntrack_pptp.c
+++ b/net/netfilter/nf_conntrack_pptp.c
@@ -69,6 +69,9 @@
 			     struct nf_conntrack_expect *exp) __read_mostly;
 EXPORT_SYMBOL_GPL(nf_nat_pptp_hook_expectfn);
 
+static void pptp_expectfn(struct nf_conn *ct,
+			  struct nf_conntrack_expect *exp);
+
 #if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
 /* PptpControlMessageType names */
 const char *const pptp_msg_name[] = {
@@ -99,6 +102,11 @@
 #define PPTP_GRE_TIMEOUT 		(10 MINS)
 #define PPTP_GRE_STREAM_TIMEOUT 	(5 HOURS)
 
+static struct nf_ct_nat_helper pptp_nat = {
+	.name			= "pptp-nat",
+	.expectfn		= pptp_expectfn,
+};
+
 static void pptp_expectfn(struct nf_conn *ct,
 			 struct nf_conntrack_expect *exp)
 {
@@ -221,7 +229,7 @@ static int exp_gre(struct nf_conn *ct, __be16 callid, __be16 peer_callid)
 			  &ct->tuplehash[dir].tuple.src.u3,
 			  &ct->tuplehash[dir].tuple.dst.u3,
 			  IPPROTO_GRE, &peer_callid, &callid);
-	exp_orig->expectfn = pptp_expectfn;
+	exp_orig->nat_helper = &pptp_nat;
 
 	/* reply direction, PAC->PNS */
 	dir = IP_CT_DIR_REPLY;
@@ -230,7 +238,7 @@ static int exp_gre(struct nf_conn *ct, __be16 callid, __be16 peer_callid)
 			  &ct->tuplehash[dir].tuple.src.u3,
 			  &ct->tuplehash[dir].tuple.dst.u3,
 			  IPPROTO_GRE, &callid, &peer_callid);
-	exp_reply->expectfn = pptp_expectfn;
+	exp_reply->nat_helper = &pptp_nat;
 
 	nf_nat_pptp_exp_gre = rcu_dereference(nf_nat_pptp_hook_exp_gre);
 	if (nf_nat_pptp_exp_gre && ct->status & IPS_NAT_MASK)
@@ -607,11 +615,13 @@ static int exp_gre(struct nf_conn *ct, __be16 callid, __be16 peer_callid)
 
 static int __init nf_conntrack_pptp_init(void)
 {
+	nf_ct_nat_helper_register(&pptp_nat);
 	return nf_conntrack_helper_register(&pptp);
 }
 
 static void __exit nf_conntrack_pptp_fini(void)
 {
+	nf_ct_nat_helper_unregister(&pptp_nat);
 	nf_conntrack_helper_unregister(&pptp);
 }
 
diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c
index eb77238..9b540a9 100644
--- a/net/netfilter/nf_nat_amanda.c
+++ b/net/netfilter/nf_nat_amanda.c
@@ -24,6 +24,11 @@
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ip_nat_amanda");
 
+static struct nf_ct_nat_helper amanda_nat = {
+	.name = "amanda-nat-follow-master",
+	.expectfn = nf_nat_follow_master,
+};
+
 static unsigned int help(struct sk_buff *skb,
 			 enum ip_conntrack_info ctinfo,
 			 unsigned int protoff,
@@ -41,7 +46,7 @@ static unsigned int help(struct sk_buff *skb,
 
 	/* When you see the packet, we need to NAT it the same as the
 	 * this one (ie. same IP: it will be TCP and master is UDP). */
-	exp->expectfn = nf_nat_follow_master;
+	exp->nat_helper = &amanda_nat;
 
 	/* Try to get same port: if not, try to change it. */
 	for (port = ntohs(exp->saved_proto.tcp.port); port != 0; port++) {
@@ -76,12 +81,14 @@ static unsigned int help(struct sk_buff *skb,
 static void __exit nf_nat_amanda_fini(void)
 {
 	RCU_INIT_POINTER(nf_nat_amanda_hook, NULL);
+	nf_ct_nat_helper_unregister(&amanda_nat);
 	synchronize_rcu();
 }
 
 static int __init nf_nat_amanda_init(void)
 {
 	BUG_ON(nf_nat_amanda_hook != NULL);
+	nf_ct_nat_helper_register(&amanda_nat);
 	RCU_INIT_POINTER(nf_nat_amanda_hook, help);
 	return 0;
 }
diff --git a/net/netfilter/nf_nat_ftp.c b/net/netfilter/nf_nat_ftp.c
index e84a578..f1f4501 100644
--- a/net/netfilter/nf_nat_ftp.c
+++ b/net/netfilter/nf_nat_ftp.c
@@ -24,6 +24,11 @@
 MODULE_DESCRIPTION("ftp NAT helper");
 MODULE_ALIAS("ip_nat_ftp");
 
+static struct nf_ct_nat_helper ftp_nat = {
+	.name = "ftp-nat-follow-master",
+	.expectfn = nf_nat_follow_master,
+};
+
 /* FIXME: Time out? --RR */
 
 static int nf_nat_ftp_fmt_cmd(struct nf_conn *ct, enum nf_ct_ftp_type type,
@@ -80,7 +85,7 @@ static unsigned int nf_nat_ftp(struct sk_buff *skb,
 
 	/* When you see the packet, we need to NAT it the same as the
 	 * this one. */
-	exp->expectfn = nf_nat_follow_master;
+	exp->nat_helper = &ftp_nat;
 
 	/* Try to get same port: if not, try to change it. */
 	for (port = ntohs(exp->saved_proto.tcp.port); port != 0; port++) {
@@ -123,12 +128,14 @@ static unsigned int nf_nat_ftp(struct sk_buff *skb,
 static void __exit nf_nat_ftp_fini(void)
 {
 	RCU_INIT_POINTER(nf_nat_ftp_hook, NULL);
+	nf_ct_nat_helper_unregister(&ftp_nat);
 	synchronize_rcu();
 }
 
 static int __init nf_nat_ftp_init(void)
 {
 	BUG_ON(nf_nat_ftp_hook != NULL);
+	nf_ct_nat_helper_register(&ftp_nat);
 	RCU_INIT_POINTER(nf_nat_ftp_hook, nf_nat_ftp);
 	return 0;
 }
diff --git a/net/netfilter/nf_nat_irc.c b/net/netfilter/nf_nat_irc.c
index 1fb2258..cf623d3 100644
--- a/net/netfilter/nf_nat_irc.c
+++ b/net/netfilter/nf_nat_irc.c
@@ -26,6 +26,11 @@
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ip_nat_irc");
 
+static struct nf_ct_nat_helper irc_nat = {
+	.name = "irc-nat-follow-master",
+	.expectfn = nf_nat_follow_master,
+};
+
 static unsigned int help(struct sk_buff *skb,
 			 enum ip_conntrack_info ctinfo,
 			 unsigned int protoff,
@@ -44,7 +49,7 @@ static unsigned int help(struct sk_buff *skb,
 
 	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
 	exp->dir = IP_CT_DIR_REPLY;
-	exp->expectfn = nf_nat_follow_master;
+	exp->nat_helper = &irc_nat;
 
 	/* Try to get same port: if not, try to change it. */
 	for (port = ntohs(exp->saved_proto.tcp.port); port != 0; port++) {
@@ -96,12 +101,14 @@ static unsigned int help(struct sk_buff *skb,
 static void __exit nf_nat_irc_fini(void)
 {
 	RCU_INIT_POINTER(nf_nat_irc_hook, NULL);
+	nf_ct_nat_helper_unregister(&irc_nat);
 	synchronize_rcu();
 }
 
 static int __init nf_nat_irc_init(void)
 {
 	BUG_ON(nf_nat_irc_hook != NULL);
+	nf_ct_nat_helper_register(&irc_nat);
 	RCU_INIT_POINTER(nf_nat_irc_hook, help);
 	return 0;
 }
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index d27c5a2..cd9333c 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -28,6 +28,13 @@
 MODULE_DESCRIPTION("SIP NAT helper");
 MODULE_ALIAS("ip_nat_sip");
 
+static void nf_nat_sip_expected(struct nf_conn *ct,
+				struct nf_conntrack_expect *exp);
+
+static struct nf_ct_nat_helper sip_nat = {
+	.name		= "sip",
+	.expectfn	= nf_nat_sip_expected,
+};
 
 static unsigned int mangle_packet(struct sk_buff *skb, unsigned int protoff,
 				  unsigned int dataoff,
@@ -376,7 +383,7 @@ static unsigned int nf_nat_sip_expect(struct sk_buff *skb, unsigned int protoff,
 	exp->tuple.dst.u3 = newaddr;
 	exp->saved_proto.udp.port = exp->tuple.dst.u.udp.port;
 	exp->dir = !dir;
-	exp->expectfn = nf_nat_sip_expected;
+	exp->nat_helper = &sip_nat;
 
 	for (; port != 0; port++) {
 		int ret;
@@ -561,13 +568,13 @@ static unsigned int nf_nat_sdp_media(struct sk_buff *skb, unsigned int protoff,
 	rtp_exp->tuple.dst.u3 = *rtp_addr;
 	rtp_exp->saved_proto.udp.port = rtp_exp->tuple.dst.u.udp.port;
 	rtp_exp->dir = !dir;
-	rtp_exp->expectfn = nf_nat_sip_expected;
+	rtp_exp->nat_helper = &sip_nat;
 
 	rtcp_exp->saved_addr = rtcp_exp->tuple.dst.u3;
 	rtcp_exp->tuple.dst.u3 = *rtp_addr;
 	rtcp_exp->saved_proto.udp.port = rtcp_exp->tuple.dst.u.udp.port;
 	rtcp_exp->dir = !dir;
-	rtcp_exp->expectfn = nf_nat_sip_expected;
+	rtcp_exp->nat_helper = &sip_nat;
 
 	/* Try to get same pair of ports: if not, try to change them. */
 	for (port = ntohs(rtp_exp->tuple.dst.u.udp.port);
@@ -618,11 +625,6 @@ static unsigned int nf_nat_sdp_media(struct sk_buff *skb, unsigned int protoff,
 	return NF_DROP;
 }
 
-static struct nf_ct_nat_helper sip_nat = {
-	.name		= "sip",
-	.expectfn	= nf_nat_sip_expected,
-};
-
 static void __exit nf_nat_sip_fini(void)
 {
 	RCU_INIT_POINTER(nf_nat_sip_hooks, NULL);
diff --git a/net/netfilter/nf_nat_tftp.c b/net/netfilter/nf_nat_tftp.c
index 7f67e1d..d279b41 100644
--- a/net/netfilter/nf_nat_tftp.c
+++ b/net/netfilter/nf_nat_tftp.c
@@ -18,6 +18,11 @@
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ip_nat_tftp");
 
+static struct nf_ct_nat_helper tftp_nat = {
+	.name = "tftp-nat-follow-master",
+	.expectfn = nf_nat_follow_master,
+};
+
 static unsigned int help(struct sk_buff *skb,
 			 enum ip_conntrack_info ctinfo,
 			 struct nf_conntrack_expect *exp)
@@ -27,7 +32,7 @@ static unsigned int help(struct sk_buff *skb,
 	exp->saved_proto.udp.port
 		= ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u.udp.port;
 	exp->dir = IP_CT_DIR_REPLY;
-	exp->expectfn = nf_nat_follow_master;
+	exp->nat_helper = &tftp_nat;
 	if (nf_ct_expect_related(exp) != 0) {
 		nf_ct_helper_log(skb, exp->master, "cannot add expectation");
 		return NF_DROP;
@@ -38,12 +43,14 @@ static unsigned int help(struct sk_buff *skb,
 static void __exit nf_nat_tftp_fini(void)
 {
 	RCU_INIT_POINTER(nf_nat_tftp_hook, NULL);
+	nf_ct_nat_helper_unregister(&tftp_nat);
 	synchronize_rcu();
 }
 
 static int __init nf_nat_tftp_init(void)
 {
 	BUG_ON(nf_nat_tftp_hook != NULL);
+	nf_ct_nat_helper_register(&tftp_nat);
 	RCU_INIT_POINTER(nf_nat_tftp_hook, help);
 	return 0;
 }
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded
  2017-05-05  0:55 [PATCH nf v6 1/3] netfilter: helper: Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper gfree.wind
  2017-05-05  0:55 ` [PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module gfree.wind
@ 2017-05-05  0:55 ` gfree.wind
  2017-05-05  1:59   ` Liping Zhang
  1 sibling, 1 reply; 6+ messages in thread
From: gfree.wind @ 2017-05-05  0:55 UTC (permalink / raw)
  To: pablo, kadlec, fw, netfilter-devel; +Cc: Gao Feng

From: Gao Feng <gfree.wind@vip.163.com>

Because the conntrack NAT module could be rmmod anytime, so we should
really leave things in clean state if such thing happens and make sure
we don't leave any packet running over code that will be gone after the
removal.

We only removed the expectations when unregister conntrack helper before.
Actually it is necessary too when remove the nat helper.

Now add one new struct module member "me" in the nf_ct_nat_helper to
represent which module it belongs to.

Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
---
 v6: Rename the helper name of ftp, tftp.. to ftp-nat-follow-master, per Pablo
 v5: Register one nat_helper for every nat module, per Pablo
 v4: Cover the nat_module assignment in dataplane, per Pablo
 v3: Rename the nf_ct_helper_expectfn, func, and member, per Pablo
 v2: Use the module as the identifier when flush expect
 v1: initial version

 include/net/netfilter/nf_conntrack_helper.h |  1 +
 net/ipv4/netfilter/nf_nat_h323.c            |  3 ++
 net/netfilter/ipvs/ip_vs_nfct.c             |  1 +
 net/netfilter/nf_conntrack_helper.c         | 62 +++++++++++++++++++----------
 net/netfilter/nf_conntrack_pptp.c           |  1 +
 net/netfilter/nf_nat_amanda.c               |  1 +
 net/netfilter/nf_nat_core.c                 |  1 +
 net/netfilter/nf_nat_ftp.c                  |  1 +
 net/netfilter/nf_nat_irc.c                  |  1 +
 net/netfilter/nf_nat_sip.c                  |  1 +
 net/netfilter/nf_nat_tftp.c                 |  1 +
 11 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h
index e8d31ca..d70c5be 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -114,6 +114,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb, unsigned int protoff,
 struct nf_ct_nat_helper {
 	struct list_head head;
 	const char *name;
+	struct module *me;
 	void (*expectfn)(struct nf_conn *ct, struct nf_conntrack_expect *exp);
 };
 
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index ce2095c..d7b40ac 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -28,16 +28,19 @@ static void ip_nat_callforwarding_expect(struct nf_conn *new,
 
 static struct nf_ct_nat_helper q931_nat = {
 	.name		= "Q.931",
+	.me		= THIS_MODULE,
 	.expectfn	= ip_nat_q931_expect,
 };
 
 static struct nf_ct_nat_helper callforwarding_nat = {
 	.name		= "callforwarding",
+	.me		= THIS_MODULE,
 	.expectfn	= ip_nat_callforwarding_expect,
 };
 
 static struct nf_ct_nat_helper follow_master_nat = {
 	.name		= "h323-nat-follow-master",
+	.me		= THIS_MODULE,
 	.expectfn	= nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
index fc5af4f..d61c236 100644
--- a/net/netfilter/ipvs/ip_vs_nfct.c
+++ b/net/netfilter/ipvs/ip_vs_nfct.c
@@ -222,6 +222,7 @@ static void ip_vs_nfct_expect_callback(struct nf_conn *ct,
 
 static struct nf_ct_nat_helper ip_vs_nat = {
 	.name = "ip-vs-nat",
+	.me = THIS_MODULE,
 	.expectfn = ip_vs_nfct_expect_callback,
 };
 
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 1fd739c..c12de31 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -130,6 +130,37 @@ static unsigned int helper_hash(const struct nf_conntrack_tuple *tuple)
 	return NULL;
 }
 
+static void
+nf_ct_flush_expect(const struct module *me)
+{
+	struct nf_conntrack_expect *exp;
+	const struct hlist_node *next;
+	u32 i;
+
+	if (!me)
+		return;
+
+	/* Get rid of expectations */
+	spin_lock_bh(&nf_conntrack_expect_lock);
+	for (i = 0; i < nf_ct_expect_hsize; i++) {
+		hlist_for_each_entry_safe(exp, next,
+					  &nf_ct_expect_hash[i], hnode) {
+			struct nf_conn_help *master_help = nfct_help(exp->master);
+
+			if ((master_help->helper && master_help->helper->me == me) ||
+			    (exp->helper && exp->helper->me == me) ||
+			    (exp->nat_helper && exp->nat_helper->me == me)) {
+				/* This expect belongs to the dying module */
+				if (del_timer(&exp->timeout)) {
+					nf_ct_unlink_expect(exp);
+					nf_ct_expect_put(exp);
+				}
+			}
+		}
+	}
+	spin_unlock_bh(&nf_conntrack_expect_lock);
+}
+
 struct nf_conntrack_helper *
 __nf_conntrack_helper_find(const char *name, u16 l3num, u8 protonum)
 {
@@ -317,6 +348,12 @@ void nf_ct_nat_helper_unregister(struct nf_ct_nat_helper *n)
 	spin_lock_bh(&nf_conntrack_expect_lock);
 	list_del_rcu(&n->head);
 	spin_unlock_bh(&nf_conntrack_expect_lock);
+
+	/* Make sure no one is still using the module unless
+	 * its a connection in the hash.
+	 */
+	synchronize_rcu();
+	nf_ct_flush_expect(n->me);
 }
 EXPORT_SYMBOL_GPL(nf_ct_nat_helper_unregister);
 
@@ -444,8 +481,6 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,
 void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 {
 	struct nf_conntrack_tuple_hash *h;
-	struct nf_conntrack_expect *exp;
-	const struct hlist_node *next;
 	const struct hlist_nulls_node *nn;
 	unsigned int last_hsize;
 	spinlock_t *lock;
@@ -457,28 +492,11 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 	nf_ct_helper_count--;
 	mutex_unlock(&nf_ct_helper_mutex);
 
-	/* Make sure every nothing is still using the helper unless its a
-	 * connection in the hash.
+	/* Make sure no one is still using the module unless
+	 * its a connection in the hash.
 	 */
 	synchronize_rcu();
-
-	/* Get rid of expectations */
-	spin_lock_bh(&nf_conntrack_expect_lock);
-	for (i = 0; i < nf_ct_expect_hsize; i++) {
-		hlist_for_each_entry_safe(exp, next,
-					  &nf_ct_expect_hash[i], hnode) {
-			struct nf_conn_help *help = nfct_help(exp->master);
-			if ((rcu_dereference_protected(
-					help->helper,
-					lockdep_is_held(&nf_conntrack_expect_lock)
-					) == me || exp->helper == me) &&
-			    del_timer(&exp->timeout)) {
-				nf_ct_unlink_expect(exp);
-				nf_ct_expect_put(exp);
-			}
-		}
-	}
-	spin_unlock_bh(&nf_conntrack_expect_lock);
+	nf_ct_flush_expect(me->me);
 
 	rtnl_lock();
 	for_each_net(net)
diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c
index 896b154..581ec82 100644
--- a/net/netfilter/nf_conntrack_pptp.c
+++ b/net/netfilter/nf_conntrack_pptp.c
@@ -104,6 +104,7 @@ static void pptp_expectfn(struct nf_conn *ct,
 
 static struct nf_ct_nat_helper pptp_nat = {
 	.name			= "pptp-nat",
+	.me			= THIS_MODULE,
 	.expectfn		= pptp_expectfn,
 };
 
diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c
index 9b540a9..e761e24 100644
--- a/net/netfilter/nf_nat_amanda.c
+++ b/net/netfilter/nf_nat_amanda.c
@@ -26,6 +26,7 @@
 
 static struct nf_ct_nat_helper amanda_nat = {
 	.name = "amanda-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 2897bd4..ae1789b 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -850,6 +850,7 @@ static void __net_exit nf_nat_net_exit(struct net *net)
 
 static struct nf_ct_nat_helper follow_master_nat = {
 	.name		= "nat-follow-master",
+	.me		= THIS_MODULE,
 	.expectfn	= nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_ftp.c b/net/netfilter/nf_nat_ftp.c
index f1f4501..82cc5e2 100644
--- a/net/netfilter/nf_nat_ftp.c
+++ b/net/netfilter/nf_nat_ftp.c
@@ -26,6 +26,7 @@
 
 static struct nf_ct_nat_helper ftp_nat = {
 	.name = "ftp-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_irc.c b/net/netfilter/nf_nat_irc.c
index cf623d3..0ce6746 100644
--- a/net/netfilter/nf_nat_irc.c
+++ b/net/netfilter/nf_nat_irc.c
@@ -28,6 +28,7 @@
 
 static struct nf_ct_nat_helper irc_nat = {
 	.name = "irc-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index cd9333c..d13aae6 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -33,6 +33,7 @@ static void nf_nat_sip_expected(struct nf_conn *ct,
 
 static struct nf_ct_nat_helper sip_nat = {
 	.name		= "sip",
+	.me		= THIS_MODULE,
 	.expectfn	= nf_nat_sip_expected,
 };
 
diff --git a/net/netfilter/nf_nat_tftp.c b/net/netfilter/nf_nat_tftp.c
index d279b41..4e60158 100644
--- a/net/netfilter/nf_nat_tftp.c
+++ b/net/netfilter/nf_nat_tftp.c
@@ -20,6 +20,7 @@
 
 static struct nf_ct_nat_helper tftp_nat = {
 	.name = "tftp-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded
  2017-05-05  0:55 ` [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded gfree.wind
@ 2017-05-05  1:59   ` Liping Zhang
  2017-05-05  2:16     ` Gao Feng
  0 siblings, 1 reply; 6+ messages in thread
From: Liping Zhang @ 2017-05-05  1:59 UTC (permalink / raw)
  To: gfree.wind
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Netfilter Developer Mailing List

Hi Feng,

2017-05-05 8:55 GMT+08:00  <gfree.wind@vip.163.com>:
[...]
> +static void
> +nf_ct_flush_expect(const struct module *me)
> +{
> +       struct nf_conntrack_expect *exp;
> +       const struct hlist_node *next;
> +       u32 i;
> +
> +       if (!me)
> +               return;
> +
> +       /* Get rid of expectations */
> +       spin_lock_bh(&nf_conntrack_expect_lock);
> +       for (i = 0; i < nf_ct_expect_hsize; i++) {
> +               hlist_for_each_entry_safe(exp, next,
> +                                         &nf_ct_expect_hash[i], hnode) {
> +                       struct nf_conn_help *master_help = nfct_help(exp->master);
> +
> +                       if ((master_help->helper && master_help->helper->me == me) ||
> +                           (exp->helper && exp->helper->me == me) ||
> +                           (exp->nat_helper && exp->nat_helper->me == me)) {
> +                               /* This expect belongs to the dying module */
> +                               if (del_timer(&exp->timeout)) {
> +                                       nf_ct_unlink_expect(exp);
> +                                       nf_ct_expect_put(exp);
> +                               }

Is this should be replaced with your helper function nf_ct_remove_expect()?

> +                       }
> +               }
> +       }
> +       spin_unlock_bh(&nf_conntrack_expect_lock);

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re:Re: [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded
  2017-05-05  1:59   ` Liping Zhang
@ 2017-05-05  2:16     ` Gao Feng
  0 siblings, 0 replies; 6+ messages in thread
From: Gao Feng @ 2017-05-05  2:16 UTC (permalink / raw)
  To: Liping Zhang
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Netfilter Developer Mailing List

Hi Liping,

At 2017-05-05 09:59:06, "Liping Zhang" <zlpnobody@gmail.com> wrote:
>Hi Feng,
>
>2017-05-05 8:55 GMT+08:00  <gfree.wind@vip.163.com>:
>[...]
>> +static void
>> +nf_ct_flush_expect(const struct module *me)
>> +{
>> +       struct nf_conntrack_expect *exp;
>> +       const struct hlist_node *next;
>> +       u32 i;
>> +
>> +       if (!me)
>> +               return;
>> +
>> +       /* Get rid of expectations */
>> +       spin_lock_bh(&nf_conntrack_expect_lock);
>> +       for (i = 0; i < nf_ct_expect_hsize; i++) {
>> +               hlist_for_each_entry_safe(exp, next,
>> +                                         &nf_ct_expect_hash[i], hnode) {
>> +                       struct nf_conn_help *master_help = nfct_help(exp->master);
>> +
>> +                       if ((master_help->helper && master_help->helper->me == me) ||
>> +                           (exp->helper && exp->helper->me == me) ||
>> +                           (exp->nat_helper && exp->nat_helper->me == me)) {
>> +                               /* This expect belongs to the dying module */
>> +                               if (del_timer(&exp->timeout)) {
>> +                                       nf_ct_unlink_expect(exp);
>> +                                       nf_ct_expect_put(exp);
>> +                               }
>
>Is this should be replaced with your helper function nf_ct_remove_expect()?

Thanks. Because this helper was applied in nf-next.git before, not nf.git.
So I didn't use the helper function. I just checked some days ago.

I pull the nf.git codes just now, the helper is already synchronized at May 1st.  
But I don't pull the codes recently, so I didn't find it before.

I would like to update it, but need to wait for a while to collect more comments.
Thanks again.

Best Regards
Feng
>
>> +                       }
>> +               }
>> +       }
>> +       spin_unlock_bh(&nf_conntrack_expect_lock);


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module
  2017-05-05  0:55 ` [PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module gfree.wind
@ 2017-05-15 17:20   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2017-05-15 17:20 UTC (permalink / raw)
  To: gfree.wind; +Cc: kadlec, fw, netfilter-devel

On Fri, May 05, 2017 at 08:55:12AM +0800, gfree.wind@vip.163.com wrote:
> diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
> index 346e764..ce2095c 100644
> --- a/net/ipv4/netfilter/nf_nat_h323.c
> +++ b/net/ipv4/netfilter/nf_nat_h323.c
> @@ -21,6 +21,26 @@
>  #include <linux/netfilter/nf_conntrack_h323.h>
>  
>  /****************************************************************************/
> +static void ip_nat_q931_expect(struct nf_conn *new,
> +			       struct nf_conntrack_expect *this);
> +static void ip_nat_callforwarding_expect(struct nf_conn *new,
> +					 struct nf_conntrack_expect *this);
> +
> +static struct nf_ct_nat_helper q931_nat = {
> +	.name		= "Q.931",
> +	.expectfn	= ip_nat_q931_expect,
> +};
> +
> +static struct nf_ct_nat_helper callforwarding_nat = {
> +	.name		= "callforwarding",
> +	.expectfn	= ip_nat_callforwarding_expect,
> +};

Almost there...

> +static struct nf_ct_nat_helper follow_master_nat = {
> +	.name		= "h323-nat-follow-master",

This is a new symbol. There was no "h323-nat-follow-master" before
this patch, and this is exposed to userspace.

This should be "nat-follow-master" instead, so you need a way to
register this for several helpers that support this.

Probably you need a type field, eg.

static struct nf_ct_nat_helper h323_follow_master_nat = {
        .type           = NF_CT_HELPER_H323,
        .name           = "nat-follow-master",
        ...
};

So we can keep using "nat-follow-master" from different helpers, as it
happens currently. This should also allow us to validate if someone
wants to attach a expectfn symbol that is not support by the helper.

We need a way to relate the helper itself with this new
nf_ct_nat_helper structure that doesn't exist in the code yet.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-15 17:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-05  0:55 [PATCH nf v6 1/3] netfilter: helper: Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper gfree.wind
2017-05-05  0:55 ` [PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module gfree.wind
2017-05-15 17:20   ` Pablo Neira Ayuso
2017-05-05  0:55 ` [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded gfree.wind
2017-05-05  1:59   ` Liping Zhang
2017-05-05  2:16     ` 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.