[BUG] alignment problem in net/core/flow.c:flow_key_compare

[BUG] alignment problem in net/core/flow.c:flow_key_compare

[Måns Rullgård]

There is a problem with alignment in the flow_key_compare function in
net/core/flow.c.  It takes arguments of type struct flowi * and casts
them to flow_compare_t *, which is 64 bits on 64-bit machines.  It
then proceeds to read and compare 64-bit values from these pointers.
The problem is that struct flowi only requires 32-bit alignment, so
these reads cause numerous unaligned exceptions.  On average, I get
nearly 1000 unaligned exceptions per second.

The solutions I see are either to force the alignment of struct flowi
to 64 bits, or to use 32-bit access in flow_key_compare.

-- 
Måns Rullgård
mru@kth.se

Re: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[Måns Rullgård]

mru@kth.se (Måns Rullgård) writes:

> There is a problem with alignment in the flow_key_compare function in
> net/core/flow.c.  It takes arguments of type struct flowi * and casts
> them to flow_compare_t *, which is 64 bits on 64-bit machines.  It
> then proceeds to read and compare 64-bit values from these pointers.
> The problem is that struct flowi only requires 32-bit alignment, so
> these reads cause numerous unaligned exceptions.  On average, I get
> nearly 1000 unaligned exceptions per second.
>
> The solutions I see are either to force the alignment of struct flowi
> to 64 bits, or to use 32-bit access in flow_key_compare.

I forgot to mention that this is kernel 2.6.4.

-- 
Måns Rullgård
mru@kth.se

Re: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[David S. Miller]

On Thu, 18 Mar 2004 12:25:57 +0100
mru@kth.se (Måns Rullgård) wrote:

> mru@kth.se (Måns Rullgård) writes:
> 
> > The solutions I see are either to force the alignment of struct flowi
> > to 64 bits, or to use 32-bit access in flow_key_compare.
> 
> I forgot to mention that this is kernel 2.6.4.

Yes, just add an alignment attribute of some kind to the struct
is probably the best idea.

Re: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[Måns Rullgård]

"David S. Miller" <davem@redhat.com> writes:

> On Thu, 18 Mar 2004 12:25:57 +0100
> mru@kth.se (Måns Rullgård) wrote:
>
>> mru@kth.se (Måns Rullgård) writes:
>> 
>> > The solutions I see are either to force the alignment of struct flowi
>> > to 64 bits, or to use 32-bit access in flow_key_compare.
>> 
>> I forgot to mention that this is kernel 2.6.4.
>
> Yes, just add an alignment attribute of some kind to the struct
> is probably the best idea.

What's the proper way of doing that in kernel code?  Should I use gcc
__attribute__ directly or is there some macro that's preferred?

-- 
Måns Rullgård
mru@kth.se

Fw: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[Andrew Morton]

This is in 2.6.4:

Begin forwarded message:

Date: Thu, 18 Mar 2004 11:53:02 +0100
From: mru@kth.se (Måns Rullgård)
To: linux-kernel@vger.kernel.org
Subject: [BUG] alignment problem in net/core/flow.c:flow_key_compare


There is a problem with alignment in the flow_key_compare function in
net/core/flow.c.  It takes arguments of type struct flowi * and casts
them to flow_compare_t *, which is 64 bits on 64-bit machines.  It
then proceeds to read and compare 64-bit values from these pointers.
The problem is that struct flowi only requires 32-bit alignment, so
these reads cause numerous unaligned exceptions.  On average, I get
nearly 1000 unaligned exceptions per second.

The solutions I see are either to force the alignment of struct flowi
to 64 bits, or to use 32-bit access in flow_key_compare.

-- 
Måns Rullgård
mru@kth.se
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040318162903.59192d13.akpm@osdl.org> (at Thu, 18 Mar 2004 16:29:03 -0800), Andrew Morton <akpm@osdl.org> says:

> The solutions I see are either to force the alignment of struct flowi
> to 64 bits, or to use 32-bit access in flow_key_compare.

Here's the patch.

===== include/net/flow.h 1.10 vs edited =====
--- 1.10/include/net/flow.h	Thu Jan 15 17:18:43 2004
+++ edited/include/net/flow.h	Fri Mar 19 11:45:54 2004
@@ -10,6 +10,12 @@
 #include <linux/in6.h>
 #include <asm/atomic.h>
 
+#if (BITS_PER_LONG == 64)
+#define FLOWI_ALIGN_BYTES 8
+#else
+#define FLOWI_ALIGN_BYTES 4
+#endif
+
 struct flowi {
 	int	oif;
 	int	iif;
@@ -77,7 +83,7 @@
 #define fl_icmp_type	uli_u.icmpt.type
 #define fl_icmp_code	uli_u.icmpt.code
 #define fl_ipsec_spi	uli_u.spi
-};
+} __attribute__((__aligned__(FLOWI_ALIGN_BYTES)));
 
 #define FLOW_DIR_IN	0
 #define FLOW_DIR_OUT	1

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[Andrew Morton]

YOSHIFUJI Hideaki / ____________ <yoshfuji@linux-ipv6.org> wrote:
>
>  
>  +#if (BITS_PER_LONG == 64)
>  +#define FLOWI_ALIGN_BYTES 8
>  +#else
>  +#define FLOWI_ALIGN_BYTES 4
>  +#endif
>  +
>   struct flowi {
>   	int	oif;
>   	int	iif;
>  @@ -77,7 +83,7 @@
>   #define fl_icmp_type	uli_u.icmpt.type
>   #define fl_icmp_code	uli_u.icmpt.code
>   #define fl_ipsec_spi	uli_u.spi
>  -};
>  +} __attribute__((__aligned__(FLOWI_ALIGN_BYTES)));
>  

Why not simply

>  +} __attribute__((__aligned__(BITS_PER_LONG/8)));

?

Re: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[David S. Miller]

On Thu, 18 Mar 2004 18:59:40 -0800
Andrew Morton <akpm@osdl.org> wrote:

> >  -};
> >  +} __attribute__((__aligned__(FLOWI_ALIGN_BYTES)));
> >  
> 
> Why not simply
> 
> >  +} __attribute__((__aligned__(BITS_PER_LONG/8)));

Works for me :-)

Re: [BUG] alignment problem in net/core/flow.c:flow_key_compare

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040318185940.45108d6a.akpm@osdl.org> (at Thu, 18 Mar 2004 18:59:40 -0800), Andrew Morton <akpm@osdl.org> says:

> Why not simply
> 
> >  +} __attribute__((__aligned__(BITS_PER_LONG/8)));
> 
> ?

Okay;

===== include/net/flow.h 1.10 vs edited =====
--- 1.10/include/net/flow.h	Thu Jan 15 17:18:43 2004
+++ edited/include/net/flow.h	Fri Mar 19 12:04:32 2004
@@ -77,7 +77,7 @@
 #define fl_icmp_type	uli_u.icmpt.type
 #define fl_icmp_code	uli_u.icmpt.code
 #define fl_ipsec_spi	uli_u.spi
-};
+} __attribute__((__aligned__(BITS_PER_LONG/8)));
 
 #define FLOW_DIR_IN	0
 #define FLOW_DIR_OUT	1

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA