[PATCH]: Fix ip_conntrack_core arguments to macros

[PATCH]: Fix ip_conntrack_core arguments to macros

[Patrick McHardy]

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

Hi Harald,
this patch fixes some places in ip_conntrack where LIST_FIND and
LIST_DELETE are passed functions with their arguments (like
LIST_FIND(ip_conntrack_hash[hash_conntrack(...)], ...). The argument is
expanded twice, this leads to two calls to hash_conntrack.

Dump of assembler code for function __ip_conntrack_find:
0x000004c0 <__ip_conntrack_find+0>:     push   %ebp
0x000004c1 <__ip_conntrack_find+1>:     mov    %esp,%ebp
0x000004c3 <__ip_conntrack_find+3>:     push   %edi
0x000004c4 <__ip_conntrack_find+4>:     push   %esi
0x000004c5 <__ip_conntrack_find+5>:     mov    0xc(%ebp),%edi
0x000004c8 <__ip_conntrack_find+8>:     mov    0x8(%ebp),%esi
0x000004cb <__ip_conntrack_find+11>:    push   %ebx
0x000004cc <__ip_conntrack_find+12>:    push   %esi
0x000004cd <__ip_conntrack_find+13>:    call   0x80 <hash_conntrack>
0x000004d2 <__ip_conntrack_find+18>:    mov    0x0,%edx
0x000004d8 <__ip_conntrack_find+24>:    pop    %ecx
0x000004d9 <__ip_conntrack_find+25>:    lea    (%edx,%eax,8),%ebx
0x000004dc <__ip_conntrack_find+28>:    lea    0x0(%esi,1),%esi
0x000004e0 <__ip_conntrack_find+32>:    mov    (%ebx),%ebx
0x000004e2 <__ip_conntrack_find+34>:    push   %esi
0x000004e3 <__ip_conntrack_find+35>:    call   0x80 <hash_conntrack>
...

Best regards,
Patrick


BTW: nice talk yesterday, i've seen the slides before but it still was
alot more interesting to here it directly from you. Sorry i didn't showed
up anymore, i had to get some sleep fast after getting up at 7 in the
morning (and drinking too much beer) ;) Are you still at linuxtag ?

[-- Attachment #2: Type: TEXT/plain, Size: 2290 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1211  -> 1.1212 
#	net/ipv4/netfilter/ip_conntrack_core.c	1.19    -> 1.20   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/07/12	kaber@trash.net		1.1212
# [NETFILTER]: Don't use hash_conntrack in macros.
# 
# Disassembly of __ip_conntrack_find shows hash_conntrack is called twice
# because the parameter to LIST_FIND is evaluated twice. Same goes for
# LIST_DELETE when CONFIG_NETFILTER_DEBUG is defined.
# --------------------------------------------
#
diff -Nru a/net/ipv4/netfilter/ip_conntrack_core.c b/net/ipv4/netfilter/ip_conntrack_core.c
--- a/net/ipv4/netfilter/ip_conntrack_core.c	Sat Jul 12 04:56:56 2003
+++ b/net/ipv4/netfilter/ip_conntrack_core.c	Sat Jul 12 04:56:56 2003
@@ -290,14 +290,15 @@
 static void
 clean_from_lists(struct ip_conntrack *ct)
 {
+	unsigned int ho, hr;
+	
 	DEBUGP("clean_from_lists(%p)\n", ct);
 	MUST_BE_WRITE_LOCKED(&ip_conntrack_lock);
-	LIST_DELETE(&ip_conntrack_hash
-		    [hash_conntrack(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple)],
-		    &ct->tuplehash[IP_CT_DIR_ORIGINAL]);
-	LIST_DELETE(&ip_conntrack_hash
-		    [hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple)],
-		    &ct->tuplehash[IP_CT_DIR_REPLY]);
+
+	ho = hash_conntrack(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
+	hr = hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
+	LIST_DELETE(&ip_conntrack_hash[ho], &ct->tuplehash[IP_CT_DIR_ORIGINAL]);
+	LIST_DELETE(&ip_conntrack_hash[hr], &ct->tuplehash[IP_CT_DIR_REPLY]);
 
 	/* Destroy all un-established, pending expectations */
 	remove_expectations(ct);
@@ -369,9 +370,10 @@
 		    const struct ip_conntrack *ignored_conntrack)
 {
 	struct ip_conntrack_tuple_hash *h;
+	unsigned int hash = hash_conntrack(tuple);
 
 	MUST_BE_READ_LOCKED(&ip_conntrack_lock);
-	h = LIST_FIND(&ip_conntrack_hash[hash_conntrack(tuple)],
+	h = LIST_FIND(&ip_conntrack_hash[hash],
 		      conntrack_tuple_cmp,
 		      struct ip_conntrack_tuple_hash *,
 		      tuple, ignored_conntrack);

Re: [PATCH]: Fix ip_conntrack_core arguments to macros

[Harald Welte]

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

On Sat, Jul 12, 2003 at 12:40:47PM +0200, Patrick McHardy wrote:
> Hi Harald,
> this patch fixes some places in ip_conntrack where LIST_FIND and
> LIST_DELETE are passed functions with their arguments (like
> LIST_FIND(ip_conntrack_hash[hash_conntrack(...)], ...). The argument is
> expanded twice, this leads to two calls to hash_conntrack.

ouch, now this is interesting.  I need to look into that further.

Thanks for the patch, give me another day to investigate (and apply)
this and your other patches.

> Best regards,
> Patrick

> BTW: nice talk yesterday, i've seen the slides before but it still was
> alot more interesting to here it directly from you. Sorry i didn't showed
> up anymore, i had to get some sleep fast after getting up at 7 in the
> morning (and drinking too much beer) ;) Are you still at linuxtag ?

Yes, a real pity.  I was expecting to have some chat with you.
Unfortunately I needed to cut down my linuxtag visit to a single day
(just friday to give my presentation) due to a personal issue :(

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH]: Fix ip_conntrack_core arguments to macros

[Patrick McHardy]

Hi Harald,
you've only added 54_ip_nat-macro-args.patch, i assume the other patch 
was 53...
(the conntrack part) and you forgot to add it. Could you also please 
have a look at
http://lists.netfilter.org/pipermail/netfilter-devel/2003-June/011869.html ?
I believe it got lost.

>Thanks for the patch, give me another day to investigate (and apply)
>this and your other patches.
>
>  
>
>>BTW: nice talk yesterday, i've seen the slides before but it still was
>>alot more interesting to here it directly from you. Sorry i didn't showed
>>up anymore, i had to get some sleep fast after getting up at 7 in the
>>morning (and drinking too much beer) ;) Are you still at linuxtag ?
>>    
>>
>
>Yes, a real pity.  I was expecting to have some chat with you.
>Unfortunately I needed to cut down my linuxtag visit to a single day
>(just friday to give my presentation) due to a personal issue :(
>  
>

;( next time for sure. it's not the same but .. i usually hang out on 
efnet, so if
you'd like to chat you can always step by in #königkaber (don't laugh at the
name, it wasn't meant to last ;).

Bye
Patrick