Re: [-next] openvswitch BUILD_BUG_ON failed

[Geert Uytterhoeven]

On Fri, Aug 30, 2013 at 3:11 AM, Jesse Gross <jesse@xxxxxxxxxx> wrote:
> On Thu, Aug 29, 2013 at 3:10 PM, David Miller <davem@xxxxxxxxxxxxx> wrote:
>> From: Jesse Gross <jesse@xxxxxxxxxx>
>> Date: Thu, 29 Aug 2013 14:42:22 -0700
>>
>>> On Thu, Aug 29, 2013 at 2:21 PM, Geert Uytterhoeven
>>> <geert@xxxxxxxxxxxxxx> wrote:
>>>> 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@xxxxxxxxxxxxxx

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/