Messages in this thread | | | Subject | Re: [PATCH 3/3] sfc: selftest: fix struct packing | From | Edward Cree <> | Date | Mon, 19 Jun 2023 11:25:49 +0100 |
| |
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.
> --- > drivers/net/ethernet/sfc/falcon/selftest.c | 21 ++++++++++++++++++++- > drivers/net/ethernet/sfc/selftest.c | 21 ++++++++++++++++++++- > drivers/net/ethernet/sfc/siena/selftest.c | 21 ++++++++++++++++++++- > 3 files changed, 60 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/sfc/falcon/selftest.c b/drivers/net/ethernet/sfc/falcon/selftest.c > index 6a454ac6f8763..fb7fcd27a33a5 100644 > --- a/drivers/net/ethernet/sfc/falcon/selftest.c > +++ b/drivers/net/ethernet/sfc/falcon/selftest.c > @@ -40,7 +40,26 @@ > */ > struct ef4_loopback_payload { > struct ethhdr header; > - struct iphdr ip; > + struct { > +#if defined(__LITTLE_ENDIAN_BITFIELD) > + __u8 ihl:4, > + version:4; > +#elif defined (__BIG_ENDIAN_BITFIELD) > + __u8 version:4, > + ihl:4; > +#else > +#error "Please fix <asm/byteorder.h>" > +#endif > + __u8 tos; > + __be16 tot_len; > + __be16 id; > + __be16 frag_off; > + __u8 ttl; > + __u8 protocol; > + __sum16 check; > + __be32 saddr; > + __be32 daddr; > + } __packed ip; /* unaligned struct iphdr */ > struct udphdr udp; > __be16 iteration; > char msg[64]; > diff --git a/drivers/net/ethernet/sfc/selftest.c b/drivers/net/ethernet/sfc/selftest.c > index 3c5227afd4977..440a57953779c 100644 > --- a/drivers/net/ethernet/sfc/selftest.c > +++ b/drivers/net/ethernet/sfc/selftest.c > @@ -43,7 +43,26 @@ > */ > struct efx_loopback_payload { > struct ethhdr header; > - struct iphdr ip; > + struct { > +#if defined(__LITTLE_ENDIAN_BITFIELD) > + __u8 ihl:4, > + version:4; > +#elif defined (__BIG_ENDIAN_BITFIELD) > + __u8 version:4, > + ihl:4; > +#else > +#error "Please fix <asm/byteorder.h>" > +#endif > + __u8 tos; > + __be16 tot_len; > + __be16 id; > + __be16 frag_off; > + __u8 ttl; > + __u8 protocol; > + __sum16 check; > + __be32 saddr; > + __be32 daddr; > + } __packed ip; /* unaligned struct iphdr */ > struct udphdr udp; > __be16 iteration; > char msg[64]; > diff --git a/drivers/net/ethernet/sfc/siena/selftest.c b/drivers/net/ethernet/sfc/siena/selftest.c > index 07715a3d6beab..b8a8b0495f661 100644 > --- a/drivers/net/ethernet/sfc/siena/selftest.c > +++ b/drivers/net/ethernet/sfc/siena/selftest.c > @@ -43,7 +43,26 @@ > */ > struct efx_loopback_payload { > struct ethhdr header; > - struct iphdr ip; > + struct { > +#if defined(__LITTLE_ENDIAN_BITFIELD) > + __u8 ihl:4, > + version:4; > +#elif defined (__BIG_ENDIAN_BITFIELD) > + __u8 version:4, > + ihl:4; > +#else > +#error "Please fix <asm/byteorder.h>" > +#endif > + __u8 tos; > + __be16 tot_len; > + __be16 id; > + __be16 frag_off; > + __u8 ttl; > + __u8 protocol; > + __sum16 check; > + __be32 saddr; > + __be32 daddr; > + } __packed ip; /* unaligned struct iphdr */ > struct udphdr udp; > __be16 iteration; > char msg[64]; >
| |