nf_nat git tree

nf_nat git tree

[Patrick McHardy]

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

I've put the nf_nat stuff in a git tree. The NAT resync-patches
and the FTP hookfn fix are folded into the original patch and
I've ported a few more helpers (only PPtP and netbios_ns are
still missing) and fixed some small bugs in the previous patches
I sent. I've also added module aliases for all helpers so users
can switch transparently.

The git tree contains only my changes, so you need to clone
Linus' tree first and then pull my tree into it:

git-clone
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
git-pull http://people.netfilter.org/~kaber/nf-2.6.20-nat.git/

(BTW, 2.6.20 doesn't mean I necessarily want to get it into 2.6.20,
just that its on top of my queued patched for 2.6.20).

Anyone interested in testing should apply the attached workaround
for the nf_conntrack_alter_reply problem on top of the git tree.

Changelog and diffstat below.


[-- Attachment #2: summary --]
[-- Type: text/plain, Size: 4436 bytes --]

 include/linux/netfilter/nf_conntrack_amanda.h            |   10 
 include/linux/netfilter/nf_conntrack_ftp.h               |   20 
 include/linux/netfilter/nf_conntrack_h323.h              |   92 
 include/linux/netfilter/nf_conntrack_helper_h323_asn1.h  |   98 
 include/linux/netfilter/nf_conntrack_helper_h323_types.h |  951 +++++++
 include/linux/netfilter/nf_conntrack_irc.h               |   15 
 include/linux/netfilter/nf_conntrack_sip.h               |   44 
 include/linux/netfilter/nf_conntrack_tftp.h              |   20 
 include/linux/netfilter_ipv4/ip_conntrack_ftp.h          |   40 
 include/net/netfilter/ipv4/nf_conntrack_ipv4.h           |   20 
 include/net/netfilter/nf_conntrack.h                     |   30 
 include/net/netfilter/nf_conntrack_core.h                |    3 
 include/net/netfilter/nf_conntrack_expect.h              |    7 
 include/net/netfilter/nf_conntrack_tuple.h               |   10 
 include/net/netfilter/nf_nat.h                           |   78 
 include/net/netfilter/nf_nat_core.h                      |   26 
 include/net/netfilter/nf_nat_helper.h                    |   33 
 include/net/netfilter/nf_nat_protocol.h                  |   74 
 include/net/netfilter/nf_nat_rule.h                      |   38 
 net/ipv4/netfilter/Kconfig                               |   98 
 net/ipv4/netfilter/Makefile                              |   19 
 net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c       |  874 ------
 net/ipv4/netfilter/ip_conntrack_helper_h323_types.c      | 1926 --------------
 net/ipv4/netfilter/ipt_MASQUERADE.c                      |   29 
 net/ipv4/netfilter/ipt_NETMAP.c                          |    4 
 net/ipv4/netfilter/ipt_REDIRECT.c                        |    6 
 net/ipv4/netfilter/ipt_SAME.c                            |   12 
 net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c           |    7 
 net/ipv4/netfilter/nf_nat_amanda.c                       |   79 
 net/ipv4/netfilter/nf_nat_core.c                         |  647 +++++
 net/ipv4/netfilter/nf_nat_ftp.c                          |  183 +
 net/ipv4/netfilter/nf_nat_h323.c                         |  613 ++++
 net/ipv4/netfilter/nf_nat_helper.c                       |  455 +++
 net/ipv4/netfilter/nf_nat_irc.c                          |  101 
 net/ipv4/netfilter/nf_nat_proto_icmp.c                   |   89 
 net/ipv4/netfilter/nf_nat_proto_tcp.c                    |  150 +
 net/ipv4/netfilter/nf_nat_proto_udp.c                    |  141 +
 net/ipv4/netfilter/nf_nat_proto_unknown.c                |   55 
 net/ipv4/netfilter/nf_nat_rule.c                         |  343 ++
 net/ipv4/netfilter/nf_nat_sip.c                          |  251 +
 net/ipv4/netfilter/nf_nat_standalone.c                   |  422 +++
 net/ipv4/netfilter/nf_nat_tftp.c                         |   52 
 net/netfilter/Kconfig                                    |  122 
 net/netfilter/Makefile                                   |    7 
 net/netfilter/nf_conntrack_amanda.c                      |  232 +
 net/netfilter/nf_conntrack_core.c                        |   20 
 net/netfilter/nf_conntrack_expect.c                      |   45 
 net/netfilter/nf_conntrack_ftp.c                         |   21 
 net/netfilter/nf_conntrack_helper_h323.c                 | 1812 ++++++++++++++
 net/netfilter/nf_conntrack_helper_h323_asn1.c            |  874 ++++++
 net/netfilter/nf_conntrack_helper_h323_types.c           | 1927 +++++++++++++++
 net/netfilter/nf_conntrack_irc.c                         |  277 ++
 net/netfilter/nf_conntrack_netlink.c                     |   48 
 net/netfilter/nf_conntrack_proto_tcp.c                   |    2 
 net/netfilter/nf_conntrack_sip.c                         |  489 +++
 net/netfilter/nf_conntrack_standalone.c                  |    5 
 net/netfilter/nf_conntrack_tftp.c                        |  156 +
 net/netfilter/xt_CONNMARK.c                              |    2 
 58 files changed, 11293 insertions(+), 2911 deletions(-)

Patrick McHardy:
      [NETFILTER]: Add NAT support for nf_conntrack
      [NETFILTER]: nf_conntrack: add helper function for expectation initialization
      [NETFILTER]: nf_conntrack/nf_nat: add SIP helper port
      [NETFILTER]: nf_conntrack/nf_nat: add TFTP helper port
      [NETFILTER]: nf_conntrack/nf_nat: add amanda helper port
      [NETFILTER]: nf_conntrack/nf_nat: add H.323 helper port
      [NETFILTER]: nf_conntrack/nf_nat: add IRC helper port

[-- Attachment #3: x --]
[-- Type: text/plain, Size: 541 bytes --]

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 1f1c257..31a4472 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -855,7 +855,7 @@ void nf_conntrack_alter_reply(struct nf_
 	NF_CT_DUMP_TUPLE(newreply);
 
 	conntrack->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
-	if (!conntrack->master && help->expecting == 0)
+	if (!conntrack->master && 0 && help->expecting == 0)
 		help->helper = __nf_ct_helper_find(newreply);
 	write_unlock_bh(&nf_conntrack_lock);
 }

Re: nf_nat git tree

[Jozsef Kadlecsik]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1381 bytes --]

Hi,

On Mon, 6 Nov 2006, Patrick McHardy wrote:

> Anyone interested in testing should apply the attached workaround
> for the nf_conntrack_alter_reply problem on top of the git tree.

Attached is a patch which tries to solve both the problem of 
nf_conntrack_alter_reply and helpers added to the conntrack entries on the 
fly by expect functions, instead of the workaround.

Some comments:

The flag approach of helpers added dynamically is hairy in the sense 
that when an expect function adds a helper to the entry *and* the flag is 
missing (i.e forgotten about), then it leads a clean, nice crash.

alter_reply is solved by allocating a new conntrack entry and replacing 
the old one with the new. This is expensive, of course: the function 
checks everything in order to avoid it. We can skip allocating completely 
if we keep the helper (whatever it is) regardless of the result of NATing 
the connection, which is implemented by the keep_helper module option of 
nf_conntrack. (It is the cleanest, but not backward compatible: in theory 
the helper may change only when DNAT alters the port.) Untested code, so 
beware!

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

[-- Attachment #2: nat-core.patch --]
[-- Type: TEXT/PLAIN, Size: 12189 bytes --]

diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/include/net/netfilter/nf_conntrack.h linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack.h
--- linux-2.6.20-orig/include/net/netfilter/nf_conntrack.h	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack.h	2006-11-06 14:12:31.000000000 +0100
@@ -137,7 +137,7 @@
 #define master_ct(conntr) (conntr->master)
 
 /* Alter reply tuple (maybe alter helper). */
-extern void
+extern struct nf_conn *
 nf_conntrack_alter_reply(struct nf_conn *conntrack,
 			 const struct nf_conntrack_tuple *newreply);
 
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack_expect.h
--- linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack_expect.h	2006-11-06 14:12:31.000000000 +0100
@@ -48,7 +48,7 @@
 };
 
 #define NF_CT_EXPECT_PERMANENT 0x1
-
+#define NF_CT_EXPECT_HELPER    0x2
 
 struct nf_conntrack_expect *
 __nf_conntrack_expect_find(const struct nf_conntrack_tuple *tuple);
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/ipv4/netfilter/nf_nat_core.c linux-2.6.20-nat-core/net/ipv4/netfilter/nf_nat_core.c
--- linux-2.6.20-orig/net/ipv4/netfilter/nf_nat_core.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/net/ipv4/netfilter/nf_nat_core.c	2006-11-06 14:12:31.000000000 +0100
@@ -311,11 +311,34 @@
 	get_unique_tuple(&new_tuple, &curr_tuple, range, conntrack, maniptype);
 
 	if (!nf_ct_tuple_equal(&new_tuple, &curr_tuple)) {
+		struct nf_conn *ct;
 		struct nf_conntrack_tuple reply;
 
 		/* Alter conntrack table so will recognize replies. */
 		nf_ct_invert_tuplepr(&reply, &new_tuple);
-		nf_conntrack_alter_reply(conntrack, &reply);
+		ct = nf_conntrack_alter_reply(conntrack, &reply);
+		
+		if (ct == NULL)
+			/* Can't do much */
+			return NF_DROP;
+			
+		if (ct != conntrack) {
+			struct nf_conn *tmp;
+			/* New conntrack with helper */
+			if (!have_to_hash) {
+				/* Unknot the old one */
+				write_lock_bh(&nf_nat_lock);
+				list_del(&info->bysource);
+				write_unlock_bh(&nf_nat_lock);
+				have_to_hash = 1;
+			}
+			tmp = conntrack;
+			conntrack = ct;
+			nf_conntrack_free(tmp);
+
+			nat = nfct_nat(conntrack);
+			info = &nat->info;
+		} 
 
 		/* Non-atomic: we own this at the moment. */
 		if (maniptype == IP_NAT_MANIP_SRC)
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c linux-2.6.20-nat-core/net/netfilter/nf_conntrack_core.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/net/netfilter/nf_conntrack_core.c	2006-11-06 14:12:31.000000000 +0100
@@ -37,6 +37,7 @@
 #include <linux/vmalloc.h>
 #include <linux/stddef.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/random.h>
 #include <linux/jhash.h>
 #include <linux/err.h>
@@ -81,6 +82,11 @@
 DEFINE_PER_CPU(struct ip_conntrack_stat, nf_conntrack_stat);
 EXPORT_PER_CPU_SYMBOL(nf_conntrack_stat);
 
+/* This is for NAT. Icky! */
+static int keep_helper = 0;	/* Backward compatibility */
+module_param(keep_helper, bool, 0400);
+MODULE_PARM_DESC(keep_helper, "keep helper found by conntrack, regardless of NAT");
+
 /*
  * This scheme offers various size of "struct nf_conn" dependent on
  * features(helper, nat, ...)
@@ -540,10 +546,36 @@
 	return dropped;
 }
 
+static inline struct nf_conn * __nf_ct_alloc_feature(u_int32_t features)
+{
+	struct nf_conn *conntrack = NULL;
+
+	DEBUGP("__nf_ct_alloc_feature: features=0x%x\n", features);
+
+	read_lock_bh(&nf_ct_cache_lock);
+	if (unlikely(!nf_ct_cache[features].use)) {
+		DEBUGP("__nf_ct_alloc_feature: not supported features = 0x%x\n",
+			features);
+		goto out;
+	}
+
+	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	if (conntrack == NULL) {
+		DEBUGP("__nf_ct_alloc_feature: Can't alloc conntrack from cache\n");
+		goto out;
+	}
+	memset(conntrack, 0, nf_ct_cache[features].size);
+
+   out:
+   	read_unlock_bh(&nf_ct_cache_lock);
+   	return conntrack;
+}
+
 static struct nf_conn *
 __nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
 		     const struct nf_conntrack_tuple *repl,
-		     const struct nf_conntrack_l3proto *l3proto)
+		     const struct nf_conntrack_l3proto *l3proto,
+		     unsigned int flags)
 {
 	struct nf_conn *conntrack = NULL;
 	u_int32_t features = 0;
@@ -577,27 +609,16 @@
 	/* FIXME: protect helper list per RCU */
 	read_lock_bh(&nf_conntrack_lock);
 	helper = __nf_ct_helper_find(repl);
-	if (helper)
+	if (helper || (flags & NF_CT_EXPECT_HELPER))
 		features |= NF_CT_F_HELP;
 	read_unlock_bh(&nf_conntrack_lock);
 
-	DEBUGP("nf_conntrack_alloc: features=0x%x\n", features);
-
-	read_lock_bh(&nf_ct_cache_lock);
-
-	if (unlikely(!nf_ct_cache[features].use)) {
-		DEBUGP("nf_conntrack_alloc: not supported features = 0x%x\n",
-			features);
-		goto out;
-	}
-
-	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	conntrack = __nf_ct_alloc_feature(features);
 	if (conntrack == NULL) {
-		DEBUGP("nf_conntrack_alloc: Can't alloc conntrack from cache\n");
-		goto out;
+		atomic_dec(&nf_conntrack_count);
+		return conntrack;
 	}
 
-	memset(conntrack, 0, nf_ct_cache[features].size);
 	conntrack->features = features;
 	if (helper) {
 		struct nf_conn_help *help = nfct_help(conntrack);
@@ -613,13 +634,8 @@
 	init_timer(&conntrack->timeout);
 	conntrack->timeout.data = (unsigned long)conntrack;
 	conntrack->timeout.function = death_by_timeout;
-	read_unlock_bh(&nf_ct_cache_lock);
 
 	return conntrack;
-out:
-	read_unlock_bh(&nf_ct_cache_lock);
-	atomic_dec(&nf_conntrack_count);
-	return conntrack;
 }
 
 struct nf_conn *nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
@@ -628,7 +644,7 @@
 	struct nf_conntrack_l3proto *l3proto;
 
 	l3proto = __nf_ct_l3proto_find(orig->src.l3num);
-	return __nf_conntrack_alloc(orig, repl, l3proto);
+	return __nf_conntrack_alloc(orig, repl, l3proto, 0);
 }
 
 void nf_conntrack_free(struct nf_conn *conntrack)
@@ -659,7 +675,12 @@
 		return NULL;
 	}
 
-	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto);
+	write_lock_bh(&nf_conntrack_lock);
+	exp = find_expectation(tuple);
+	write_unlock_bh(&nf_conntrack_lock);
+
+	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto,
+					 exp->flags);
 	if (conntrack == NULL || IS_ERR(conntrack)) {
 		DEBUGP("Can't allocate conntrack.\n");
 		return (struct nf_conntrack_tuple_hash *)conntrack;
@@ -672,7 +693,6 @@
 	}
 
 	write_lock_bh(&nf_conntrack_lock);
-	exp = find_expectation(tuple);
 
 	if (exp) {
 		DEBUGP("conntrack: expectation arrives ct=%p exp=%p\n",
@@ -840,24 +860,84 @@
 						     orig->dst.protonum));
 }
 
-/* Alter reply tuple (maybe alter helper).  This is for NAT, and is
-   implicitly racy: see __nf_conntrack_confirm */
-void nf_conntrack_alter_reply(struct nf_conn *conntrack,
-			      const struct nf_conntrack_tuple *newreply)
+/* Allocate a new conntrack entry with helper support based on
+ * the original one and update the unconfirmed list. */
+static inline struct nf_conn* 
+nf_ct_realloc(struct nf_conn *conntrack,
+	      const struct nf_conntrack_tuple *newreply,
+	      struct nf_conntrack_helper *helper)
 {
-	struct nf_conn_help *help = nfct_help(conntrack);
+	struct nf_conn *ct = NULL;
+	struct nf_conn_help *help;
+	const struct nf_conntrack_tuple *orig;
+	struct nf_conntrack_l3proto *l3proto;
+	u_int32_t features = 0;
 
+	orig =  &conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
+	l3proto = __nf_ct_l3proto_find(orig->src.l3num);
+	features = l3proto->get_features(orig) | NF_CT_F_HELP;
+	
+	if (features == conntrack->features)
+		return conntrack;
+		
+	ct = __nf_ct_alloc_feature(features);
+	
+	if (ct == NULL)
+		return NULL;
+		
+	/* Copy over old conntrack */
+	memcpy(ct, conntrack, sizeof(struct nf_conn) 
+			      + sizeof(struct nf_conn_nat)
+			      + __alignof__(struct nf_conn_nat));
+	
+	help = nfct_help(ct);
+	help->helper = helper;
+		
+	/* This is an unconfirmed entry, so not in hashtable yet. */
 	write_lock_bh(&nf_conntrack_lock);
+	list_del(&conntrack->tuplehash[IP_CT_DIR_ORIGINAL].list);
+	list_add(&ct->tuplehash[IP_CT_DIR_ORIGINAL].list, &unconfirmed);
+	write_unlock_bh(&nf_conntrack_lock);
+	
+	return ct;
+}
+
+/* Alter reply tuple (maybe alter helper).  This is for NAT, and is
+   implicitly racy: see __nf_conntrack_confirm */
+struct nf_conn* nf_conntrack_alter_reply(struct nf_conn *conntrack,
+					 const struct nf_conntrack_tuple *newreply)
+{
+	struct nf_conntrack_helper *helper;
+	
 	/* Should be unconfirmed, so not in hash table yet */
 	NF_CT_ASSERT(!nf_ct_is_confirmed(conntrack));
 
 	DEBUGP("Altering reply tuple of %p to ", conntrack);
 	NF_CT_DUMP_TUPLE(newreply);
+	
+	if (keep_helper)
+		goto ALTER_REPLY;
 
+	read_lock_bh(&nf_conntrack_lock);
+	helper = __nf_ct_helper_find(newreply);
+	read_unlock_bh(&nf_conntrack_lock);
+	
+	if (!helper)
+		goto ALTER_REPLY;
+	
+	/* We need to allocate a new conntrack entry with helper support.
+	 * Should not really happen! */
+	conntrack = nf_ct_realloc(conntrack, newreply, helper);	
+	if (!conntrack)
+		return NULL;
+	atomic_inc(&nf_conntrack_count);
+
+  ALTER_REPLY:
+	write_lock_bh(&nf_conntrack_lock);
 	conntrack->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
-	if (!conntrack->master && help->expecting == 0)
-		help->helper = __nf_ct_helper_find(newreply);
 	write_unlock_bh(&nf_conntrack_lock);
+	
+	return conntrack;
 }
 
 /* Refresh conntrack for this many jiffies and do accounting if do_acct is 1 */
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c linux-2.6.20-nat-core/net/netfilter/nf_conntrack_helper_h323.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/net/netfilter/nf_conntrack_helper_h323.c	2006-11-06 14:39:04.000000000 +0100
@@ -693,6 +693,7 @@
 				    port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_h245_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect H.245 ");
@@ -805,6 +806,7 @@
 					      taddr, port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_q931_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect Call Forwarding ");
@@ -1298,6 +1300,7 @@
 				    port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_q931_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1363,6 +1366,7 @@
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_UDP, NULL, &port);
 	exp->expectfn = nf_conntrack_ras_expect;
+        exp->flags = NF_CT_EXPECT_HELPER;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect RAS ");
@@ -1558,7 +1562,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
-	exp->flags = NF_CT_EXPECT_PERMANENT;
+	exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_HELPER;
 	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
@@ -1608,7 +1612,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
-	exp->flags = NF_CT_EXPECT_PERMANENT;
+	exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_HELPER;
 	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {

Re: nf_nat git tree

[Jozsef Kadlecsik]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1684 bytes --]

On Mon, 6 Nov 2006, Jozsef Kadlecsik wrote:

> alter_reply is solved by allocating a new conntrack entry and replacing 
> the old one with the new. This is expensive, of course: the function 
> checks everything in order to avoid it. We can skip allocating completely 
> if we keep the helper (whatever it is) regardless of the result of NATing 
> the connection, which is implemented by the keep_helper module option of 
> nf_conntrack. (It is the cleanest, but not backward compatible: in theory 
> the helper may change only when DNAT alters the port.) Untested code, so 
> beware!

This is crap, of course, I missed a few points: a pointer to skbuff should 
be passed along, just to attach the new conntrack entry to it. And the 
whole reallocating is fragile as the new conntrack pointer should be 
passed upwards too. Sigh.

The big question is, how many guys out there rely on the feature that they 
can DNAT non-standard ports to the standard port and then the standard 
helper is attached to the connection by netfilter?

The attached patch fixes alter_reply by allocating large enough conntrack 
entry in advance (Yasuyuki's suggestion) with the price that all conntrack 
entries are full sized when NAT is enabled (default). I preserved the 
'keep_helper' module parameter from the previous patch so that one can 
disable always allocating max size entries, but then conntrack won't try 
to find a new helper after NAT.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

[-- Attachment #2: nat-core.patch1 --]
[-- Type: TEXT/PLAIN, Size: 8470 bytes --]

diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack_expect.h
--- linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack_expect.h	2006-11-06 14:12:31.000000000 +0100
@@ -48,7 +48,7 @@
 };
 
 #define NF_CT_EXPECT_PERMANENT 0x1
-
+#define NF_CT_EXPECT_HELPER    0x2
 
 struct nf_conntrack_expect *
 __nf_conntrack_expect_find(const struct nf_conntrack_tuple *tuple);
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c linux-2.6.20-nat-core/net/netfilter/nf_conntrack_core.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/net/netfilter/nf_conntrack_core.c	2006-11-07 10:46:08.000000000 +0100
@@ -81,6 +81,11 @@
 DEFINE_PER_CPU(struct ip_conntrack_stat, nf_conntrack_stat);
 EXPORT_PER_CPU_SYMBOL(nf_conntrack_stat);
 
+/* This is for NAT. Icky! */
+static int keep_helper = 0;	/* Backward compatibility */
+module_param(keep_helper, bool, 0400);
+MODULE_PARM_DESC(keep_helper, "keep helper found by conntrack, regardless of NAT");
+
 /*
  * This scheme offers various size of "struct nf_conn" dependent on
  * features(helper, nat, ...)
@@ -540,14 +545,40 @@
 	return dropped;
 }
 
+static inline struct nf_conn * __nf_ct_alloc_feature(u_int32_t features)
+{
+	struct nf_conn *conntrack = NULL;
+
+	DEBUGP("__nf_ct_alloc_feature: features=0x%x\n", features);
+
+	read_lock_bh(&nf_ct_cache_lock);
+	if (unlikely(!nf_ct_cache[features].use)) {
+		DEBUGP("__nf_ct_alloc_feature: not supported features = 0x%x\n",
+			features);
+		goto out;
+	}
+
+	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	if (conntrack == NULL) {
+		DEBUGP("__nf_ct_alloc_feature: Can't alloc conntrack from cache\n");
+		goto out;
+	}
+	memset(conntrack, 0, nf_ct_cache[features].size);
+
+   out:
+   	read_unlock_bh(&nf_ct_cache_lock);
+   	return conntrack;
+}
+
 static struct nf_conn *
 __nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
 		     const struct nf_conntrack_tuple *repl,
-		     const struct nf_conntrack_l3proto *l3proto)
+		     const struct nf_conntrack_l3proto *l3proto,
+		     unsigned int flags)
 {
 	struct nf_conn *conntrack = NULL;
 	u_int32_t features = 0;
-	struct nf_conntrack_helper *helper;
+	struct nf_conntrack_helper *helper = NULL;
 
 	if (unlikely(!nf_conntrack_hash_rnd_initted)) {
 		get_random_bytes(&nf_conntrack_hash_rnd, 4);
@@ -574,30 +605,28 @@
 	/*  find features needed by this conntrack. */
 	features = l3proto->get_features(orig);
 
-	/* FIXME: protect helper list per RCU */
-	read_lock_bh(&nf_conntrack_lock);
-	helper = __nf_ct_helper_find(repl);
-	if (helper)
+	/* Expectfn wants to set the helper */
+	if (flags & NF_CT_EXPECT_HELPER)
 		features |= NF_CT_F_HELP;
-	read_unlock_bh(&nf_conntrack_lock);
-
-	DEBUGP("nf_conntrack_alloc: features=0x%x\n", features);
-
-	read_lock_bh(&nf_ct_cache_lock);
-
-	if (unlikely(!nf_ct_cache[features].use)) {
-		DEBUGP("nf_conntrack_alloc: not supported features = 0x%x\n",
-			features);
-		goto out;
+	else {
+		/* FIXME: protect helper list per RCU */
+		read_lock_bh(&nf_conntrack_lock);
+		helper = __nf_ct_helper_find(repl);
+		if (helper)
+			features |= NF_CT_F_HELP;
+		read_unlock_bh(&nf_conntrack_lock);
+		
+		/* NAT may change (i.e. add) helper: 
+		 * enable helper support in general */
+		if ((features & NF_CT_F_NAT) && !keep_helper)
+			features |= NF_CT_F_HELP;
 	}
-
-	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	conntrack = __nf_ct_alloc_feature(features);
 	if (conntrack == NULL) {
-		DEBUGP("nf_conntrack_alloc: Can't alloc conntrack from cache\n");
-		goto out;
+		atomic_dec(&nf_conntrack_count);
+		return conntrack;
 	}
 
-	memset(conntrack, 0, nf_ct_cache[features].size);
 	conntrack->features = features;
 	if (helper) {
 		struct nf_conn_help *help = nfct_help(conntrack);
@@ -613,13 +642,8 @@
 	init_timer(&conntrack->timeout);
 	conntrack->timeout.data = (unsigned long)conntrack;
 	conntrack->timeout.function = death_by_timeout;
-	read_unlock_bh(&nf_ct_cache_lock);
 
 	return conntrack;
-out:
-	read_unlock_bh(&nf_ct_cache_lock);
-	atomic_dec(&nf_conntrack_count);
-	return conntrack;
 }
 
 struct nf_conn *nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
@@ -628,7 +652,7 @@
 	struct nf_conntrack_l3proto *l3proto;
 
 	l3proto = __nf_ct_l3proto_find(orig->src.l3num);
-	return __nf_conntrack_alloc(orig, repl, l3proto);
+	return __nf_conntrack_alloc(orig, repl, l3proto, 0);
 }
 
 void nf_conntrack_free(struct nf_conn *conntrack)
@@ -659,7 +683,12 @@
 		return NULL;
 	}
 
-	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto);
+	read_lock_bh(&nf_conntrack_lock);
+	exp = find_expectation(tuple);
+	read_unlock_bh(&nf_conntrack_lock);
+
+	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto,
+					 exp->flags);
 	if (conntrack == NULL || IS_ERR(conntrack)) {
 		DEBUGP("Can't allocate conntrack.\n");
 		return (struct nf_conntrack_tuple_hash *)conntrack;
@@ -672,7 +701,6 @@
 	}
 
 	write_lock_bh(&nf_conntrack_lock);
-	exp = find_expectation(tuple);
 
 	if (exp) {
 		DEBUGP("conntrack: expectation arrives ct=%p exp=%p\n",
@@ -847,16 +875,19 @@
 {
 	struct nf_conn_help *help = nfct_help(conntrack);
 
-	write_lock_bh(&nf_conntrack_lock);
 	/* Should be unconfirmed, so not in hash table yet */
 	NF_CT_ASSERT(!nf_ct_is_confirmed(conntrack));
 
 	DEBUGP("Altering reply tuple of %p to ", conntrack);
 	NF_CT_DUMP_TUPLE(newreply);
-
+	
+	write_lock_bh(&nf_conntrack_lock);
+	if (!keep_helper) {
+		NF_CT_ASSERT(conntrack->features & NF_CT_F_HELP);
+		if (!conntrack->master && help->expecting == 0)
+			help->helper = __nf_ct_helper_find(newreply);
+	}
 	conntrack->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
-	if (!conntrack->master && help->expecting == 0)
-		help->helper = __nf_ct_helper_find(newreply);
 	write_unlock_bh(&nf_conntrack_lock);
 }
 
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c linux-2.6.20-nat-core/net/netfilter/nf_conntrack_helper_h323.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/net/netfilter/nf_conntrack_helper_h323.c	2006-11-06 14:39:04.000000000 +0100
@@ -693,6 +693,7 @@
 				    port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_h245_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect H.245 ");
@@ -805,6 +806,7 @@
 					      taddr, port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_q931_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect Call Forwarding ");
@@ -1298,6 +1300,7 @@
 				    port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_q931_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1363,6 +1366,7 @@
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_UDP, NULL, &port);
 	exp->expectfn = nf_conntrack_ras_expect;
+        exp->flags = NF_CT_EXPECT_HELPER;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect RAS ");
@@ -1558,7 +1562,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
-	exp->flags = NF_CT_EXPECT_PERMANENT;
+	exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_HELPER;
 	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
@@ -1608,7 +1612,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
-	exp->flags = NF_CT_EXPECT_PERMANENT;
+	exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_HELPER;
 	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {

Re: nf_nat git tree

[Patrick McHardy]

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

Jozsef Kadlecsik wrote:
> On Mon, 6 Nov 2006, Jozsef Kadlecsik wrote:
> 
> 
>>alter_reply is solved by allocating a new conntrack entry and replacing 
>>the old one with the new. This is expensive, of course: the function 
>>checks everything in order to avoid it. We can skip allocating completely 
>>if we keep the helper (whatever it is) regardless of the result of NATing 
>>the connection, which is implemented by the keep_helper module option of 
>>nf_conntrack. (It is the cleanest, but not backward compatible: in theory 
>>the helper may change only when DNAT alters the port.) Untested code, so 
>>beware!
> 
> 
> This is crap, of course, I missed a few points: a pointer to skbuff should 
> be passed along, just to attach the new conntrack entry to it. And the 
> whole reallocating is fragile as the new conntrack pointer should be 
> passed upwards too. Sigh.
> 
> The big question is, how many guys out there rely on the feature that they 
> can DNAT non-standard ports to the standard port and then the standard 
> helper is attached to the connection by netfilter?


Probably not many, but I'm pretty certain at least some people really
use this feature.

> The attached patch fixes alter_reply by allocating large enough conntrack 
> entry in advance (Yasuyuki's suggestion) with the price that all conntrack 
> entries are full sized when NAT is enabled (default). I preserved the 
> 'keep_helper' module parameter from the previous patch so that one can 
> disable always allocating max size entries, but then conntrack won't try 
> to find a new helper after NAT.


I think it makes sense to let helpers like H.323 specify that they want
to assign a helper to an expected connection, although I would prefer
a struct nf_conntrack_helper pointer in struct nf_conntrack_expect
to get rid of using nf_conntrack_lock in helpers and the manual override
functions (something like the attached patch, although it doesn't
actually assign the helper yet because of the problems mentioned below).

>  static struct nf_conn *
>  __nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
>  		     const struct nf_conntrack_tuple *repl,
> -		     const struct nf_conntrack_l3proto *l3proto)
> +		     const struct nf_conntrack_l3proto *l3proto,
> +		     unsigned int flags)
>  {
>  	struct nf_conn *conntrack = NULL;
>  	u_int32_t features = 0;
> -	struct nf_conntrack_helper *helper;
> +	struct nf_conntrack_helper *helper = NULL;
>  
>  	if (unlikely(!nf_conntrack_hash_rnd_initted)) {
>  		get_random_bytes(&nf_conntrack_hash_rnd, 4);
> @@ -574,30 +605,28 @@
>  	/*  find features needed by this conntrack. */
>  	features = l3proto->get_features(orig);
>  
> -	/* FIXME: protect helper list per RCU */
> -	read_lock_bh(&nf_conntrack_lock);
> -	helper = __nf_ct_helper_find(repl);
> -	if (helper)
> +	/* Expectfn wants to set the helper */
> +	if (flags & NF_CT_EXPECT_HELPER)
>  		features |= NF_CT_F_HELP;
> -	read_unlock_bh(&nf_conntrack_lock);
> -
> -	DEBUGP("nf_conntrack_alloc: features=0x%x\n", features);
> -
> -	read_lock_bh(&nf_ct_cache_lock);
> -
> -	if (unlikely(!nf_ct_cache[features].use)) {
> -		DEBUGP("nf_conntrack_alloc: not supported features = 0x%x\n",
> -			features);
> -		goto out;
> +	else {
> +		/* FIXME: protect helper list per RCU */
> +		read_lock_bh(&nf_conntrack_lock);
> +		helper = __nf_ct_helper_find(repl);
> +		if (helper)
> +			features |= NF_CT_F_HELP;
> +		read_unlock_bh(&nf_conntrack_lock);
> +		
> +		/* NAT may change (i.e. add) helper: 
> +		 * enable helper support in general */
> +		if ((features & NF_CT_F_NAT) && !keep_helper)
> +			features |= NF_CT_F_HELP;
>  	}
> -
> -	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
> +	conntrack = __nf_ct_alloc_feature(features);
>  	if (conntrack == NULL) {
> -		DEBUGP("nf_conntrack_alloc: Can't alloc conntrack from cache\n");
> -		goto out;
> +		atomic_dec(&nf_conntrack_count);
> +		return conntrack;
>  	}
>  
> -	memset(conntrack, 0, nf_ct_cache[features].size);
>  	conntrack->features = features;
>  	if (helper) {
>  		struct nf_conn_help *help = nfct_help(conntrack);
> @@ -613,13 +642,8 @@
>  	init_timer(&conntrack->timeout);
>  	conntrack->timeout.data = (unsigned long)conntrack;
>  	conntrack->timeout.function = death_by_timeout;
> -	read_unlock_bh(&nf_ct_cache_lock);
>  
>  	return conntrack;
> -out:
> -	read_unlock_bh(&nf_ct_cache_lock);
> -	atomic_dec(&nf_conntrack_count);
> -	return conntrack;
>  }
>  
>  struct nf_conn *nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
> @@ -628,7 +652,7 @@
>  	struct nf_conntrack_l3proto *l3proto;
>  
>  	l3proto = __nf_ct_l3proto_find(orig->src.l3num);
> -	return __nf_conntrack_alloc(orig, repl, l3proto);
> +	return __nf_conntrack_alloc(orig, repl, l3proto, 0);
>  }
>  
>  void nf_conntrack_free(struct nf_conn *conntrack)
> @@ -659,7 +683,12 @@
>  		return NULL;
>  	}
>  
> -	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto);
> +	read_lock_bh(&nf_conntrack_lock);
> +	exp = find_expectation(tuple);
> +	read_unlock_bh(&nf_conntrack_lock);


This leaves a race between looking up the expectation and assigning it
to the conntrack and putting it on the unconfirmed list. If the helper
is unloaded during this race it won't be able to clean up the reference
and we'll get a use-after-free. The same problem exists in the helper
lookup in __nf_conntrack_alloc(), we need to hold the lock until the
conntrack is on the unconfirmed list. A second problem with the helper
lookup on allocation is that nf_conntrack_netlink will (unlike
ip_conntrack_netlink) automatically assign a helper to new conntracks.

IMO this just shows the real problem with the features allocation
scheme, we don't know everything in advance (especially considering
nf_conntrack_netlink), and trying to work around it results in these
problems. I really can't see a clean solution while keeping the
features allocation. The two obvious other solutions are to either
always allocate for everything as IPv4 connection tracking does, or
use pointers and extra allocations (or ct_extend, but I can't really
remember too much about it).


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 10997 bytes --]

diff --git a/include/linux/netfilter/nf_conntrack_h323.h b/include/linux/netfilter/nf_conntrack_h323.h
index 3faeb58..fbf9a94 100644
--- a/include/linux/netfilter/nf_conntrack_h323.h
+++ b/include/linux/netfilter/nf_conntrack_h323.h
@@ -32,10 +32,6 @@ struct nf_conn;
 extern int get_h225_addr(struct nf_conn *ct, unsigned char *data,
 			 TransportAddress *taddr,
 			 union nf_conntrack_address *addr, __be16 *port);
-extern void nf_conntrack_h245_expect(struct nf_conn *new,
-				     struct nf_conntrack_expect *this);
-extern void nf_conntrack_q931_expect(struct nf_conn *new,
-				     struct nf_conntrack_expect *this);
 extern int (*set_h245_addr_hook) (struct sk_buff **pskb,
 				  unsigned char **data, int dataoff,
 				  H245_TransportAddress *taddr,
diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
index 2923bec..1c23ec2 100644
--- a/include/net/netfilter/nf_conntrack_expect.h
+++ b/include/net/netfilter/nf_conntrack_expect.h
@@ -22,6 +22,9 @@ struct nf_conntrack_expect
 	void (*expectfn)(struct nf_conn *new,
 			 struct nf_conntrack_expect *this);
 
+	/* Helper to assign to new connection */
+	struct nf_conntrack_helper *helper;
+
 	/* The conntrack of the master connection */
 	struct nf_conn *master;
 
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index 81eb00f..a51fd46 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -324,18 +324,6 @@ static int nat_t120(struct sk_buff **psk
 	return 0;
 }
 
-/****************************************************************************
- * This conntrack expect function replaces nf_conntrack_h245_expect()
- * which was set by nf_conntrack_helper_h323.c. It calls both
- * nf_nat_follow_master() and nf_conntrack_h245_expect()
- ****************************************************************************/
-static void ip_nat_h245_expect(struct nf_conn *new,
-			       struct nf_conntrack_expect *this)
-{
-	nf_nat_follow_master(new, this);
-	nf_conntrack_h245_expect(new, this);
-}
-
 /****************************************************************************/
 static int nat_h245(struct sk_buff **pskb, struct nf_conn *ct,
 		    enum ip_conntrack_info ctinfo,
@@ -349,7 +337,7 @@ static int nat_h245(struct sk_buff **psk
 
 	/* Set expectations for NAT */
 	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
-	exp->expectfn = ip_nat_h245_expect;
+	exp->expectfn = nf_nat_follow_master;
 	exp->dir = !dir;
 
 	/* Check existing expects */
@@ -399,7 +387,7 @@ static void ip_nat_q931_expect(struct nf
 
 	if (this->tuple.src.u3.ip != 0) {	/* Only accept calls from GK */
 		nf_nat_follow_master(new, this);
-		goto out;
+		return;
 	}
 
 	/* This must be a fresh one. */
@@ -420,9 +408,6 @@ static void ip_nat_q931_expect(struct nf
 
 	/* hook doesn't matter, but it has to do destination manip */
 	nf_nat_setup_info(new, &range, NF_IP_PRE_ROUTING);
-
-      out:
-	nf_conntrack_q931_expect(new, this);
 }
 
 /****************************************************************************/
@@ -510,8 +495,6 @@ static void ip_nat_callforwarding_expect
 
 	/* hook doesn't matter, but it has to do destination manip */
 	nf_nat_setup_info(new, &range, NF_IP_PRE_ROUTING);
-
-	nf_conntrack_q931_expect(new, this);
 }
 
 /****************************************************************************/
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 59de125..7041d25 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -211,6 +211,7 @@ void nf_conntrack_expect_init(struct nf_
 
 	exp->flags = 0;
 	exp->expectfn = NULL;
+	exp->helper = NULL;
 	exp->tuple.src.l3num = family;
 	exp->tuple.dst.protonum = proto;
 	exp->mask.src.l3num = 0xFFFF;
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index bd3bb15..ee77ed3 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -128,7 +128,8 @@ void nf_conntrack_helper_unregister(stru
 	/* Get rid of expectations */
 	list_for_each_entry_safe(exp, tmp, &nf_conntrack_expect_list, list) {
 		struct nf_conn_help *help = nfct_help(exp->master);
-		if (help->helper == me && del_timer(&exp->timeout)) {
+		if ((exp->helper == me || help->helper == me) &&
+		    del_timer(&exp->timeout)) {
 			nf_ct_unlink_expect(exp);
 			nf_conntrack_expect_put(exp);
 		}
diff --git a/net/netfilter/nf_conntrack_helper_h323.c b/net/netfilter/nf_conntrack_helper_h323.c
index ed45ff2..1295a93 100644
--- a/net/netfilter/nf_conntrack_helper_h323.c
+++ b/net/netfilter/nf_conntrack_helper_h323.c
@@ -619,15 +619,6 @@ static struct nf_conntrack_helper nf_con
 };
 
 /****************************************************************************/
-void nf_conntrack_h245_expect(struct nf_conn *new,
-			      struct nf_conntrack_expect *this)
-{
-	write_lock_bh(&nf_conntrack_lock);
-	nfct_help(new)->helper = &nf_conntrack_helper_h245;
-	write_unlock_bh(&nf_conntrack_lock);
-}
-
-/****************************************************************************/
 int get_h225_addr(struct nf_conn *ct, unsigned char *data,
 		  TransportAddress *taddr,
 		  union nf_conntrack_address *addr, __be16 *port)
@@ -685,6 +676,7 @@ static int expect_h245(struct sk_buff **
 				 &ct->tuplehash[!dir].tuple.src.u3,
 				 &ct->tuplehash[!dir].tuple.dst.u3,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_h245;
 
 	if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
 		   &ct->tuplehash[!dir].tuple.dst.u3,
@@ -694,8 +686,6 @@ static int expect_h245(struct sk_buff **
 		ret = nat_h245_hook(pskb, ct, ctinfo, data, dataoff, taddr,
 				    port, exp);
 	} else {		/* Conntrack only */
-		exp->expectfn = nf_conntrack_h245_expect;
-
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect H.245 ");
 			NF_CT_DUMP_TUPLE(&exp->tuple);
@@ -708,10 +698,6 @@ static int expect_h245(struct sk_buff **
 	return ret;
 }
 
-/* Forwarding declaration */
-void nf_conntrack_q931_expect(struct nf_conn *new,
-			      struct nf_conntrack_expect *this);
-
 /* If the calling party is on the same side of the forward-to party,
  * we don't need to track the second call */
 static int callforward_do_filter(union nf_conntrack_address *src,
@@ -767,6 +753,8 @@ #endif
 
 }
 
+static struct nf_conntrack_helper nf_conntrack_helper_q931;
+
 /****************************************************************************/
 static int expect_callforwarding(struct sk_buff **pskb,
 				 struct nf_conn *ct,
@@ -799,6 +787,7 @@ static int expect_callforwarding(struct 
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 
 	if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
 		   &ct->tuplehash[!dir].tuple.dst.u3,
@@ -808,8 +797,6 @@ static int expect_callforwarding(struct 
 		ret = nat_callforwarding_hook(pskb, ct, ctinfo, data, dataoff,
 					      taddr, port, exp);
 	} else {		/* Conntrack only */
-		exp->expectfn = nf_conntrack_q931_expect;
-
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect Call Forwarding ");
 			NF_CT_DUMP_TUPLE(&exp->tuple);
@@ -1203,15 +1190,6 @@ static struct nf_conntrack_helper nf_con
 };
 
 /****************************************************************************/
-void nf_conntrack_q931_expect(struct nf_conn *new,
-			      struct nf_conntrack_expect *this)
-{
-	write_lock_bh(&nf_conntrack_lock);
-	nfct_help(new)->helper = &nf_conntrack_helper_q931;
-	write_unlock_bh(&nf_conntrack_lock);
-}
-
-/****************************************************************************/
 static unsigned char *get_udp_data(struct sk_buff **pskb, unsigned int protoff,
 				   int *datalen)
 {
@@ -1295,14 +1273,13 @@ static int expect_q931(struct sk_buff **
 					NULL,
 				 &ct->tuplehash[!dir].tuple.dst.u3,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 	exp->flags = NF_CT_EXPECT_PERMANENT;	/* Accept multiple calls */
 
 	if (nat_q931_hook && ct->status & IPS_NAT_MASK) {	/* Need NAT */
 		ret = nat_q931_hook(pskb, ct, ctinfo, data, taddr, i,
 				    port, exp);
 	} else {		/* Conntrack only */
-		exp->expectfn = nf_conntrack_q931_expect;
-
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_ras: expect Q.931 ");
 			NF_CT_DUMP_TUPLE(&exp->tuple);
@@ -1331,9 +1308,7 @@ static int process_grq(struct sk_buff **
 	return 0;
 }
 
-/* Declare before using */
-static void nf_conntrack_ras_expect(struct nf_conn *new,
-				    struct nf_conntrack_expect *this);
+static struct nf_conntrack_helper nf_conntrack_helper_ras;
 
 /****************************************************************************/
 static int process_gcf(struct sk_buff **pskb, struct nf_conn *ct,
@@ -1366,7 +1341,7 @@ static int process_gcf(struct sk_buff **
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_UDP, NULL, &port);
-	exp->expectfn = nf_conntrack_ras_expect;
+	exp->helper = &nf_conntrack_helper_ras;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect RAS ");
@@ -1562,8 +1537,8 @@ static int process_acf(struct sk_buff **
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 	exp->flags = NF_CT_EXPECT_PERMANENT;
-	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1612,8 +1587,8 @@ static int process_lcf(struct sk_buff **
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 	exp->flags = NF_CT_EXPECT_PERMANENT;
-	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1758,15 +1733,6 @@ static struct nf_conntrack_helper nf_con
 };
 
 /****************************************************************************/
-static void nf_conntrack_ras_expect(struct nf_conn *new,
-				    struct nf_conntrack_expect *this)
-{
-	write_lock_bh(&nf_conntrack_lock);
-	nfct_help(new)->helper = &nf_conntrack_helper_ras;
-	write_unlock_bh(&nf_conntrack_lock);
-}
-
-/****************************************************************************/
 /* Not __exit - called from init() */
 static void fini(void)
 {
@@ -1798,8 +1764,6 @@ module_init(init);
 module_exit(fini);
 
 EXPORT_SYMBOL_GPL(get_h225_addr);
-EXPORT_SYMBOL_GPL(nf_conntrack_h245_expect);
-EXPORT_SYMBOL_GPL(nf_conntrack_q931_expect);
 EXPORT_SYMBOL_GPL(set_h245_addr_hook);
 EXPORT_SYMBOL_GPL(set_h225_addr_hook);
 EXPORT_SYMBOL_GPL(set_sig_addr_hook);

Re: nf_nat git tree

[Jozsef Kadlecsik]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3458 bytes --]

On Wed, 15 Nov 2006, Patrick McHardy wrote:

> > The big question is, how many guys out there rely on the feature that they 
> > can DNAT non-standard ports to the standard port and then the standard 
> > helper is attached to the connection by netfilter?
> 
> Probably not many, but I'm pretty certain at least some people really
> use this feature.

Yes, it's a nice feature and the only "workaround" would be to register 
the helper on all required non-standard ports.
 
> > The attached patch fixes alter_reply by allocating large enough conntrack 
> > entry in advance (Yasuyuki's suggestion) with the price that all conntrack 
> > entries are full sized when NAT is enabled (default). I preserved the 
> > 'keep_helper' module parameter from the previous patch so that one can 
> > disable always allocating max size entries, but then conntrack won't try 
> > to find a new helper after NAT.
> 
> I think it makes sense to let helpers like H.323 specify that they want
> to assign a helper to an expected connection, although I would prefer
> a struct nf_conntrack_helper pointer in struct nf_conntrack_expect
> to get rid of using nf_conntrack_lock in helpers and the manual override
> functions (something like the attached patch, although it doesn't
> actually assign the helper yet because of the problems mentioned below).

In that case there is no need for the NF_CT_EXPECT_HELPER flag because 
the non NULL helper pointer in struct nf_conntrack_expect can be used to 
figure out the required conntrack features.
 
> This leaves a race between looking up the expectation and assigning it 
> to the conntrack and putting it on the unconfirmed list. If the helper 
> is unloaded during this race it won't be able to clean up the reference 
> and we'll get a use-after-free. The same problem exists in the helper 
> lookup in __nf_conntrack_alloc(), we need to hold the lock until the 
> conntrack is on the unconfirmed list. A second problem with the helper 
> lookup on allocation is that nf_conntrack_netlink will (unlike 
> ip_conntrack_netlink) automatically assign a helper to new conntracks.

Yes, and the present code in 2.6 suffers the same race condition in 
__nf_conntrack_alloc().
 
> IMO this just shows the real problem with the features allocation
> scheme, we don't know everything in advance (especially considering
> nf_conntrack_netlink), and trying to work around it results in these
> problems. I really can't see a clean solution while keeping the
> features allocation. The two obvious other solutions are to either
> always allocate for everything as IPv4 connection tracking does, or
> use pointers and extra allocations (or ct_extend, but I can't really
> remember too much about it).

We can move the actual setting of the helper from __nf_conntrack_alloc() 
to the write-locked region in init_conntrack() with the price of the 
duplicated search on the helper list.

As of ctnetlink_new_conntrack(), we can also assign the helper when adding 
the entry to the hash table.

Attached is two patches which implements the above, over your git tree.
The first one with the NF_CT_EXPECT_HELPER flag and the second one with 
the helper pointer in struct nf_conntrack_expect.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

[-- Attachment #2: nat-core2.patch --]
[-- Type: TEXT/PLAIN, Size: 9886 bytes --]

diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack_expect.h
--- linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/include/net/netfilter/nf_conntrack_expect.h	2006-11-06 14:12:31.000000000 +0100
@@ -48,7 +48,7 @@
 };
 
 #define NF_CT_EXPECT_PERMANENT 0x1
-
+#define NF_CT_EXPECT_HELPER    0x2
 
 struct nf_conntrack_expect *
 __nf_conntrack_expect_find(const struct nf_conntrack_tuple *tuple);
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c linux-2.6.20-nat-core/net/netfilter/nf_conntrack_core.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/net/netfilter/nf_conntrack_core.c	2006-11-15 14:05:11.000000000 +0100
@@ -81,6 +81,11 @@
 DEFINE_PER_CPU(struct ip_conntrack_stat, nf_conntrack_stat);
 EXPORT_PER_CPU_SYMBOL(nf_conntrack_stat);
 
+/* This is for NAT. Icky! */
+static int keep_helper = 0;	/* Backward compatibility */
+module_param(keep_helper, bool, 0400);
+MODULE_PARM_DESC(keep_helper, "keep helper found by conntrack, regardless of NAT");
+
 /*
  * This scheme offers various size of "struct nf_conn" dependent on
  * features(helper, nat, ...)
@@ -404,6 +409,18 @@
 		 &nf_conntrack_hash[repl_hash]);
 }
 
+static inline void 
+__nf_conntrack_set_helper(struct nf_conn *ct,
+			  const struct nf_conntrack_tuple *repl)
+{
+	/* Set helper, if required */
+	if (ct->features & NF_CT_F_HELP) {
+		struct nf_conn_help *help = nfct_help(ct);
+		NF_CT_ASSERT(help);
+		help->helper = __nf_ct_helper_find(repl);
+	}
+}
+
 void nf_conntrack_hash_insert(struct nf_conn *ct)
 {
 	unsigned int hash, repl_hash;
@@ -412,6 +429,7 @@
 	repl_hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 
 	write_lock_bh(&nf_conntrack_lock);
+	__nf_conntrack_set_helper(ct, &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 	__nf_conntrack_hash_insert(ct, hash, repl_hash);
 	write_unlock_bh(&nf_conntrack_lock);
 }
@@ -540,14 +558,52 @@
 	return dropped;
 }
 
+static inline struct nf_conn * __nf_ct_alloc_feature(u_int32_t features)
+{
+	struct nf_conn *conntrack = NULL;
+
+	DEBUGP("__nf_ct_alloc_feature: features=0x%x\n", features);
+
+	read_lock_bh(&nf_ct_cache_lock);
+	if (unlikely(!nf_ct_cache[features].use)) {
+		DEBUGP("__nf_ct_alloc_feature: not supported features = 0x%x\n",
+			features);
+		goto out;
+	}
+
+	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	if (conntrack == NULL) {
+		DEBUGP("__nf_ct_alloc_feature: Can't alloc conntrack from cache\n");
+		goto out;
+	}
+	memset(conntrack, 0, nf_ct_cache[features].size);
+
+   out:
+   	read_unlock_bh(&nf_ct_cache_lock);
+   	return conntrack;
+}
+
+static struct nf_conntrack_helper *
+__nf_conntrack_helper_required(const struct nf_conntrack_tuple *repl)
+{
+	struct nf_conntrack_helper *helper;
+
+	/* FIXME: protect helper list per RCU */
+	read_lock_bh(&nf_conntrack_lock);
+	helper = __nf_ct_helper_find(repl);
+	read_unlock_bh(&nf_conntrack_lock);
+
+	return helper;
+}	
+
 static struct nf_conn *
 __nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
 		     const struct nf_conntrack_tuple *repl,
-		     const struct nf_conntrack_l3proto *l3proto)
+		     const struct nf_conntrack_l3proto *l3proto,
+		     unsigned int flags)
 {
 	struct nf_conn *conntrack = NULL;
 	u_int32_t features = 0;
-	struct nf_conntrack_helper *helper;
 
 	if (unlikely(!nf_conntrack_hash_rnd_initted)) {
 		get_random_bytes(&nf_conntrack_hash_rnd, 4);
@@ -574,37 +630,22 @@
 	/*  find features needed by this conntrack. */
 	features = l3proto->get_features(orig);
 
-	/* FIXME: protect helper list per RCU */
-	read_lock_bh(&nf_conntrack_lock);
-	helper = __nf_ct_helper_find(repl);
-	if (helper)
+	/* Helper oraculum:
+	 * - expectfn wants to set the helper
+	 * - NAT may change (i.e. add) helper
+	 * - standard helper found
+	 */
+	if ((flags & NF_CT_EXPECT_HELPER)
+	    || ((features & NF_CT_F_NAT) && !keep_helper)
+	    || (__nf_conntrack_helper_required(repl) != NULL))
 		features |= NF_CT_F_HELP;
-	read_unlock_bh(&nf_conntrack_lock);
-
-	DEBUGP("nf_conntrack_alloc: features=0x%x\n", features);
-
-	read_lock_bh(&nf_ct_cache_lock);
-
-	if (unlikely(!nf_ct_cache[features].use)) {
-		DEBUGP("nf_conntrack_alloc: not supported features = 0x%x\n",
-			features);
-		goto out;
-	}
-
-	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	conntrack = __nf_ct_alloc_feature(features);
 	if (conntrack == NULL) {
-		DEBUGP("nf_conntrack_alloc: Can't alloc conntrack from cache\n");
-		goto out;
+		atomic_dec(&nf_conntrack_count);
+		return conntrack;
 	}
 
-	memset(conntrack, 0, nf_ct_cache[features].size);
 	conntrack->features = features;
-	if (helper) {
-		struct nf_conn_help *help = nfct_help(conntrack);
-		NF_CT_ASSERT(help);
-		help->helper = helper;
-	}
-
 	atomic_set(&conntrack->ct_general.use, 1);
 	conntrack->ct_general.destroy = destroy_conntrack;
 	conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
@@ -613,13 +654,8 @@
 	init_timer(&conntrack->timeout);
 	conntrack->timeout.data = (unsigned long)conntrack;
 	conntrack->timeout.function = death_by_timeout;
-	read_unlock_bh(&nf_ct_cache_lock);
 
 	return conntrack;
-out:
-	read_unlock_bh(&nf_ct_cache_lock);
-	atomic_dec(&nf_conntrack_count);
-	return conntrack;
 }
 
 struct nf_conn *nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
@@ -628,7 +664,7 @@
 	struct nf_conntrack_l3proto *l3proto;
 
 	l3proto = __nf_ct_l3proto_find(orig->src.l3num);
-	return __nf_conntrack_alloc(orig, repl, l3proto);
+	return __nf_conntrack_alloc(orig, repl, l3proto, 0);
 }
 
 void nf_conntrack_free(struct nf_conn *conntrack)
@@ -659,7 +695,12 @@
 		return NULL;
 	}
 
-	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto);
+	read_lock_bh(&nf_conntrack_lock);
+	exp = find_expectation(tuple);
+	read_unlock_bh(&nf_conntrack_lock);
+
+	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto,
+					 exp->flags);
 	if (conntrack == NULL || IS_ERR(conntrack)) {
 		DEBUGP("Can't allocate conntrack.\n");
 		return (struct nf_conntrack_tuple_hash *)conntrack;
@@ -672,7 +713,6 @@
 	}
 
 	write_lock_bh(&nf_conntrack_lock);
-	exp = find_expectation(tuple);
 
 	if (exp) {
 		DEBUGP("conntrack: expectation arrives ct=%p exp=%p\n",
@@ -691,6 +731,9 @@
 	} else
 		NF_CT_STAT_INC(new);
 
+	/* Set helper protected by the lock */
+	__nf_conntrack_set_helper(conntrack, &repl_tuple);
+
 	/* Overload tuple linked list to put us in unconfirmed list. */
 	list_add(&conntrack->tuplehash[IP_CT_DIR_ORIGINAL].list, &unconfirmed);
 
@@ -847,16 +890,19 @@
 {
 	struct nf_conn_help *help = nfct_help(conntrack);
 
-	write_lock_bh(&nf_conntrack_lock);
 	/* Should be unconfirmed, so not in hash table yet */
 	NF_CT_ASSERT(!nf_ct_is_confirmed(conntrack));
 
 	DEBUGP("Altering reply tuple of %p to ", conntrack);
 	NF_CT_DUMP_TUPLE(newreply);
-
+	
+	write_lock_bh(&nf_conntrack_lock);
+	if (!keep_helper) {
+		NF_CT_ASSERT(conntrack->features & NF_CT_F_HELP);
+		if (!conntrack->master && help->expecting == 0)
+			help->helper = __nf_ct_helper_find(newreply);
+	}
 	conntrack->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
-	if (!conntrack->master && help->expecting == 0)
-		help->helper = __nf_ct_helper_find(newreply);
 	write_unlock_bh(&nf_conntrack_lock);
 }
 
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c linux-2.6.20-nat-core/net/netfilter/nf_conntrack_helper_h323.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core/net/netfilter/nf_conntrack_helper_h323.c	2006-11-06 14:39:04.000000000 +0100
@@ -693,6 +693,7 @@
 				    port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_h245_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect H.245 ");
@@ -805,6 +806,7 @@
 					      taddr, port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_q931_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect Call Forwarding ");
@@ -1298,6 +1300,7 @@
 				    port, exp);
 	} else {		/* Conntrack only */
 		exp->expectfn = nf_conntrack_q931_expect;
+		exp->flags = NF_CT_EXPECT_HELPER;
 
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1363,6 +1366,7 @@
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_UDP, NULL, &port);
 	exp->expectfn = nf_conntrack_ras_expect;
+        exp->flags = NF_CT_EXPECT_HELPER;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect RAS ");
@@ -1558,7 +1562,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
-	exp->flags = NF_CT_EXPECT_PERMANENT;
+	exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_HELPER;
 	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
@@ -1608,7 +1612,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
-	exp->flags = NF_CT_EXPECT_PERMANENT;
+	exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_HELPER;
 	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {

[-- Attachment #3: nat-core-patrick.patch --]
[-- Type: TEXT/PLAIN, Size: 18684 bytes --]

diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/include/linux/netfilter/nf_conntrack_h323.h linux-2.6.20-nat-core-p/include/linux/netfilter/nf_conntrack_h323.h
--- linux-2.6.20-orig/include/linux/netfilter/nf_conntrack_h323.h	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core-p/include/linux/netfilter/nf_conntrack_h323.h	2006-11-15 14:08:53.000000000 +0100
@@ -32,10 +32,6 @@
 extern int get_h225_addr(struct nf_conn *ct, unsigned char *data,
 			 TransportAddress *taddr,
 			 union nf_conntrack_address *addr, __be16 *port);
-extern void nf_conntrack_h245_expect(struct nf_conn *new,
-				     struct nf_conntrack_expect *this);
-extern void nf_conntrack_q931_expect(struct nf_conn *new,
-				     struct nf_conntrack_expect *this);
 extern int (*set_h245_addr_hook) (struct sk_buff **pskb,
 				  unsigned char **data, int dataoff,
 				  H245_TransportAddress *taddr,
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h linux-2.6.20-nat-core-p/include/net/netfilter/nf_conntrack_expect.h
--- linux-2.6.20-orig/include/net/netfilter/nf_conntrack_expect.h	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core-p/include/net/netfilter/nf_conntrack_expect.h	2006-11-15 14:09:50.000000000 +0100
@@ -22,6 +22,9 @@
 	void (*expectfn)(struct nf_conn *new,
 			 struct nf_conntrack_expect *this);
 
+	/* Helper to assign to new connection */
+	struct nf_conntrack_helper *helper;
+
 	/* The conntrack of the master connection */
 	struct nf_conn *master;
 
@@ -49,7 +52,6 @@
 
 #define NF_CT_EXPECT_PERMANENT 0x1
 
-
 struct nf_conntrack_expect *
 __nf_conntrack_expect_find(const struct nf_conntrack_tuple *tuple);
 
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/ipv4/netfilter/nf_nat_h323.c linux-2.6.20-nat-core-p/net/ipv4/netfilter/nf_nat_h323.c
--- linux-2.6.20-orig/net/ipv4/netfilter/nf_nat_h323.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core-p/net/ipv4/netfilter/nf_nat_h323.c	2006-11-15 14:14:51.000000000 +0100
@@ -324,18 +324,6 @@
 	return 0;
 }
 
-/****************************************************************************
- * This conntrack expect function replaces nf_conntrack_h245_expect()
- * which was set by nf_conntrack_helper_h323.c. It calls both
- * nf_nat_follow_master() and nf_conntrack_h245_expect()
- ****************************************************************************/
-static void ip_nat_h245_expect(struct nf_conn *new,
-			       struct nf_conntrack_expect *this)
-{
-	nf_nat_follow_master(new, this);
-	nf_conntrack_h245_expect(new, this);
-}
-
 /****************************************************************************/
 static int nat_h245(struct sk_buff **pskb, struct nf_conn *ct,
 		    enum ip_conntrack_info ctinfo,
@@ -349,7 +337,7 @@
 
 	/* Set expectations for NAT */
 	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
-	exp->expectfn = ip_nat_h245_expect;
+	exp->expectfn = nf_nat_follow_master;
 	exp->dir = !dir;
 
 	/* Check existing expects */
@@ -399,7 +387,7 @@
 
 	if (this->tuple.src.u3.ip != 0) {	/* Only accept calls from GK */
 		nf_nat_follow_master(new, this);
-		goto out;
+		return;
 	}
 
 	/* This must be a fresh one. */
@@ -420,9 +408,6 @@
 
 	/* hook doesn't matter, but it has to do destination manip */
 	nf_nat_setup_info(new, &range, NF_IP_PRE_ROUTING);
-
-      out:
-	nf_conntrack_q931_expect(new, this);
 }
 
 /****************************************************************************/
@@ -510,8 +495,6 @@
 
 	/* hook doesn't matter, but it has to do destination manip */
 	nf_nat_setup_info(new, &range, NF_IP_PRE_ROUTING);
-
-	nf_conntrack_q931_expect(new, this);
 }
 
 /****************************************************************************/
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_core.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_core.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_core.c	2006-11-15 14:34:16.000000000 +0100
@@ -81,6 +81,11 @@
 DEFINE_PER_CPU(struct ip_conntrack_stat, nf_conntrack_stat);
 EXPORT_PER_CPU_SYMBOL(nf_conntrack_stat);
 
+/* This is for NAT. Icky! */
+static int keep_helper = 0;	/* Backward compatibility */
+module_param(keep_helper, bool, 0400);
+MODULE_PARM_DESC(keep_helper, "keep helper found by conntrack, regardless of NAT");
+
 /*
  * This scheme offers various size of "struct nf_conn" dependent on
  * features(helper, nat, ...)
@@ -404,6 +409,18 @@
 		 &nf_conntrack_hash[repl_hash]);
 }
 
+static inline void 
+__nf_conntrack_set_helper(struct nf_conn *ct,
+			  const struct nf_conntrack_tuple *repl)
+{
+	/* Set helper, if required */
+	if (ct->features & NF_CT_F_HELP) {
+		struct nf_conn_help *help = nfct_help(ct);
+		NF_CT_ASSERT(help);
+		help->helper = __nf_ct_helper_find(repl);
+	}
+}
+
 void nf_conntrack_hash_insert(struct nf_conn *ct)
 {
 	unsigned int hash, repl_hash;
@@ -412,6 +429,7 @@
 	repl_hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 
 	write_lock_bh(&nf_conntrack_lock);
+	__nf_conntrack_set_helper(ct, &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 	__nf_conntrack_hash_insert(ct, hash, repl_hash);
 	write_unlock_bh(&nf_conntrack_lock);
 }
@@ -540,14 +558,52 @@
 	return dropped;
 }
 
+static inline struct nf_conn * __nf_ct_alloc_feature(u_int32_t features)
+{
+	struct nf_conn *conntrack = NULL;
+
+	DEBUGP("__nf_ct_alloc_feature: features=0x%x\n", features);
+
+	read_lock_bh(&nf_ct_cache_lock);
+	if (unlikely(!nf_ct_cache[features].use)) {
+		DEBUGP("__nf_ct_alloc_feature: not supported features = 0x%x\n",
+			features);
+		goto out;
+	}
+
+	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	if (conntrack == NULL) {
+		DEBUGP("__nf_ct_alloc_feature: Can't alloc conntrack from cache\n");
+		goto out;
+	}
+	memset(conntrack, 0, nf_ct_cache[features].size);
+
+   out:
+   	read_unlock_bh(&nf_ct_cache_lock);
+   	return conntrack;
+}
+
+static struct nf_conntrack_helper *
+__nf_conntrack_helper_required(const struct nf_conntrack_tuple *repl)
+{
+	struct nf_conntrack_helper *helper;
+
+	/* FIXME: protect helper list per RCU */
+	read_lock_bh(&nf_conntrack_lock);
+	helper = __nf_ct_helper_find(repl);
+	read_unlock_bh(&nf_conntrack_lock);
+
+	return helper;
+}	
+
 static struct nf_conn *
 __nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
 		     const struct nf_conntrack_tuple *repl,
-		     const struct nf_conntrack_l3proto *l3proto)
+		     const struct nf_conntrack_l3proto *l3proto,
+		     unsigned int expect_helper)
 {
 	struct nf_conn *conntrack = NULL;
 	u_int32_t features = 0;
-	struct nf_conntrack_helper *helper;
 
 	if (unlikely(!nf_conntrack_hash_rnd_initted)) {
 		get_random_bytes(&nf_conntrack_hash_rnd, 4);
@@ -574,37 +630,22 @@
 	/*  find features needed by this conntrack. */
 	features = l3proto->get_features(orig);
 
-	/* FIXME: protect helper list per RCU */
-	read_lock_bh(&nf_conntrack_lock);
-	helper = __nf_ct_helper_find(repl);
-	if (helper)
+	/* Helper oraculum:
+	 * - expectation carries a helper function
+	 * - NAT may change (i.e. add) helper
+	 * - standard helper found
+	 */
+	if (expect_helper
+	    || ((features & NF_CT_F_NAT) && !keep_helper)
+	    || (__nf_conntrack_helper_required(repl) != NULL))
 		features |= NF_CT_F_HELP;
-	read_unlock_bh(&nf_conntrack_lock);
-
-	DEBUGP("nf_conntrack_alloc: features=0x%x\n", features);
-
-	read_lock_bh(&nf_ct_cache_lock);
-
-	if (unlikely(!nf_ct_cache[features].use)) {
-		DEBUGP("nf_conntrack_alloc: not supported features = 0x%x\n",
-			features);
-		goto out;
-	}
-
-	conntrack = kmem_cache_alloc(nf_ct_cache[features].cachep, GFP_ATOMIC);
+	conntrack = __nf_ct_alloc_feature(features);
 	if (conntrack == NULL) {
-		DEBUGP("nf_conntrack_alloc: Can't alloc conntrack from cache\n");
-		goto out;
+		atomic_dec(&nf_conntrack_count);
+		return conntrack;
 	}
 
-	memset(conntrack, 0, nf_ct_cache[features].size);
 	conntrack->features = features;
-	if (helper) {
-		struct nf_conn_help *help = nfct_help(conntrack);
-		NF_CT_ASSERT(help);
-		help->helper = helper;
-	}
-
 	atomic_set(&conntrack->ct_general.use, 1);
 	conntrack->ct_general.destroy = destroy_conntrack;
 	conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
@@ -613,13 +654,8 @@
 	init_timer(&conntrack->timeout);
 	conntrack->timeout.data = (unsigned long)conntrack;
 	conntrack->timeout.function = death_by_timeout;
-	read_unlock_bh(&nf_ct_cache_lock);
 
 	return conntrack;
-out:
-	read_unlock_bh(&nf_ct_cache_lock);
-	atomic_dec(&nf_conntrack_count);
-	return conntrack;
 }
 
 struct nf_conn *nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
@@ -628,7 +664,7 @@
 	struct nf_conntrack_l3proto *l3proto;
 
 	l3proto = __nf_ct_l3proto_find(orig->src.l3num);
-	return __nf_conntrack_alloc(orig, repl, l3proto);
+	return __nf_conntrack_alloc(orig, repl, l3proto, 0);
 }
 
 void nf_conntrack_free(struct nf_conn *conntrack)
@@ -659,7 +695,12 @@
 		return NULL;
 	}
 
-	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto);
+	read_lock_bh(&nf_conntrack_lock);
+	exp = find_expectation(tuple);
+	read_unlock_bh(&nf_conntrack_lock);
+
+	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto,
+					 exp && exp->helper != NULL);
 	if (conntrack == NULL || IS_ERR(conntrack)) {
 		DEBUGP("Can't allocate conntrack.\n");
 		return (struct nf_conntrack_tuple_hash *)conntrack;
@@ -672,7 +713,6 @@
 	}
 
 	write_lock_bh(&nf_conntrack_lock);
-	exp = find_expectation(tuple);
 
 	if (exp) {
 		DEBUGP("conntrack: expectation arrives ct=%p exp=%p\n",
@@ -691,6 +731,12 @@
 	} else
 		NF_CT_STAT_INC(new);
 
+	/* Set helper protected by the lock */
+	if (exp && exp->helper)
+		nfct_help(conntrack)->helper = exp->helper;
+	else
+		__nf_conntrack_set_helper(conntrack, &repl_tuple);
+
 	/* Overload tuple linked list to put us in unconfirmed list. */
 	list_add(&conntrack->tuplehash[IP_CT_DIR_ORIGINAL].list, &unconfirmed);
 
@@ -847,16 +893,19 @@
 {
 	struct nf_conn_help *help = nfct_help(conntrack);
 
-	write_lock_bh(&nf_conntrack_lock);
 	/* Should be unconfirmed, so not in hash table yet */
 	NF_CT_ASSERT(!nf_ct_is_confirmed(conntrack));
 
 	DEBUGP("Altering reply tuple of %p to ", conntrack);
 	NF_CT_DUMP_TUPLE(newreply);
-
+	
+	write_lock_bh(&nf_conntrack_lock);
+	if (!keep_helper) {
+		NF_CT_ASSERT(conntrack->features & NF_CT_F_HELP);
+		if (!conntrack->master && help->expecting == 0)
+			help->helper = __nf_ct_helper_find(newreply);
+	}
 	conntrack->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
-	if (!conntrack->master && help->expecting == 0)
-		help->helper = __nf_ct_helper_find(newreply);
 	write_unlock_bh(&nf_conntrack_lock);
 }
 
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_expect.c linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_expect.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_expect.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_expect.c	2006-11-15 14:15:20.000000000 +0100
@@ -211,6 +211,7 @@
 
 	exp->flags = 0;
 	exp->expectfn = NULL;
+	exp->helper = NULL;
 	exp->tuple.src.l3num = family;
 	exp->tuple.dst.protonum = proto;
 	exp->mask.src.l3num = 0xFFFF;
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_helper.c linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_helper.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_helper.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_helper.c	2006-11-15 14:40:57.000000000 +0100
@@ -128,7 +128,8 @@
 	/* Get rid of expectations */
 	list_for_each_entry_safe(exp, tmp, &nf_conntrack_expect_list, list) {
 		struct nf_conn_help *help = nfct_help(exp->master);
-		if (help->helper == me && del_timer(&exp->timeout)) {
+		if ((exp->helper == me || help->helper == me) &&
+		    del_timer(&exp->timeout)) {
 			nf_ct_unlink_expect(exp);
 			nf_conntrack_expect_put(exp);
 		}
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_helper_h323.c
--- linux-2.6.20-orig/net/netfilter/nf_conntrack_helper_h323.c	2006-11-06 14:08:15.000000000 +0100
+++ linux-2.6.20-nat-core-p/net/netfilter/nf_conntrack_helper_h323.c	2006-11-15 14:51:29.000000000 +0100
@@ -617,15 +617,6 @@
 };
 
 /****************************************************************************/
-void nf_conntrack_h245_expect(struct nf_conn *new,
-			      struct nf_conntrack_expect *this)
-{
-	write_lock_bh(&nf_conntrack_lock);
-	nfct_help(new)->helper = &nf_conntrack_helper_h245;
-	write_unlock_bh(&nf_conntrack_lock);
-}
-
-/****************************************************************************/
 int get_h225_addr(struct nf_conn *ct, unsigned char *data,
 		  TransportAddress *taddr,
 		  union nf_conntrack_address *addr, __be16 *port)
@@ -683,6 +674,7 @@
 				 &ct->tuplehash[!dir].tuple.src.u3,
 				 &ct->tuplehash[!dir].tuple.dst.u3,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_h245;
 
 	if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
 		   &ct->tuplehash[!dir].tuple.dst.u3,
@@ -692,8 +684,6 @@
 		ret = nat_h245_hook(pskb, ct, ctinfo, data, dataoff, taddr,
 				    port, exp);
 	} else {		/* Conntrack only */
-		exp->expectfn = nf_conntrack_h245_expect;
-
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect H.245 ");
 			NF_CT_DUMP_TUPLE(&exp->tuple);
@@ -706,10 +696,6 @@
 	return ret;
 }
 
-/* Forwarding declaration */
-void nf_conntrack_q931_expect(struct nf_conn *new,
-			      struct nf_conntrack_expect *this);
-
 /* If the calling party is on the same side of the forward-to party,
  * we don't need to track the second call */
 static int callforward_do_filter(union nf_conntrack_address *src,
@@ -763,6 +749,8 @@
 
 }
 
+static struct nf_conntrack_helper nf_conntrack_helper_q931;
+
 /****************************************************************************/
 static int expect_callforwarding(struct sk_buff **pskb,
 				 struct nf_conn *ct,
@@ -795,6 +783,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 
 	if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
 		   &ct->tuplehash[!dir].tuple.dst.u3,
@@ -804,8 +793,6 @@
 		ret = nat_callforwarding_hook(pskb, ct, ctinfo, data, dataoff,
 					      taddr, port, exp);
 	} else {		/* Conntrack only */
-		exp->expectfn = nf_conntrack_q931_expect;
-
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_q931: expect Call Forwarding ");
 			NF_CT_DUMP_TUPLE(&exp->tuple);
@@ -1199,15 +1186,6 @@
 };
 
 /****************************************************************************/
-void nf_conntrack_q931_expect(struct nf_conn *new,
-			      struct nf_conntrack_expect *this)
-{
-	write_lock_bh(&nf_conntrack_lock);
-	nfct_help(new)->helper = &nf_conntrack_helper_q931;
-	write_unlock_bh(&nf_conntrack_lock);
-}
-
-/****************************************************************************/
 static unsigned char *get_udp_data(struct sk_buff **pskb, unsigned int protoff,
 				   int *datalen)
 {
@@ -1291,14 +1269,13 @@
 					NULL,
 				 &ct->tuplehash[!dir].tuple.dst.u3,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 	exp->flags = NF_CT_EXPECT_PERMANENT;	/* Accept multiple calls */
 
 	if (nat_q931_hook && ct->status & IPS_NAT_MASK) {	/* Need NAT */
 		ret = nat_q931_hook(pskb, ct, ctinfo, data, taddr, i,
 				    port, exp);
 	} else {		/* Conntrack only */
-		exp->expectfn = nf_conntrack_q931_expect;
-
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_ras: expect Q.931 ");
 			NF_CT_DUMP_TUPLE(&exp->tuple);
@@ -1327,9 +1304,7 @@
 	return 0;
 }
 
-/* Declare before using */
-static void nf_conntrack_ras_expect(struct nf_conn *new,
-				    struct nf_conntrack_expect *this);
+static struct nf_conntrack_helper nf_conntrack_helper_ras;
 
 /****************************************************************************/
 static int process_gcf(struct sk_buff **pskb, struct nf_conn *ct,
@@ -1362,7 +1337,7 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_UDP, NULL, &port);
-	exp->expectfn = nf_conntrack_ras_expect;
+	exp->helper = &nf_conntrack_helper_ras;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect RAS ");
@@ -1558,8 +1533,8 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 	exp->flags = NF_CT_EXPECT_PERMANENT;
-	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1608,8 +1583,8 @@
 	nf_conntrack_expect_init(exp, ct->tuplehash[!dir].tuple.src.l3num,
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
+	exp->helper = &nf_conntrack_helper_q931;
 	exp->flags = NF_CT_EXPECT_PERMANENT;
-	exp->expectfn = nf_conntrack_q931_expect;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1754,15 +1729,6 @@
 };
 
 /****************************************************************************/
-static void nf_conntrack_ras_expect(struct nf_conn *new,
-				    struct nf_conntrack_expect *this)
-{
-	write_lock_bh(&nf_conntrack_lock);
-	nfct_help(new)->helper = &nf_conntrack_helper_ras;
-	write_unlock_bh(&nf_conntrack_lock);
-}
-
-/****************************************************************************/
 /* Not __exit - called from init() */
 static void fini(void)
 {
@@ -1794,8 +1760,6 @@
 module_exit(fini);
 
 EXPORT_SYMBOL_GPL(get_h225_addr);
-EXPORT_SYMBOL_GPL(nf_conntrack_h245_expect);
-EXPORT_SYMBOL_GPL(nf_conntrack_q931_expect);
 EXPORT_SYMBOL_GPL(set_h245_addr_hook);
 EXPORT_SYMBOL_GPL(set_h225_addr_hook);
 EXPORT_SYMBOL_GPL(set_sig_addr_hook);