Re: [PATCH net-next resend 1/2] enetc: Fix endianness issues for enetc_ethtool

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 19 Nov 2020 13:37:21 +0300 Denis Kirjanov wrote:
> On 11/19/20, Claudiu Manoil <claudiu.manoil@nxp.com> wrote:
> > These particular fields are specified in the H/W reference
> > manual as having network byte order format, so enforce big
> > endian annotation for them and clear the related sparse
> > warnings in the process.
> >
> > Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/enetc/enetc_hw.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > index 68ef4f959982..04efccd11162 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > @@ -472,10 +472,10 @@ struct enetc_cmd_rfse {
> >  	u8 smac_m[6];
> >  	u8 dmac_h[6];
> >  	u8 dmac_m[6];
> > -	u32 sip_h[4];
> > -	u32 sip_m[4];
> > -	u32 dip_h[4];
> > -	u32 dip_m[4];
> > +	__be32 sip_h[4];
> > +	__be32 sip_m[4];
> > +	__be32 dip_h[4];
> > +	__be32 dip_m[4];
> >  	u16 ethtype_h;
> >  	u16 ethtype_m;
> >  	u16 ethtype4_h;  
> 
> Hi Claudiu,
> 
> Why the struct is declared without packed?
> I'm seeing that the structure is used in dma transfers in the driver

We prefer not to pack structs unnecessarily in netdev, because it
forces compilers to do inefficient loads on some arches. If the
structure is laid out correctly according to normal C data layout rules
it should be left alone. 

You can add compile times assertions on the size of the structures
to make double-sure things don't break.