Re: [PATCH 2] Re: what's the lockingrules for ip_conntrack_expect_list?

[Martin Josefsson <gandalf@wlug.westbo.se>]

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

On Mon, 2002-10-14 at 13:33, Harald Welte wrote:

> > Making find_proto() an inline adds two exported symbols, protocol_list
> > and ip_conntrack_generic_protocol.
> 
> I didn't think about this, sorry.  So I think we now go for the previous
> solution... export an __ function :(

You can't think of everything :)

I saw that you submitted the __ patch, I havn't had time to test the
ip_conntrack_change_expect patch but it looks correct to me, and it
compiles. If you feel dangerous you can submit it.

I've attached it again. (it applies cleanly to 2.4.20-pre10)
 
-- 
/Martin

Never argue with an idiot. They drag you down to their level, then beat
you with experience.

[-- Attachment #2: ip_conntrack_change_expect-lockfix.diff --]
[-- Type: text/plain, Size: 1494 bytes --]

--- linux-2.4.20-pre10/net/ipv4/netfilter/ip_conntrack_core.c.mjufs	2002-10-12 15:23:22.000000000 +0200
+++ linux-2.4.20-pre10/net/ipv4/netfilter/ip_conntrack_core.c	2002-10-12 15:23:00.000000000 +0200
@@ -1060,7 +1060,10 @@
 int ip_conntrack_change_expect(struct ip_conntrack_expect *expect,
 			       struct ip_conntrack_tuple *newtuple)
 {
+	int ret;
+
 	MUST_BE_READ_LOCKED(&ip_conntrack_lock);
+	WRITE_LOCK(&ip_conntrack_expect_tuple_lock);
 
 	DEBUGP("change_expect:\n");
 	DEBUGP("exp tuple: "); DUMP_TUPLE(&expect->tuple);
@@ -1073,26 +1076,25 @@
 		    && LIST_FIND(&ip_conntrack_expect_list, expect_clash,
 			         struct ip_conntrack_expect *, newtuple, &expect->mask)) {
 			/* Force NAT to find an unused tuple */
-			return -1;
+			ret = -1;
 		} else {
-			WRITE_LOCK(&ip_conntrack_expect_tuple_lock);
 			memcpy(&expect->ct_tuple, &expect->tuple, sizeof(expect->tuple));
 			memcpy(&expect->tuple, newtuple, sizeof(expect->tuple));
-			WRITE_UNLOCK(&ip_conntrack_expect_tuple_lock);
-			return 0;
+			ret = 0;
 		}
 	} else {
 		/* Resent packet */
 		DEBUGP("change expect: resent packet\n");
 		if (ip_ct_tuple_equal(&expect->tuple, newtuple)) {
-			return 0;
+			ret = 0;
 		} else {
 			/* Force NAT to choose again the same port */
-			return -1;
+			ret = -1;
 		}
 	}
+	WRITE_UNLOCK(&ip_conntrack_expect_tuple_lock);
 	
-	return -1;
+	return ret;
 }
 
 /* Alter reply tuple (maybe alter helper).  If it's already taken,