Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760038Ab3ICVpi (ORCPT ); Tue, 3 Sep 2013 17:45:38 -0400 Received: from na3sys009aog124.obsmtp.com ([74.125.149.151]:53271 "HELO na3sys009aog124.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1755324Ab3ICVph (ORCPT ); Tue, 3 Sep 2013 17:45:37 -0400 MIME-Version: 1.0 In-Reply-To: References: <20130829.181001.145658561677052552.davem@davemloft.net> From: Jesse Gross Date: Tue, 3 Sep 2013 14:44:32 -0700 Message-ID: Subject: Re: [-next] openvswitch BUILD_BUG_ON failed To: Geert Uytterhoeven Cc: David Miller , Andy Zhou , "dev@openvswitch.org" , netdev , Linux Kernel Mailing List , Linux-Next Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2738 Lines: 59 On Sat, Aug 31, 2013 at 5:11 AM, Geert Uytterhoeven wrote: > On Fri, Aug 30, 2013 at 3:11 AM, Jesse Gross wrote: >> On Thu, Aug 29, 2013 at 3:10 PM, David Miller wrote: >>> From: Jesse Gross >>> Date: Thu, 29 Aug 2013 14:42:22 -0700 >>> >>>> On Thu, Aug 29, 2013 at 2:21 PM, Geert Uytterhoeven >>>> 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. Well they caught this bug and really don't cost anything. > 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. It's possible that we might want to pack the structure in the future. More generally though, the contents of the struct is really independent of the alignment requirements here because we're accessing it as an array of bytes in long-sized chunks so implicitly depending on the size of the members is not that great. -- 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/