diff mbox

openvswitch: Fix alignment of struct sw_flow_key.

Message ID 1378408625-18415-1-git-send-email-jesse@nicira.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Jesse Gross Sept. 5, 2013, 7:17 p.m. UTC
sw_flow_key alignment was declared as " __aligned(__alignof__(long))".
However, this breaks on the m68k architecture where long is 32 bit in
size but 16 bit aligned by default. This aligns to the size of a long to
ensure that we can always do comparsions in full long-sized chunks. It
also adds an additional build check to catch any reduction in alignment.

CC: Andy Zhou <azhou@nicira.com>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 net/openvswitch/flow.c | 1 +
 net/openvswitch/flow.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Sept. 5, 2013, 7:25 p.m. UTC | #1
On Thu, Sep 5, 2013 at 9:17 PM, Jesse Gross <jesse@nicira.com> wrote:
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -1981,6 +1981,7 @@ nla_put_failure:
>   * Returns zero if successful or a negative error code. */
>  int ovs_flow_init(void)
>  {
> +       BUILD_BUG_ON(__alignof__(struct sw_flow_key) % __alignof__(long));

> -} __aligned(__alignof__(long));
> +} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */

These don't match: the struct definition says aligned to 4 or 8 bytes, the check
checks for a multiple of the alignment of "long", which is 2, 4 or 8.

Anyway, BITS_PER_LONG/8 is always a multiple of __alignof__(long), so your
check will never fail.

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Sept. 5, 2013, 7:54 p.m. UTC | #2
From: Jesse Gross <jesse@nicira.com>
Date: Thu,  5 Sep 2013 12:17:05 -0700

> sw_flow_key alignment was declared as " __aligned(__alignof__(long))".
> However, this breaks on the m68k architecture where long is 32 bit in
> size but 16 bit aligned by default. This aligns to the size of a long to
> ensure that we can always do comparsions in full long-sized chunks. It
> also adds an additional build check to catch any reduction in alignment.
> 
> CC: Andy Zhou <azhou@nicira.com>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Jesse Gross <jesse@nicira.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index ad1aeeb..fb36f85 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -1981,6 +1981,7 @@  nla_put_failure:
  * Returns zero if successful or a negative error code. */
 int ovs_flow_init(void)
 {
+	BUILD_BUG_ON(__alignof__(struct sw_flow_key) % __alignof__(long));
 	BUILD_BUG_ON(sizeof(struct sw_flow_key) % sizeof(long));
 
 	flow_cache = kmem_cache_create("sw_flow", sizeof(struct sw_flow), 0,
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index b65f885..212fbf7 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -125,7 +125,7 @@  struct sw_flow_key {
 			} nd;
 		} ipv6;
 	};
-} __aligned(__alignof__(long));
+} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */
 
 struct sw_flow {
 	struct rcu_head rcu;