Messages in this thread | | | Date | Mon, 19 Jun 2023 16:55:27 +0200 | From | "Arnd Bergmann" <> | Subject | Re: [PATCH 3/3] sfc: selftest: fix struct packing |
| |
On Mon, Jun 19, 2023, at 12:25, Edward Cree wrote: > On 19/06/2023 10:12, Arnd Bergmann wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> Three of the sfc drivers define a packed loopback_payload structure with an >> ethernet header followed by an IP header. However, the kernel definition >> of iphdr specifies that this is 4-byte aligned, causing a W=1 warning: >> >> net/ethernet/sfc/siena/selftest.c:46:15: error: field ip within 'struct efx_loopback_payload' is less aligned than 'struct iphdr' and is usually due to 'struct efx_loopback_payload' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access] >> struct iphdr ip; >> >> As the iphdr packing is not easily changed without breaking other code, >> change the three structures to use a local definition instead. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Duplicating the definition isn't the prettiest thing in the world; it'd > do for a quick fix if needed but I assume W=1 warnings aren't blocking > anyone, so maybe defer this one for now and I'll follow up soon with a > rewrite that fixes this more cleanly? My idea is to drop the __packed > from the containing struct, make efx_begin_loopback() copy the layers > separately, and efx_loopback_rx_packet() similarly do something less > direct than casting the packet data to the struct. > > But I don't insist on it; if you want this fix in immediately then I'm > okay with that too.
It's not urgent, if you can come up with a nicer solution, that is probably better than applying my patch now. I have a patch [1] that addresses -Wunaligned-access for all x86 and arm randconfig builds, and this currently affects 23 drivers. Most of the changes don't have changelog texts yet, and some need a more detailed analysis to come up with a correct patch. I'd probably aim for linux-6.6 or later to get them all done, at which point we could move the warning from W=1 to the default set.
Arnd
[1] https://pastebin.com/g6D9NTS4
| |