Skip to contentSkip to conversation

ipt_connlimit / speed-up.

PATCH In review Pawel Sikora 3 participants 3 messages Apr 21, 2005 – Apr 24, 2005 netfilter

lore
Share & tools
lore

Pawel Sikora

author
#0 lore raw

PATCH 1/1 ipt_connlimit / speed-up.

Hi All,

I've attached a small patch reduces register usage and redundant &0xff.

Regards,
Paweł.

ipt_iphash: (original)

        movl    %eax, %ecx
        movzbl  %ah, %edx
        pushl   %ebx
        movl    %eax, %ebx
        shrl    $16, %ecx
        andl    $255, %eax
        xorl    %eax, %edx
        andl    $255, %ecx
        shrl    $24, %ebx
        xorl    %edx, %ecx
        xorl    %ecx, %ebx
        movl    %ebx, %eax
        popl    %ebx

ret

ipt_iphash: (fixed)

        movl    %eax, %edx
        shrl    $8, %edx
        xorl    %eax, %edx
        shrl    $16, %eax
        xorl    %eax, %edx
        shrl    $8, %eax
        xorl    %eax, %edx
        movzbl  %dl,%eax

ret

Signature · 3 lines
/* Copyright (C) 2003, SCO, Inc. This is valuable Intellectual Property. */

                           #define say(x) lie(x)
linux-2.6.11/net/ipv4/netfilter/ipt_connlimit.c +6 -8
Index: connlimit/linux-2.6.11/net/ipv4/netfilter/ipt_connlimit.c
===================================================================
@@ -35,15 +35,13 @@
 	struct list_head iphash[256];
 };
 
-static int ipt_iphash(u_int32_t addr)
+static inline unsigned __attribute__((regparm(1), const))
+ipt_iphash(const unsigned addr)
 {
-	int hash;
-
-	hash  =  addr        & 0xff;
-	hash ^= (addr >>  8) & 0xff;
-	hash ^= (addr >> 16) & 0xff;
-	hash ^= (addr >> 24) & 0xff;
-	return hash;
+	return ((addr ^
+		(addr >>  8) ^
+		(addr >> 16) ^
+		(addr >> 24)) & 0xff);
 }
 
 static int count_them(struct ipt_connlimit_data *data,
#1 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Pawel Sikora on 2005-04-21 03:10 UTC:

I just had to test one alternative.. :)

Quoted 9 lines

ipt_iphash: (fixed)

movl %eax, %edx

shrl $8, %edx

xorl %eax, %edx
shrl $16, %eax

Quoted 8 lines

xorl %eax, %edx
shrl $8, %eax

xorl %eax, %edx

movzbl %dl,%eax

ret

I grouped the instructions of your solution according to my outdated and maybe incomplete knowledge of how Pentium processors execute instructions in parallell. This gives 7 groups.

linux-2.6.11/net/ipv4/netfilter/ipt_connlimit.c line 44

Quoted 4 lines
linux-2.6.11/net/ipv4/netfilter/ipt_connlimit.c +4 -0
+	return ((addr ^
+		(addr >>  8) ^
+		(addr >> 16) ^
+		(addr >> 24)) & 0xff);

My approach:

	unsigned tmp = addr ^ (addr >> 16);
	return ((tmp >> 8) ^ tmp) & 255;

Which gives:

movl %eax, %edx
shrl $16, %eax

xorl %edx, %eax

movl %eax, %edx

shrl $8, %edx

xorl %edx, %eax

andl $255, %eax

ret

This also gives 7 groups. If the compiler had only used eax instead in "shrl $8, %edx", the instruction could have been grouped with the previous instruction and my solution would have beaten yours, but no, the compiler chose to modify the copy instead of the original.. :)

So both my and your solution end up using the same amount of cpu although my code has one instruction less and also results in one byte less of assembler instructions.. :)

However, on my processor, your solution results in 7 groups of code while mine results in 8 (again, the compiler chose to modify a copy instead of the original), so I'm going to vote for your version :D

For the fun of it, here's what your version looks like on x86_64:

	movl	%edi, %eax
	movl	%edi, %edx

	shrl	$8, %eax
	shrl	$16, %edx

	xorl	%edi, %eax
	shrl	$24, %edi

	xorl	%edx, %eax

	xorl	%edi, %eax

	andl	$255, %eax

	ret

Keep up the good work :)

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCZ2fYxyF48ZTvn+4RAgHyAJ9B3vcxnPRHZOhHT3Eh6U/p8Mjh8QCeJwL2 OPiZ1VtkT8Y+Jug3UwCDdIg=
=UwcV
-----END PGP SIGNATURE-----

#2 lore raw
In reply to Pawel Sikora

linux-2.6.11/net/ipv4/netfilter/ipt_connlimit.c line 38

Quoted 6 lines · Pawel Sikora

Hi All,

I've attached a small patch reduces register usage and redundant &0xff.

linux-2.6.11/net/ipv4/netfilter/ipt_connlimit.c +1 -1
-static int ipt_iphash(u_int32_t addr)
+static inline unsigned __attribute__((regparm(1), const))

Why do you need attribute(regparm) for an inline function?

Regards
Patrick