nf_conntrack allocation issue

nf_conntrack allocation issue

[Patrick McHardy]

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

I've added these three patches to fix the allocation problems.
I don't want to add the new module option introduced by
Jozsef's patch since the current situation is just ridiculous,
we need to search for both helpers and expectations twice for
every new connection, so this needs to be redesigned anyway.

It seems to work fine in some quick testing. This was the
last problem I'm aware of, so I'm probably going to push
all patches soon.

Last thing is the helper renaming, any objections to prefix
all helpers with nf_conntrack_helper_?

[-- Attachment #2: 01.diff --]
[-- Type: text/plain, Size: 4061 bytes --]

[NETFILTER]: nf_conntrack: automatic helper assignment for expectations

Some helpers (namely H.323) manually assign further helpers to expected
connections. This is not possible with nf_conntrack anymore since we
need to know whether a helper is used at allocation time.

Handle the helper assignment centrally, which allows to perform the
correct allocation and as a nice side effect eliminates the need
for the H.323 helper to fiddle with nf_conntrack_lock.

Mid term the allocation scheme really needs to be redesigned since
we do both the helper and expectation lookup _twice_ for every new
connection.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit fa7ba8735f8bdcc47b2ca5d11c1b8ec6fb7ae241
tree ddcc58530cfeded09283ffd6659c6a44aaaeb294
parent 35d1fac7ef0f6a79e41f86af6d28036570dc21b9
author Patrick McHardy <kaber@trash.net> Thu, 30 Nov 2006 16:23:42 +0100
committer Patrick McHardy <kaber@trash.net> Thu, 30 Nov 2006 16:23:42 +0100

 include/net/netfilter/nf_conntrack_expect.h |    3 +++
 net/netfilter/nf_conntrack_core.c           |   19 ++++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
index 2d335f0..5d853e8 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/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index a672806..44b8744 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -545,10 +545,10 @@ static int early_drop(struct list_head *
 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,
+		     u_int32_t features)
 {
 	struct nf_conn *conntrack = NULL;
-	u_int32_t features = 0;
 	struct nf_conntrack_helper *helper;
 
 	if (unlikely(!nf_conntrack_hash_rnd_initted)) {
@@ -574,7 +574,7 @@ __nf_conntrack_alloc(const struct nf_con
 	}
 
 	/*  find features needed by this conntrack. */
-	features = l3proto->get_features(orig);
+	features |= l3proto->get_features(orig);
 
 	/* FIXME: protect helper list per RCU */
 	read_lock_bh(&nf_conntrack_lock);
@@ -624,7 +624,7 @@ struct nf_conn *nf_conntrack_alloc(const
 	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)
@@ -649,13 +649,20 @@ init_conntrack(const struct nf_conntrack
 	struct nf_conn *conntrack;
 	struct nf_conntrack_tuple repl_tuple;
 	struct nf_conntrack_expect *exp;
+	u_int32_t features = 0;
 
 	if (!nf_ct_invert_tuple(&repl_tuple, tuple, l3proto, l4proto)) {
 		DEBUGP("Can't invert tuple.\n");
 		return NULL;
 	}
 
-	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto);
+	read_lock_bh(&nf_conntrack_lock);
+	exp = __nf_conntrack_expect_find(tuple);
+	if (exp && exp->helper)
+		features = NF_CT_F_HELP;
+	read_unlock_bh(&nf_conntrack_lock);
+
+	conntrack = __nf_conntrack_alloc(tuple, &repl_tuple, l3proto, features);
 	if (conntrack == NULL || IS_ERR(conntrack)) {
 		DEBUGP("Can't allocate conntrack.\n");
 		return (struct nf_conntrack_tuple_hash *)conntrack;
@@ -676,6 +683,8 @@ init_conntrack(const struct nf_conntrack
 		/* Welcome, Mr. Bond.  We've been expecting you... */
 		__set_bit(IPS_EXPECTED_BIT, &conntrack->status);
 		conntrack->master = exp->master;
+		if (exp->helper)
+			nfct_help(conntrack)->helper = exp->helper;
 #ifdef CONFIG_NF_CONNTRACK_MARK
 		conntrack->mark = exp->master->mark;
 #endif

[-- Attachment #3: 02.diff --]
[-- Type: text/plain, Size: 1086 bytes --]

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index b41e785..8c34b67 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -579,7 +579,8 @@ __nf_conntrack_alloc(const struct nf_con
 	/* FIXME: protect helper list per RCU */
 	read_lock_bh(&nf_conntrack_lock);
 	helper = __nf_ct_helper_find(repl);
-	if (helper)
+	/* NAT might want to assign a helper later */
+	if (helper || features & NF_CT_F_NAT)
 		features |= NF_CT_F_HELP;
 	read_unlock_bh(&nf_conntrack_lock);
 
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 83242cd..91b7859 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -866,7 +866,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 && help && help->expecting == 0)
 		help->helper = __nf_ct_helper_find(newreply);
 	write_unlock_bh(&nf_conntrack_lock);
 }

[-- Attachment #4: 03.diff --]
[-- Type: text/plain, Size: 9502 bytes --]

diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index a7e818d..c9ff0c6 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_helper_h323.c b/net/netfilter/nf_conntrack_helper_h323.c
index 67b935b..755c195 100644
--- a/net/netfilter/nf_conntrack_helper_h323.c
+++ b/net/netfilter/nf_conntrack_helper_h323.c
@@ -109,6 +109,10 @@ int (*nat_q931_hook) (struct sk_buff **p
 static DEFINE_SPINLOCK(nf_h323_lock);
 static char *h323_buffer;
 
+static struct nf_conntrack_helper nf_conntrack_helper_h245;
+static struct nf_conntrack_helper nf_conntrack_helper_q931;
+static struct nf_conntrack_helper nf_conntrack_helper_ras;
+
 /****************************************************************************/
 static int get_tpkt_data(struct sk_buff **pskb, unsigned int protoff,
 			 struct nf_conn *ct, enum ip_conntrack_info ctinfo,
@@ -304,9 +308,6 @@ static int expect_rtp_rtcp(struct sk_buf
 		ret = nat_rtp_rtcp(pskb, ct, ctinfo, data, dataoff,
 				   taddr, port, rtp_port, rtp_exp, rtcp_exp);
 	} else {		/* Conntrack only */
-		rtp_exp->expectfn = NULL;
-		rtcp_exp->expectfn = NULL;
-
 		if (nf_conntrack_expect_related(rtp_exp) == 0) {
 			if (nf_conntrack_expect_related(rtcp_exp) == 0) {
 				DEBUGP("nf_ct_h323: expect RTP ");
@@ -365,7 +366,6 @@ static int expect_t120(struct sk_buff **
 		ret = nat_t120(pskb, ct, ctinfo, data, dataoff, taddr,
 			       port, exp);
 	} else {		/* Conntrack only */
-		exp->expectfn = NULL;
 		if (nf_conntrack_expect_related(exp) == 0) {
 			DEBUGP("nf_ct_h323: expect T.120 ");
 			NF_CT_DUMP_TUPLE(&exp->tuple);
@@ -622,15 +622,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)
@@ -689,6 +680,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,
@@ -699,8 +691,6 @@ static int expect_h245(struct sk_buff **
 		ret = nat_h245(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);
@@ -713,10 +703,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,
@@ -805,6 +791,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,
@@ -815,8 +802,6 @@ static int expect_callforwarding(struct 
 		ret = nat_callforwarding(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);
@@ -1210,15 +1195,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)
 {
@@ -1303,14 +1279,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 */
 
 	nat_q931 = rcu_dereference(nat_q931_hook);
 	if (nat_q931 && ct->status & IPS_NAT_MASK) {	/* Need NAT */
 		ret = nat_q931(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);
@@ -1342,10 +1317,6 @@ 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 int process_gcf(struct sk_buff **pskb, struct nf_conn *ct,
 		       enum ip_conntrack_info ctinfo,
@@ -1377,7 +1348,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 ");
@@ -1583,7 +1554,7 @@ static int process_acf(struct sk_buff **
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
 	exp->flags = NF_CT_EXPECT_PERMANENT;
-	exp->expectfn = nf_conntrack_q931_expect;
+	exp->helper = &nf_conntrack_helper_q931;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1636,7 +1607,7 @@ static int process_lcf(struct sk_buff **
 				 &ct->tuplehash[!dir].tuple.src.u3, &addr,
 				 IPPROTO_TCP, NULL, &port);
 	exp->flags = NF_CT_EXPECT_PERMANENT;
-	exp->expectfn = nf_conntrack_q931_expect;
+	exp->helper = &nf_conntrack_helper_q931;
 
 	if (nf_conntrack_expect_related(exp) == 0) {
 		DEBUGP("nf_ct_ras: expect Q.931 ");
@@ -1785,15 +1756,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)
 {
@@ -1825,8 +1787,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_conntrack allocation issue

[Yasuyuki KOZAKAI]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 30 Nov 2006 16:45:19 +0100

> I've added these three patches to fix the allocation problems.
> I don't want to add the new module option introduced by
> Jozsef's patch since the current situation is just ridiculous,
> we need to search for both helpers and expectations twice for
> every new connection, so this needs to be redesigned anyway.

I agree.

> It seems to work fine in some quick testing. This was the
> last problem I'm aware of, so I'm probably going to push
> all patches soon.
> 
> Last thing is the helper renaming, any objections to prefix
> all helpers with nf_conntrack_helper_?

no objection.

-- Yasuyuki Kozakai

Re: nf_conntrack allocation issue

[Jozsef Kadlecsik]

Hi Patrick,

On Thu, 30 Nov 2006, Patrick McHardy wrote:

> I've added these three patches to fix the allocation problems.
> I don't want to add the new module option introduced by
> Jozsef's patch since the current situation is just ridiculous,

In your patch the part

--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -579,7 +579,8 @@ __nf_conntrack_alloc(const struct nf_con
        /* FIXME: protect helper list per RCU */
        read_lock_bh(&nf_conntrack_lock);
        helper = __nf_ct_helper_find(repl);
-       if (helper)
+       /* NAT might want to assign a helper later */
+       if (helper || features & NF_CT_F_NAT)
                features |= NF_CT_F_HELP;
        read_unlock_bh(&nf_conntrack_lock);
 
means that in the IPv4+NAT case we loose what we wanted to gain by the 
features, i.e. for IPv4 with NAT all conntrack entries will be full sized.

> we need to search for both helpers and expectations twice for
> every new connection, so this needs to be redesigned anyway.

As I see this is the price we pay for the features.

What disturbs me is that by the patch segment above, in spite of the 
features we does not gain anything in memory footprint. And the only 
reason for this is that we execute an internal helper lookup after (D)NAT, 
which can *possibly* change the helper (and which is a nice feature).

In order not to loose practically the features in the IPv4+NAT case I 
added the 'keep_helper' module parameter. Because - I think - the typical 
DNAT setups do not rely on the second helper lookup, the good default 
value were '1', i.e. do not waste time on looking up the helper again 
after DNAT *and* in the new structure keep the features. The backward 
compatibility would dictate that the default value is '0'.

I do not argue for the module parameter. But let us do not throw away the 
features in the case of the most usual configuration in the present days.
 
> Last thing is the helper renaming, any objections to prefix
> all helpers with nf_conntrack_helper_?

Fine, that'll make the code more clear.

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

Re: nf_conntrack allocation issue

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> Hi Patrick,
> 
> In your patch the part
> 
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -579,7 +579,8 @@ __nf_conntrack_alloc(const struct nf_con
>         /* FIXME: protect helper list per RCU */
>         read_lock_bh(&nf_conntrack_lock);
>         helper = __nf_ct_helper_find(repl);
> -       if (helper)
> +       /* NAT might want to assign a helper later */
> +       if (helper || features & NF_CT_F_NAT)
>                 features |= NF_CT_F_HELP;
>         read_unlock_bh(&nf_conntrack_lock);
>  
> means that in the IPv4+NAT case we loose what we wanted to gain by the 
> features, i.e. for IPv4 with NAT all conntrack entries will be full sized.

Yes - actually that part is similar to your patch besides the
keep_helper option.

>>we need to search for both helpers and expectations twice for
>>every new connection, so this needs to be redesigned anyway.
> 
> 
> As I see this is the price we pay for the features.

I see it as a sign of serious misfit of the allocation scheme.
The helper and expectation lookups both really suck performance
wise already, doing them twice is just horrible.

> What disturbs me is that by the patch segment above, in spite of the 
> features we does not gain anything in memory footprint. And the only 
> reason for this is that we execute an internal helper lookup after (D)NAT, 
> which can *possibly* change the helper (and which is a nice feature).
> 
> In order not to loose practically the features in the IPv4+NAT case I 
> added the 'keep_helper' module parameter. Because - I think - the typical 
> DNAT setups do not rely on the second helper lookup, the good default 
> value were '1', i.e. do not waste time on looking up the helper again 
> after DNAT *and* in the new structure keep the features. The backward 
> compatibility would dictate that the default value is '0'.
> 
> I do not argue for the module parameter. But let us do not throw away the 
> features in the case of the most usual configuration in the present days.

The reason why I didn't want to add it is that IMO the allocation
scheme needs to be redesigned anyway, so when we do it we might end
up with a module parameter we had for two months and need to carry
around forever. We can still add it any time we want, but for now
my priority is to get it right (i.e. working properly, not
optimized) without doing anything we might regret later. Module
parameters for rather obscure internal stuff tend not to get used
anyway, so most users don't loose anything.

Re: nf_conntrack allocation issue

[Jozsef Kadlecsik]

On Fri, 1 Dec 2006, Patrick McHardy wrote:

[...]  
> The reason why I didn't want to add it is that IMO the allocation
> scheme needs to be redesigned anyway, so when we do it we might end
> up with a module parameter we had for two months and need to carry
> around forever. We can still add it any time we want, but for now
> my priority is to get it right (i.e. working properly, not
> optimized) without doing anything we might regret later. Module
> parameters for rather obscure internal stuff tend not to get used
> anyway, so most users don't loose anything.

You're right. And that double lookup is truly horrid, we'll have to find a 
way to eliminate.

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

Re: nf_conntrack allocation issue

[Patrick McHardy]

Jozsef Kadlecsik wrote:
>>Last thing is the helper renaming, any objections to prefix
>>all helpers with nf_conntrack_helper_?
> 
> 
> Fine, that'll make the code more clear.

Actually I only wanted to rename the files because I kept getting
annoyed by tab completion on the pptp and H.323 helpers requiring
5 interactions. Prefixing everything with nf_conntrack_helper_
turned out quite ugly (way too long), so I now did the opposite
and renamed PPTP and H.323 to be more consistent with the other
helpers.