[RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[Eric Dumazet]

On 64bit arches, we can speedup some IPV6 addresses compares, using 64 bits fields in struct in6_addr.

I am not sure if this patch wont break some user ABI, maybe we should use some ifdef(KERNEL) ?

This patch saves some space, and also reduce number of conditional branches in ipv6_addr_equal()

# size vmlinux*
   text    data     bss     dec     hex filename
6257978  692044  660472 7610494  74207e vmlinux
6259178  692044  660472 7611694  74252e vmlinux.before_patch

Note : Maybe ipv6_addr_cmp() could be defined on top of ipv6_addr_equal() ?

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
---
 include/linux/in6.h |    2 ++
 include/net/ipv6.h  |   17 +++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/linux/in6.h b/include/linux/in6.h
index 2a61c82..a4241a6 100644
--- a/include/linux/in6.h
+++ b/include/linux/in6.h
@@ -34,10 +34,12 @@ struct in6_addr
 		__u8		u6_addr8[16];
 		__be16		u6_addr16[8];
 		__be32		u6_addr32[4];
+		__be64		u6_addr64[2];
 	} in6_u;
 #define s6_addr			in6_u.u6_addr8
 #define s6_addr16		in6_u.u6_addr16
 #define s6_addr32		in6_u.u6_addr32
+#define s6_addr64		in6_u.u6_addr64
 };
 
 /* IPv6 Wildcard Address (::) and Loopback Address (::1) defined in RFC2553
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index f70afef..6493ff6 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -343,10 +343,15 @@ #endif
 static inline int ipv6_addr_equal(const struct in6_addr *a1,
 				  const struct in6_addr *a2)
 {
-	return (a1->s6_addr32[0] == a2->s6_addr32[0] &&
-		a1->s6_addr32[1] == a2->s6_addr32[1] &&
-		a1->s6_addr32[2] == a2->s6_addr32[2] &&
-		a1->s6_addr32[3] == a2->s6_addr32[3]);
+#if defined(CONFIG_64BIT)
+	return (((a1->s6_addr64[0] ^ a2->s6_addr64[0]) |
+		(a1->s6_addr64[1] ^ a2->s6_addr64[1])) == 0);
+#else
+	return (((a1->s6_addr32[0] ^ a2->s6_addr32[0]) |
+		(a1->s6_addr32[1] ^ a2->s6_addr32[1]) |
+		(a1->s6_addr32[2] ^ a2->s6_addr32[2]) |
+		(a1->s6_addr32[3] ^ a2->s6_addr32[3])) == 0);
+#endif
 }
 
 static inline int __ipv6_prefix_equal(const __be32 *a1, const __be32 *a2,
@@ -377,8 +382,12 @@ static inline int ipv6_prefix_equal(cons
 
 static inline int ipv6_addr_any(const struct in6_addr *a)
 {
+#if defined(CONFIG_64BIT)
+	return ((a->s6_addr64[0] | a->s6_addr64[1]) == 0);
+#else
 	return ((a->s6_addr32[0] | a->s6_addr32[1] | 
 		 a->s6_addr32[2] | a->s6_addr32[3] ) == 0); 
+#endif
 }
 
 /*

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20070430162851.ca3c7869.dada1@cosmosbay.com> (at Mon, 30 Apr 2007 16:28:51 +0200), Eric Dumazet <dada1@cosmosbay.com> says:

> On 64bit arches, we can speedup some IPV6 addresses compares, using 64 bits fields in struct in6_addr.
> 
> I am not sure if this patch wont break some user ABI, maybe we should use some ifdef(KERNEL) ?

AFAIK, it did and I guess it will.  So, please do not do this even though
it is enclosed with ifdef(__KERNEL__), so far, at least, for 2.6.22.

--yoshfuji

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[Brian Haley]

Eric Dumazet wrote:
> On 64bit arches, we can speedup some IPV6 addresses compares, using 64 bits fields in struct in6_addr.

> diff --git a/include/linux/in6.h b/include/linux/in6.h
> index 2a61c82..a4241a6 100644
> --- a/include/linux/in6.h
> +++ b/include/linux/in6.h
> @@ -34,10 +34,12 @@ struct in6_addr
>  		__u8		u6_addr8[16];
>  		__be16		u6_addr16[8];
>  		__be32		u6_addr32[4];
> +		__be64		u6_addr64[2];
>  	} in6_u;
>  #define s6_addr			in6_u.u6_addr8
>  #define s6_addr16		in6_u.u6_addr16
>  #define s6_addr32		in6_u.u6_addr32
> +#define s6_addr64		in6_u.u6_addr64
>  };

I also had this idea back in 2004:

 >> I will eventually do a 64-bit comparison to see if putting an
 >> #ifdef CONFIG_64BIT is worth it.
 >
 > No, because we cannot assume 64bit alignment.
 >
 > --yoshfuji

The problem is that drivers don't necessarily align the address on the 
correct boundary, so on some 64-bit arches this could be fatal.  There's 
ways around it since I did it in a previous life, but you'd need to copy 
the addresses and hide them in the skb in the rare case, neither of 
which is a great thing to do.

Unless Yoshifuji has a better solution...

-Brian

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[David Miller]

From: Brian Haley <brian.haley@hp.com>
Date: Mon, 30 Apr 2007 12:27:07 -0400

> The problem is that drivers don't necessarily align the address on the 
> correct boundary, so on some 64-bit arches this could be fatal.  There's 
> ways around it since I did it in a previous life, but you'd need to copy 
> the addresses and hide them in the skb in the rare case, neither of 
> which is a great thing to do.

Yes, the majority of the network drivers are only ensuring 32-bit
alignment after the ethernet header currently on receive.  They
were designed with ipv4 in mind long ago and then the logic just
gets copied everywhere.

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[Eric Dumazet]

David Miller a écrit :
> From: Brian Haley <brian.haley@hp.com>
> Date: Mon, 30 Apr 2007 12:27:07 -0400
> 
>> The problem is that drivers don't necessarily align the address on the 
>> correct boundary, so on some 64-bit arches this could be fatal.  There's 
>> ways around it since I did it in a previous life, but you'd need to copy 
>> the addresses and hide them in the skb in the rare case, neither of 
>> which is a great thing to do.
> 
> Yes, the majority of the network drivers are only ensuring 32-bit
> alignment after the ethernet header currently on receive.  They
> were designed with ipv4 in mind long ago and then the logic just
> gets copied everywhere.
> 
> 

Yes I see...

Maybe we could at least define a 'struct in6_addr_k' for internal structures 
only, to speedup some parts of IPV6 stack.

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Mon, 30 Apr 2007 21:08:45 +0200

> Maybe we could at least define a 'struct in6_addr_k' for internal
> structures only, to speedup some parts of IPV6 stack.

I think it's better to put this on the backburner for now :-)

We could achieve all of this if we start from the bottom
and work our way up.

Although there are many variations, the logic drivers use to use
skb_reserve() to align the receive header tends to take on a certain
pattern.  We could pull that out into a common piece of code and then
make it ensure 64-bit alignment without a lot of pain.

The hard part of the work will be handling the cases that don't
fit the pattern, and don't forget about NET_IP_ALIGN :-)

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Mon, 30 Apr 2007 16:28:51 +0200

> On 64bit arches, we can speedup some IPV6 addresses compares, using 64 bits fields in struct in6_addr.
> 
> I am not sure if this patch wont break some user ABI, maybe we should use some ifdef(KERNEL) ?
> 
> This patch saves some space, and also reduce number of conditional branches in ipv6_addr_equal()
> 
> # size vmlinux*
>    text    data     bss     dec     hex filename
> 6257978  692044  660472 7610494  74207e vmlinux
> 6259178  692044  660472 7611694  74252e vmlinux.before_patch
> 
> Note : Maybe ipv6_addr_cmp() could be defined on top of ipv6_addr_equal() ?
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

Unfortunately, as mentioned elsewhere, we're only able to assume
32-bit alignment of ipv6 packet headers and that isn't likely to
change any time soon.

So, for example, you patch would fill my log file with unaligned trap
messages on my sparc64 workstation.

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[Andi Kleen]

David Miller <davem@davemloft.net> writes:
> 
> Unfortunately, as mentioned elsewhere, we're only able to assume
> 32-bit alignment of ipv6 packet headers and that isn't likely to
> change any time soon.

On x86 it would be fine at least -- unaligned access is cheap. I believe
the same is true for POWER4/5/s390x. Should it be made conditional perhaps?

-Andi

Re: [RFC, PATCH] IPV6 : add 64 bits components in struct in6_addr to speedup ipv6_addr_equal() & ipv6_addr_any()

[David Miller]

From: Andi Kleen <andi@firstfloor.org>
Date: 30 Apr 2007 23:34:20 +0200

> David Miller <davem@davemloft.net> writes:
> > 
> > Unfortunately, as mentioned elsewhere, we're only able to assume
> > 32-bit alignment of ipv6 packet headers and that isn't likely to
> > change any time soon.
> 
> On x86 it would be fine at least -- unaligned access is cheap. I believe
> the same is true for POWER4/5/s390x. Should it be made conditional perhaps?

We have that conditional, it's callet {get,put}_unaligned() but it's
better to fix the infrastructure so that conditionalization isn't
even needed.