Thread (8 messages) 8 messages, 3 authors, 2013-09-04

Re: [-next] openvswitch BUILD_BUG_ON failed

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2013-08-31 12:11:28
Also in: linux-next, lkml

On Fri, Aug 30, 2013 at 3:11 AM, Jesse Gross [off-list ref] wrote:
On Thu, Aug 29, 2013 at 3:10 PM, David Miller [off-list ref] wrote:
quoted
From: Jesse Gross <redacted>
Date: Thu, 29 Aug 2013 14:42:22 -0700
quoted
On Thu, Aug 29, 2013 at 2:21 PM, Geert Uytterhoeven
[off-list ref] wrote:
quoted
However, I have some doubts about other alignment "enforcements":

"__aligned(__alignof__(long))" makes the whole struct aligned to the
alignment rule for "long":
   1. This is only 2 bytes on m68k, i.e. != sizeof(long).
   2. This is 4 bytes on many 32-bit platforms, which may be less than the
      default alignment for "__be64" (cfr. some members of struct
      ovs_key_ipv4_tunnel), so this may make those 64-bit members unaligned.
Do any of those 32-bit architectures actually care about alignment of
64 bit values? On 32-bit x86, a long is 32 bits but the alignment
requirement of __be64 is also 32 bit.
All except x86-32 do, it is in fact the odd man out with respect to this
issue.
Thanks, good to know.

Andy, do you want to modify your patch to just drop the alignment
specification as Geert suggested (but definitely keep the new build
assert that you added)? It's probably better to just send the patch to
netdev (against net-next) as well since you'll likely get better
comments there and we can fix this faster if you cut out the
middleman.
Why do you want to keep the build asserts?
Is this in-memory structure also transfered as-is over the network?
If yes, you definitely want the padding.

Nevertheless, as the struct contains u32 and even __be64 members, the
size of the struct will always be a multiple of the alignment unit for
64-bit quantities (and thus also for long), as per the C standard.
Hence the check

    BUILD_BUG_ON(sizeof(struct sw_flow_key) % __alignof__(long));

will only catch bad compiler bugs or people adding __packed to the struct.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help