[PATCH] locking issue in alter_reply

[PATCH] locking issue in alter_reply

[Pablo Neira]

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

Hi,

This the first of a set of patches. Actually this patch is a resend, I 
think that it got lost. Actually, we don't need to write lock the 
conntrack table because we always call alter_reply with a non-confirmed 
conntrack.

regards,
Pablo

[-- Attachment #2: lock-alter-reply.patch --]
[-- Type: text/x-patch, Size: 869 bytes --]

diff -u -r1.2 ip_conntrack_core.c
--- a/net/ipv4/netfilter/ip_conntrack_core.c	4 Aug 2004 15:26:55 -0000	1.2
+++ b/net/ipv4/netfilter/ip_conntrack_core.c	4 Aug 2004 16:46:10 -0000
@@ -1075,9 +1075,9 @@
 int ip_conntrack_alter_reply(struct ip_conntrack *conntrack,
 			     const struct ip_conntrack_tuple *newreply)
 {
-	WRITE_LOCK(&ip_conntrack_lock);
+	READ_LOCK(&ip_conntrack_lock);
 	if (__ip_conntrack_find(newreply, conntrack)) {
-		WRITE_UNLOCK(&ip_conntrack_lock);
+		READ_UNLOCK(&ip_conntrack_lock);
 		return 0;
 	}
 	/* Should be unconfirmed, so not in hash table yet */
@@ -1089,7 +1089,7 @@
 	conntrack->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
 	if (!conntrack->master && list_empty(&conntrack->sibling_list))
 		conntrack->helper = ip_ct_find_helper(newreply);
-	WRITE_UNLOCK(&ip_conntrack_lock);
+	READ_UNLOCK(&ip_conntrack_lock);
 
 	return 1;
 }

Re: [PATCH] locking issue in alter_reply

[Patrick McHardy]

Hi Pablo,

Pablo Neira wrote:

> Hi,
>
> This the first of a set of patches. Actually this patch is a resend, I 
> think that it got lost. Actually, we don't need to write lock the 
> conntrack table because we always call alter_reply with a 
> non-confirmed conntrack.

Harald suggested to concentrate our efforts on nf_conntrack and
only continue to put bugfixes in ip_conntrack, and I agree with him.
This patch is fairly small, but I'll let him decide.

Regards
Patrick

Re: [PATCH] locking issue in alter_reply

[Pablo Neira]

Hi Patrick,

Patrick McHardy wrote:

> Harald suggested to concentrate our efforts on nf_conntrack and
> only continue to put bugfixes in ip_conntrack, and I agree with him.
> This patch is fairly small, but I'll let him decide.


I see, actually I haven't seen that patch. Well I agree with you, that 
layer 3 abstraction for the conntrack looks interesting. So I'll 
concentrate my efforts in nf_conntrack since now.

regards,
Pablo