2021-11-05 17:22:16

by Volodymyr Mytnyk

[permalink] [raw]
Subject: [PATCH net v4] net: marvell: prestera: fix hw structure laid out

From: Volodymyr Mytnyk <[email protected]>

The prestera FW v4.0 support commit has been merged
accidentally w/o review comments addressed and waiting
for the final patch set to be uploaded. So, fix the remaining
comments related to structure laid out and build issues.

Reported-by: kernel test robot <[email protected]>
Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
Signed-off-by: Volodymyr Mytnyk <[email protected]>
---

Changes in V2:
- fix structure laid out discussed in:
+ [PATCH net-next v4] net: marvell: prestera: add firmware v4.0 support
https://www.spinics.net/lists/kernel/msg4127689.html
+ [PATCH] [-next] net: marvell: prestera: Add explicit padding
https://www.spinics.net/lists/kernel/msg4130293.html

Changes in V3:
- update commit message
- fix more laid out comments
- split into two patches suggested in:
https://www.spinics.net/lists/netdev/msg778322.html

Changes in V4:
- removed unnecessary __packed attr
- get rid of __packed by adding alignment field
to prestera_msg_port_param.
- fix prestera_msg_acl_match alignment

.../net/ethernet/marvell/prestera/prestera_hw.c | 131 +++++++++++----------
1 file changed, 68 insertions(+), 63 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 4f5f52dcdd9d..5550c0a8bec3 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -180,109 +180,113 @@ struct prestera_msg_common_resp {
struct prestera_msg_ret ret;
};

-union prestera_msg_switch_param {
- u8 mac[ETH_ALEN];
- __le32 ageing_timeout_ms;
-} __packed;
-
struct prestera_msg_switch_attr_req {
struct prestera_msg_cmd cmd;
__le32 attr;
- union prestera_msg_switch_param param;
- u8 pad[2];
+ union {
+ __le32 ageing_timeout_ms;
+ struct {
+ u8 mac[ETH_ALEN];
+ u8 __pad[2];
+ };
+ } param;
};

struct prestera_msg_switch_init_resp {
struct prestera_msg_ret ret;
__le32 port_count;
__le32 mtu_max;
- u8 switch_id;
- u8 lag_max;
- u8 lag_member_max;
__le32 size_tbl_router_nexthop;
-} __packed __aligned(4);
+ u8 switch_id;
+ u8 lag_max;
+ u8 lag_member_max;
+};

struct prestera_msg_event_port_param {
union {
struct {
- u8 oper;
__le32 mode;
__le32 speed;
+ u8 oper;
u8 duplex;
u8 fc;
u8 fec;
- } __packed mac;
+ } mac;
struct {
- u8 mdix;
__le64 lmode_bmap;
+ u8 mdix;
u8 fc;
- } __packed phy;
- } __packed;
-} __packed __aligned(4);
+ u8 __pad[2];
+ } __packed phy; /* make sure always 12 bytes size */
+ };
+};

struct prestera_msg_port_cap_param {
__le64 link_mode;
- u8 type;
- u8 fec;
- u8 fc;
- u8 transceiver;
+ u8 type;
+ u8 fec;
+ u8 fc;
+ u8 transceiver;
};

struct prestera_msg_port_flood_param {
u8 type;
u8 enable;
+ u8 __pad[2];
};

union prestera_msg_port_param {
+ __le32 mtu;
+ __le32 speed;
+ __le32 link_mode;
u8 admin_state;
u8 oper_state;
- __le32 mtu;
u8 mac[ETH_ALEN];
u8 accept_frm_type;
- __le32 speed;
u8 learning;
u8 flood;
- __le32 link_mode;
u8 type;
u8 duplex;
u8 fec;
u8 fc;
-
union {
struct {
- u8 admin:1;
+ u8 admin;
u8 fc;
u8 ap_enable;
+ u8 __reserved[5];
union {
struct {
__le32 mode;
- u8 inband:1;
__le32 speed;
- u8 duplex;
- u8 fec;
- u8 fec_supp;
- } __packed reg_mode;
+ u8 inband;
+ u8 duplex;
+ u8 fec;
+ u8 fec_supp;
+ } reg_mode;
struct {
__le32 mode;
__le32 speed;
- u8 fec;
- u8 fec_supp;
- } __packed ap_modes[PRESTERA_AP_PORT_MAX];
- } __packed;
- } __packed mac;
+ u8 fec;
+ u8 fec_supp;
+ u8 __pad[2];
+ } ap_modes[PRESTERA_AP_PORT_MAX];
+ };
+ } mac;
struct {
- u8 admin:1;
- u8 adv_enable;
__le64 modes;
__le32 mode;
+ u8 admin;
+ u8 adv_enable;
u8 mdix;
- } __packed phy;
- } __packed link;
+ u8 __pad;
+ } phy;
+ } link;

struct prestera_msg_port_cap_param cap;
struct prestera_msg_port_flood_param flood_ext;
struct prestera_msg_event_port_param link_evt;
-} __packed;
+};

struct prestera_msg_port_attr_req {
struct prestera_msg_cmd cmd;
@@ -290,14 +294,12 @@ struct prestera_msg_port_attr_req {
__le32 port;
__le32 dev;
union prestera_msg_port_param param;
-} __packed __aligned(4);
-
+};

struct prestera_msg_port_attr_resp {
struct prestera_msg_ret ret;
union prestera_msg_port_param param;
-} __packed __aligned(4);
-
+};

struct prestera_msg_port_stats_resp {
struct prestera_msg_ret ret;
@@ -322,13 +324,13 @@ struct prestera_msg_vlan_req {
__le32 port;
__le32 dev;
__le16 vid;
- u8 is_member;
- u8 is_tagged;
+ u8 is_member;
+ u8 is_tagged;
};

struct prestera_msg_fdb_req {
struct prestera_msg_cmd cmd;
- u8 dest_type;
+ __le32 flush_mode;
union {
struct {
__le32 port;
@@ -336,11 +338,12 @@ struct prestera_msg_fdb_req {
};
__le16 lag_id;
} dest;
- u8 mac[ETH_ALEN];
__le16 vid;
- u8 dynamic;
- __le32 flush_mode;
-} __packed __aligned(4);
+ u8 dest_type;
+ u8 dynamic;
+ u8 mac[ETH_ALEN];
+ u8 __pad[2];
+};

struct prestera_msg_bridge_req {
struct prestera_msg_cmd cmd;
@@ -363,11 +366,12 @@ struct prestera_msg_acl_action {

struct prestera_msg_acl_match {
__le32 type;
+ __le32 __reserved;
union {
struct {
u8 key;
u8 mask;
- } __packed u8;
+ } u8;
struct {
__le16 key;
__le16 mask;
@@ -383,7 +387,7 @@ struct prestera_msg_acl_match {
struct {
u8 key[ETH_ALEN];
u8 mask[ETH_ALEN];
- } __packed mac;
+ } mac;
} keymask;
};

@@ -446,7 +450,8 @@ struct prestera_msg_stp_req {
__le32 port;
__le32 dev;
__le16 vid;
- u8 state;
+ u8 state;
+ u8 __pad;
};

struct prestera_msg_rxtx_req {
@@ -497,21 +502,21 @@ union prestera_msg_event_fdb_param {

struct prestera_msg_event_fdb {
struct prestera_msg_event id;
- u8 dest_type;
+ __le32 vid;
union {
__le32 port_id;
__le16 lag_id;
} dest;
- __le32 vid;
union prestera_msg_event_fdb_param param;
-} __packed __aligned(4);
+ u8 dest_type;
+};

-static inline void prestera_hw_build_tests(void)
+static void prestera_hw_build_tests(void)
{
/* check requests */
BUILD_BUG_ON(sizeof(struct prestera_msg_common_req) != 4);
BUILD_BUG_ON(sizeof(struct prestera_msg_switch_attr_req) != 16);
- BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_req) != 120);
+ BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_req) != 144);
BUILD_BUG_ON(sizeof(struct prestera_msg_port_info_req) != 8);
BUILD_BUG_ON(sizeof(struct prestera_msg_vlan_req) != 16);
BUILD_BUG_ON(sizeof(struct prestera_msg_fdb_req) != 28);
@@ -528,7 +533,7 @@ static inline void prestera_hw_build_tests(void)
/* check responses */
BUILD_BUG_ON(sizeof(struct prestera_msg_common_resp) != 8);
BUILD_BUG_ON(sizeof(struct prestera_msg_switch_init_resp) != 24);
- BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_resp) != 112);
+ BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_resp) != 136);
BUILD_BUG_ON(sizeof(struct prestera_msg_port_stats_resp) != 248);
BUILD_BUG_ON(sizeof(struct prestera_msg_port_info_resp) != 20);
BUILD_BUG_ON(sizeof(struct prestera_msg_bridge_resp) != 12);
@@ -561,9 +566,9 @@ static int __prestera_cmd_ret(struct prestera_switch *sw,
if (err)
return err;

- if (__le32_to_cpu(ret->cmd.type) != PRESTERA_CMD_TYPE_ACK)
+ if (ret->cmd.type != __cpu_to_le32(PRESTERA_CMD_TYPE_ACK))
return -EBADE;
- if (__le32_to_cpu(ret->status) != PRESTERA_CMD_ACK_OK)
+ if (ret->status != __cpu_to_le32(PRESTERA_CMD_ACK_OK))
return -EINVAL;

return 0;
--
2.7.4


2021-11-05 20:32:44

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out

On 11/5/21 9:49 AM, Volodymyr Mytnyk wrote:
> From: Volodymyr Mytnyk <[email protected]>
>
> The prestera FW v4.0 support commit has been merged
> accidentally w/o review comments addressed and waiting
> for the final patch set to be uploaded. So, fix the remaining
> comments related to structure laid out and build issues.
>
> Reported-by: kernel test robot <[email protected]>
> Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
> Signed-off-by: Volodymyr Mytnyk <[email protected]>

The patch does not apply to the mainline kernel, so I can not test it there.
It does apply to linux-next, and m68k:allmodconfig builds there with the patch
applied. However, m68k:allmodconfig also builds in -next with this patch _not_
applied, so I can not really say if it does any good or bad.
In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
still fails to build.

Guenter

2021-11-06 00:30:04

by Volodymyr Mytnyk

[permalink] [raw]
Subject: Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out

>
> > From: Volodymyr Mytnyk <[email protected]>
> >
> > The prestera FW v4.0 support commit has been merged
> > accidentally w/o review comments addressed and waiting
> > for the final patch set to be uploaded. So, fix the remaining
> > comments related to structure laid out and build issues.
> >
> > Reported-by: kernel test robot <[email protected]>
> > Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
> > Signed-off-by: Volodymyr Mytnyk <[email protected]>
>
> The patch does not apply to the mainline kernel, so I can not test it there.
> It does apply to linux-next, and m68k:allmodconfig builds there with the patch
> applied. However, m68k:allmodconfig also builds in -next with this patch _not_
> applied, so I can not really say if it does any good or bad.
> In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
> still fails to build.
>
> Guenter

Hi Guenter,

The mainline kernel doesn't have the base ("net: marvell: prestera: add firmware v4.0 support") commit yet, so the patch will not be applied.

This patch is based on net/master, so you can try the patch there.

To apply this patch to mainline, the following list of patches should be ported from net/master first:
- bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
- 236f57fe1b88 ("net: marvell: prestera: Add explicit padding")
- a46a5036e7d2 ("net: marvell: prestera: fix patchwork build problems")

Volodymyr

2021-11-06 13:47:05

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out

Hi Volodymyr,

On Fri, Nov 5, 2021 at 11:42 PM Volodymyr Mytnyk
<[email protected]> wrote:
> > > From: Volodymyr Mytnyk <[email protected]>
> > >
> > > The prestera FW v4.0 support commit has been merged
> > > accidentally w/o review comments addressed and waiting
> > > for the final patch set to be uploaded. So, fix the remaining
> > > comments related to structure laid out and build issues.
> > >
> > > Reported-by: kernel test robot <[email protected]>
> > > Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
> > > Signed-off-by: Volodymyr Mytnyk <[email protected]>
> >
> > The patch does not apply to the mainline kernel, so I can not test it there.
> > It does apply to linux-next, and m68k:allmodconfig builds there with the patch
> > applied. However, m68k:allmodconfig also builds in -next with this patch _not_
> > applied, so I can not really say if it does any good or bad.
> > In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
> > still fails to build.

> The mainline kernel doesn't have the base ("net: marvell: prestera: add firmware v4.0 support") commit yet, so the patch will not be applied.

Mainline has this broken commit as of Nov 2.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-06 18:50:31

by Volodymyr Mytnyk

[permalink] [raw]
Subject: Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out

> On Fri, Nov 5, 2021 at 11:42 PM Volodymyr Mytnyk
> <[email protected]> wrote:
> > > > From: Volodymyr Mytnyk <[email protected]>
> > > >
> > > > The prestera FW v4.0 support commit has been merged
> > > > accidentally w/o review comments addressed and waiting
> > > > for the final patch set to be uploaded. So, fix the remaining
> > > > comments related to structure laid out and build issues.
> > > >
> > > > Reported-by: kernel test robot <[email protected]>
> > > > Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
> > > > Signed-off-by: Volodymyr Mytnyk <[email protected]>
> > >
> > > The patch does not apply to the mainline kernel, so I can not test it there.
> > > It does apply to linux-next, and m68k:allmodconfig builds there with the patch
> > > applied. However, m68k:allmodconfig also builds in -next with this patch _not_
> > > applied, so I can not really say if it does any good or bad.
> > > In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
> > > still fails to build.
>
> > The mainline kernel doesn't have the base ("net: marvell: prestera: add firmware v4.0 support") commit yet, so the patch will not be applied.
>
> Mainline has this broken commit as of Nov 2.

Hi Geert,

Right, this one is it there. My fault, thx.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/prestera/prestera_hw.c?id=bb5dbf2cc64d5cfa696765944c784c0010c48ae8

So, only 236f57fe1b88 ("net: marvell: prestera: Add explicit padding") from net/master is needed to fix the build and to be able to apply this patch.

Sorry for confusion.

Volodymyr

>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds

2021-11-08 02:24:37

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <[email protected]>:

On Fri, 5 Nov 2021 18:49:24 +0200 you wrote:
> From: Volodymyr Mytnyk <[email protected]>
>
> The prestera FW v4.0 support commit has been merged
> accidentally w/o review comments addressed and waiting
> for the final patch set to be uploaded. So, fix the remaining
> comments related to structure laid out and build issues.
>
> [...]

Here is the summary with links:
- [net,v4] net: marvell: prestera: fix hw structure laid out
https://git.kernel.org/netdev/net/c/e1464db5c57e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html