Re: [PATCH net v3] net: marvell: prestera: fix hw structure laid out

On Fri, Nov 5, 2021 at 5:33 PM Volodymyr Mytnyk [C] [off-list ref] wrote:

quoted

However, there is still this structure that lacks explicit padding:

struct prestera_msg_acl_match {
        __le32 type;
        /* there is a four-byte hole on most architectures, but not on
x86-32 or m68k */

Checked holes on x86_64 with pahole, and there is no holes. Maybe on some
other there will be. Will add 4byte member here to make sure. Thx.

That is very strange, as the union has an __le64 member that should be
64-bit aligned on x86_64.

quoted

struct prestera_msg_event_port_param {
        union {
                struct {
                        __le32 mode;
                        __le32 speed;
                        u8 oper;
                        u8 duplex;
                        u8 fc;
                        u8 fec;
                } mac;
                struct {
                        __le64 lmode_bmap;
                        u8 mdix;
                        u8 fc;
                        u8 __pad[2];
                } __packed phy;
        } __packed;
} __packed;

There is no need to make the outer aggregates __packed, I would
mark only the innermost ones here: mode, speed and lmode_bmap.
Same for prestera_msg_port_cap_param and prestera_msg_port_param.

Will add __packed only to innermost ones. Looks like only phy is required to have __packed.

I think you need it on both lmode_bmap and mode/speed
to get a completely unaligned structure. If you mark phy as __packed,
that will implicitly mark lmode_bmap as packed but leave the
four-byte alignment on mode and speed, so the entire structure
is still four-byte aligned.

       Arnd