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

Subsystems: marvell prestera ethernet switch driver, networking drivers, the rest

STALE1735d

6 messages, 4 authors, 2021-11-07 · open the first message on its own page

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

From: Volodymyr Mytnyk <hidden>
Date: 2021-11-05 16:50:11

From: Volodymyr Mytnyk <redacted>

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 <redacted>
Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
Signed-off-by: Volodymyr Mytnyk <redacted>
---

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

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

From: Guenter Roeck <linux@roeck-us.net>
Date: 2021-11-05 20:15:17

On 11/5/21 9:49 AM, Volodymyr Mytnyk wrote:
From: Volodymyr Mytnyk <redacted>

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 <redacted>
Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
Signed-off-by: Volodymyr Mytnyk <redacted>
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

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

From: Volodymyr Mytnyk <hidden>
Date: 2021-11-05 22:42:44

quoted
From: Volodymyr Mytnyk <redacted>

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 <redacted>
Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
Signed-off-by: Volodymyr Mytnyk <redacted>
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

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

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-11-06 08:50:14

Hi Volodymyr,

On Fri, Nov 5, 2021 at 11:42 PM Volodymyr Mytnyk
[off-list ref] wrote:
quoted
quoted
From: Volodymyr Mytnyk <redacted>

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 <redacted>
Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
Signed-off-by: Volodymyr Mytnyk <redacted>
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 -- geert@linux-m68k.org

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

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

From: Volodymyr Mytnyk <hidden>
Date: 2021-11-06 10:54:19

On Fri, Nov 5, 2021 at 11:42 PM Volodymyr Mytnyk
[off-list ref] wrote:
quoted
quoted
quoted
From: Volodymyr Mytnyk <redacted>

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 <redacted>
Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
Signed-off-by: Volodymyr Mytnyk <redacted>
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.
quoted
        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 -- geert@linux-m68k.org

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

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

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-11-07 19:40:12

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller [off-list ref]:

On Fri,  5 Nov 2021 18:49:24 +0200 you wrote:
From: Volodymyr Mytnyk <redacted>

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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help