newnat patch

From kadlec@blackhole.kfki.hu Mon Sep 17 11:09:21 2001
Date: Fri, 22 Jun 2001 14:15:06 +0200 (CEST)
From: Jozsef Kadlecsik 
To: Harald Welte 
Cc: Netfilter Development Mailinglist 
Subject: Re: Status of the newnat code

On Mon, 18 Jun 2001, Harald Welte wrote:

> > > Resent packets do not generate new expectations, this should be covered
> > > by the helper.

I believe the netfilter infrastructure should do the job and it should not
be left individually for all the helpers. Also, the checking for resent
packets might be harder to do in a helper than in the core.

So I wrote a version, in which the (naive) checking for resent packets is
done in the core. It required to integrate ip_conntrack_alloc_expect into
ip_conntrack_expect_related. Also, because all checks are done at holding
the ip_conntrack_lock, I converted the expectations counter into a simple
integer.

It passed all (ftp) tests from the testsuite, so at least backward
compatible :-). The incremental patch against the original newnat code is
below.

> > Let's assume a client, who wants to download our CPAN mirror, which
> > currently contains 52755 files, by FTP. That would mean at the end
> > 52755 entry in sibling_list, all of them are - except one - long dead and
> > nonexistent connections.
>
> no. As soon as the established expectation dies, the conntrack of the
> data connection removes the respective struct ip_conntrack_expect.
>
> only the expects for still established connections persist.

Aaack! Did you hear that thumping noise? That was my head banging my
table... Yes, you're right!

Still, I dont' understand two lines in destroy_conntrack:

        WRITE_LOCK(&ip_conntrack_lock);
        /* remove it from the expectation list, as there may
         * expectations survive and we want to leave their list
         * in a consistent state -HW */

I read the sentence ones, twice, several times, still don't understand
the line below. Shouldn't destroy_expectations be called instead??
This is a conntrack entry which is just destroyed. It can happen, if it
has no expected connections but it may have (multiple) pending
expectations.

        list_del(&ct->sibling_list);
        /* Destroy our master expectation, if we've been expected */
        if (ct->master)

The master expectation was already deleted from the global list
at the initialization of the connection, however unexpect_related
wants to delete it from both the local and global lists.

                unexpect_related(ct->master);
        WRITE_UNLOCK(&ip_conntrack_lock);

Where am I wrong (again)?

Regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
WWW-Home: http://www.kfki.hu/~kadlec
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

diff -urN --exclude-from=diff.exclude linux-2.4.5/include/linux/netfilter_ipv4.newnat1/ip_conntrack.h linux-2.4.5/include/linux/netfilter_ipv4.newnat2/ip_conntrack.h
--- linux-2.4.5/include/linux/netfilter_ipv4.newnat1/ip_conntrack.h	Tue Jun 12 09:10:50 2001
+++ linux-2.4.5/include/linux/netfilter_ipv4.newnat2/ip_conntrack.h	Wed Jun 20 17:15:12 2001
@@ -126,6 +126,9 @@
 	/* If we're expecting another related connection, this will be
            in expected linked list */
 	struct list_head sibling_list;
+
+	/* Current number of expected connections */
+	unsigned int expecting;

 	/* If we were expected by an expectation, this will be it */
 	struct ip_conntrack_expect *master;
diff -urN --exclude-from=diff.exclude linux-2.4.5/include/linux/netfilter_ipv4.newnat1/ip_conntrack_helper.h linux-2.4.5/include/linux/netfilter_ipv4.newnat2/ip_conntrack_helper.h
--- linux-2.4.5/include/linux/netfilter_ipv4.newnat1/ip_conntrack_helper.h	Tue Jun 12 09:10:50 2001
+++ linux-2.4.5/include/linux/netfilter_ipv4.newnat2/ip_conntrack_helper.h	Wed Jun 20 17:31:22 2001
@@ -14,6 +14,9 @@
 	struct ip_conntrack_tuple tuple;
 	struct ip_conntrack_tuple mask;

+	/* Maximum number of concurrent expected connections */
+	unsigned int max_expected;
+
 	/* Function to call when data passes; return verdict, or -1 to
            invalidate. */
 	int (*help)(const struct iphdr *, size_t len,
@@ -24,10 +27,7 @@
 extern int ip_conntrack_helper_register(struct ip_conntrack_helper *);
 extern void ip_conntrack_helper_unregister(struct ip_conntrack_helper *);

-struct ip_conntrack_expect *
-ip_conntrack_alloc_expect(struct ip_conntrack *related_to);
-
-/* Add an expected connection: can only have one per connection */
+/* Add an expected connection: can have more than one per connection */
 extern int ip_conntrack_expect_related(struct ip_conntrack *related_to,
 				       struct ip_conntrack_expect *exp);
 extern void ip_conntrack_unexpect_related(struct ip_conntrack_expect *exp);
diff -urN --exclude-from=diff.exclude linux-2.4.5/net/ipv4/netfilter.newnat1/ip_conntrack_core.c linux-2.4.5/net/ipv4/netfilter.newnat2/ip_conntrack_core.c
--- linux-2.4.5/net/ipv4/netfilter.newnat1/ip_conntrack_core.c	Tue Jun 12 09:10:50 2001
+++ linux-2.4.5/net/ipv4/netfilter.newnat2/ip_conntrack_core.c	Wed Jun 20 17:23:57 2001
@@ -163,6 +163,8 @@
 	/* delete from global and local lists */
 	list_del(&expect->list);
 	list_del(&expect->expected_list);
+	if (!expect->sibling)
+		expect->expectant->expecting--;
 	kfree(expect);
 }

@@ -616,6 +618,7 @@
 		expected->sibling = conntrack;
 		IP_NF_ASSERT(master_ct(conntrack));
 		LIST_DELETE(&expect_list, expected);
+		expected->expectant->expecting--;
 		nf_conntrack_get(&master_ct(conntrack)->infos[0]);
 	}
 	atomic_inc(&ip_conntrack_count);
@@ -786,53 +789,56 @@
 	return ip_ct_tuple_mask_cmp(&i->tuple, &new->tuple, &intersect_mask);
 }

-/* Allocate a new expectation */
-struct ip_conntrack_expect *
-ip_conntrack_alloc_expect(struct ip_conntrack *related_to)
-{
-	struct ip_conntrack_expect *expect;
-
-	/* FIXME: Ideally we want to have a helper-defined limit
-	 * for the maximum number of expectations for one master conn -HW */
-
-	expect = (struct ip_conntrack_expect *)
-		 kmalloc(sizeof(*expect), GFP_ATOMIC);
-
-	if (!expect) {
-		if (net_ratelimit())
-			printk("ip_conntrack: OOM allocating expect\n");
-		return NULL;
-	}
-
-	/* initialize */
-	memset(expect, 0, sizeof(*expect));
-	INIT_LIST_HEAD(&expect->list);
-	INIT_LIST_HEAD(&expect->expected_list);
-
-	/* FIXME: bump the above mentioned expectation count -HW */
-
-	return expect;
-}
-
 /* Add a related connection. */
 int ip_conntrack_expect_related(struct ip_conntrack *related_to,
-				struct ip_conntrack_expect *exp)
+				struct ip_conntrack_expect *expect)
 {
+	struct ip_conntrack_expect *new;
+
 	WRITE_LOCK(&ip_conntrack_lock);
-	DEBUGP("ip_conntrack_expect_related(%p, %p)\n", related_to, exp);
+	DEBUGP("ip_conntrack_expect_related %p\n", related_to);
+	DUMP_TUPLE(&expect->tuple);
+	DUMP_TUPLE(&expect->mask);
+
+	if (LIST_FIND(&related_to->sibling_list, expect_clash,
+		      struct ip_conntrack_expect *, expect)) {
+		WRITE_UNLOCK(&ip_conntrack_lock);
+		DEBUGP("expect_related: resent packet\n");
+		return -EEXIST;
+	}

+	if (related_to->helper->max_expected
+	    && related_to->expecting >= related_to->helper->max_expected) {
+	    	DEBUGP("expect_related: max number of expected connections (%i) reached\n",
+	    	       related_to->helper->max_expected);
+		WRITE_UNLOCK(&ip_conntrack_lock);
+		return -EPERM;
+	}
+
 	if (LIST_FIND(&expect_list, expect_clash,
-		      struct ip_conntrack_expect *, exp)) {
+		      struct ip_conntrack_expect *, expect)) {
 		WRITE_UNLOCK(&ip_conntrack_lock);
 		DEBUGP("expect_related: busy!\n");
 		return -EBUSY;
 	}

-	exp->expectant = related_to;
+	new = (struct ip_conntrack_expect *)
+	      kmalloc(sizeof(*expect), GFP_ATOMIC);
+
+	if (!new) {
+		WRITE_UNLOCK(&ip_conntrack_lock);
+		DEBUGP("expect_relaed: OOM allocating expect\n");
+		return -ENOMEM;
+	}
+	memcpy(new, expect, sizeof(*expect));
+	INIT_LIST_HEAD(&new->list);
+	INIT_LIST_HEAD(&new->expected_list);
+
+	new->expectant = related_to;
 	/* add to expected list for this connection */
-	list_add(&exp->expected_list, &related_to->sibling_list);
+	list_prepend(&related_to->sibling_list, &new->expected_list);
 	/* add to global list of expectations */
-	list_prepend(&expect_list, &exp->list);
+	list_prepend(&expect_list, &new->list);

 	WRITE_UNLOCK(&ip_conntrack_lock);

diff -urN --exclude-from=diff.exclude linux-2.4.5/net/ipv4/netfilter.newnat1/ip_conntrack_ftp.c linux-2.4.5/net/ipv4/netfilter.newnat2/ip_conntrack_ftp.c
--- linux-2.4.5/net/ipv4/netfilter.newnat1/ip_conntrack_ftp.c	Tue Jun 12 09:10:50 2001
+++ linux-2.4.5/net/ipv4/netfilter.newnat2/ip_conntrack_ftp.c	Wed Jun 20 17:30:03 2001
@@ -243,8 +243,8 @@
 	int dir = CTINFO2DIR(ctinfo);
 	unsigned int matchlen, matchoff;
 	struct ip_ct_ftp_master *ct_ftp_info = &ct->help.ct_ftp_info;
-	struct ip_ct_ftp_expect *exp_ftp_info;
-	struct ip_conntrack_expect *exp;
+	struct ip_conntrack_expect expect, *exp = &expect;
+	struct ip_ct_ftp_expect *exp_ftp_info = &exp->help.exp_ftp_info;

 	unsigned int i;
 	int found = 0;
@@ -334,17 +334,6 @@
 	       (int)matchlen, data + matchoff,
 	       matchlen, ntohl(tcph->seq) + matchoff);

-	/* allocate a new expectation */
-	exp = ip_conntrack_alloc_expect(ct);
-	if (!exp) {
-		if (net_ratelimit())
-			printk("conntrack_ftp: unable to allocate new "
-			       "expectation\n");
-		return NF_ACCEPT;
-	}
-
-	exp_ftp_info = &exp->help.exp_ftp_info;
-
 	/* Update the ftp info */
 	LOCK_BH(&ip_ftp_lock);
 	if (htonl((array[0] << 24) | (array[1] << 16) | (array[2] << 8) | array[3])
@@ -416,6 +405,7 @@
 		ftp[i].tuple.dst.protonum = IPPROTO_TCP;
 		ftp[i].mask.src.u.tcp.port = 0xFFFF;
 		ftp[i].mask.dst.protonum = 0xFFFF;
+		ftp[i].max_expected = 1;
 		ftp[i].help = help;
 		DEBUGP("ip_ct_ftp: registering helper for port %d\n",
 				ports[i]);
diff -urN --exclude-from=diff.exclude linux-2.4.5/net/ipv4/netfilter.newnat1/ip_conntrack_standalone.c linux-2.4.5/net/ipv4/netfilter.newnat2/ip_conntrack_standalone.c
--- linux-2.4.5/net/ipv4/netfilter.newnat1/ip_conntrack_standalone.c	Tue Jun 12 09:10:50 2001
+++ linux-2.4.5/net/ipv4/netfilter.newnat2/ip_conntrack_standalone.c	Wed Jun 20 17:32:12 2001
@@ -327,7 +327,6 @@
 EXPORT_SYMBOL(ip_conntrack_helper_unregister);
 EXPORT_SYMBOL(ip_ct_selective_cleanup);
 EXPORT_SYMBOL(ip_ct_refresh);
-EXPORT_SYMBOL(ip_conntrack_alloc_expect);
 EXPORT_SYMBOL(ip_conntrack_expect_related);
 EXPORT_SYMBOL(ip_conntrack_unexpect_related);
 EXPORT_SYMBOL(ip_conntrack_tuple_taken);
diff -urN --exclude-from=diff.exclude linux-2.4.5/net/ipv4/netfilter.newnat1/ip_nat_core.c linux-2.4.5/net/ipv4/netfilter.newnat2/ip_nat_core.c
--- linux-2.4.5/net/ipv4/netfilter.newnat1/ip_nat_core.c	Tue Jun 12 09:10:50 2001
+++ linux-2.4.5/net/ipv4/netfilter.newnat2/ip_nat_core.c	Thu Jun 14 11:54:03 2001
@@ -21,6 +21,7 @@
 #define ASSERT_READ_LOCK(x) MUST_BE_READ_LOCKED(&ip_nat_lock)
 #define ASSERT_WRITE_LOCK(x) MUST_BE_WRITE_LOCKED(&ip_nat_lock)

+#include 
 #include 
 #include 
 #include 
diff -urN --exclude-from=diff.exclude linux-2.4.5/net/ipv4/netfilter.newnat1/ip_nat_ftp.c linux-2.4.5/net/ipv4/netfilter.newnat2/ip_nat_ftp.c
--- linux-2.4.5/net/ipv4/netfilter.newnat1/ip_nat_ftp.c	Tue Jun 12 09:10:50 2001
+++ linux-2.4.5/net/ipv4/netfilter.newnat2/ip_nat_ftp.c	Wed Jun 20 17:28:18 2001
@@ -177,11 +177,7 @@
 	struct iphdr *iph = (*pskb)->nh.iph;
 	struct tcphdr *tcph = (void *)iph + iph->ihl*4;
 	u_int16_t port;
-	struct ip_conntrack_expect *exp;
-
-	exp = ip_conntrack_alloc_expect(ct);
-	if (!exp)
-		return 0;
+	struct ip_conntrack_expect expect, *exp = &expect;

 	/* copy whole old expectation */
 	memcpy(exp, oldexp, sizeof(*oldexp));
@@ -298,7 +294,7 @@

 static int __init init(void)
 {
-	int i, ret;
+	int i, ret = 0;
 	char *tmpname;

 	if (ports[0] == 0)
diff -urN --exclude-from=diff.exclude linux-2.4.5/net/ipv4/netfilter.newnat1/ip_nat_helper.c linux-2.4.5/net/ipv4/netfilter.newnat2/ip_nat_helper.c
--- linux-2.4.5/net/ipv4/netfilter.newnat1/ip_nat_helper.c	Fri Apr 27 23:15:01 2001
+++ linux-2.4.5/net/ipv4/netfilter.newnat2/ip_nat_helper.c	Thu Jun 14 11:54:40 2001
@@ -19,6 +19,7 @@
 #define ASSERT_READ_LOCK(x) MUST_BE_READ_LOCKED(&ip_nat_lock)
 #define ASSERT_WRITE_LOCK(x) MUST_BE_WRITE_LOCKED(&ip_nat_lock)

+#include 
 #include 
 #include 
 #include