net/mlx5e: Avoid a hundred -Wflex-array-member-not-at-end warnings

PATCH Applied Gustavo A. R. Silva 5 participants 8 messages Feb 26, 2025 – Feb 28, 2025

Subsystems (5)

Networking Network drivers InfiniBand Mellanox mlx5 core +1 more subsystems

lore

Share & tools

lore

Gustavo A. R. Silva

author

2025-02-26 03:17 UTC #0 lore raw

PATCH v3 1/1 net/mlx5e: Avoid a hundred -Wflex-array-member-not-at-end warnings

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

So, in this particular case, we create a new struct mlx5e_umr_wqe_hdr to enclose the header part of flexible structure struct mlx5e_umr_wqe. This is, all the members except the flexible arrays inline_mtts, inline_klms and inline_ksms in the anonymous union. We then replace the header part with struct mlx5e_umr_wqe_hdr hdr; in struct mlx5e_umr_wqe, and change the type of the object currently causing trouble umr_wqe from struct mlx5e_umr_wqe to struct mlx5e_umr_wqe_hdr --this last bit gets rid of the flex-array-in-the-middle part and avoid the warnings.

Also, no new members should be added to struct mlx5e_umr_wqe, instead any new members must be included in the header structure struct mlx5e_umr_wqe_hdr. To enforce this, we use static_assert(), ensuring that the memory layout of both the flexible structure and the newly created header struct remain consistent.

The next step is to refactor the rest of the related code accordingly, which means adding a bunch of hdr. wherever needed.

Lastly, we use container_of() whenever we need to retrieve a pointer to the flexible structure struct mlx5e_umr_wqe.

So, with these changes, fix 125 of the following warnings:

drivers/net/ethernet/mellanox/mlx5/core/en.h:664:48: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v3:
- Update error message in the assertion.
- Also, this part is intentionally left as-is (to keep the assertion as close as possible to the flex struct):

| CHECK: Please use a blank line after function/struct/union/enum declarations
| #63: FILE: drivers/net/ethernet/mellanox/mlx5/core/en.h:249:
| };
| +static_assert(offsetof(struct mlx5e_umr_wqe, inline_mtts) == sizeof(struct mlx5e_umr_wqe_hdr),

Changes in v2:
- Split the header members of struct mlx5e_umr_wqe into a separate struct mlx5e_umr_wqe_hdr, and refactor the code accordingly. (Jakub)
- Update the changelog text.
- Link: https://lore.kernel.org/linux-hardening/Z76FE8oZO2Ssuj9T@kspp/

v1:
- Link: https://lore.kernel.org/linux-hardening/Z6GCJY8G9EzASrwQ@kspp/

 drivers/net/ethernet/mellanox/mlx5/core/en.h  | 10 +++++++--
 .../ethernet/mellanox/mlx5/core/en/xsk/rx.c   |  6 ++---
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  8 ++++---
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 22 +++++++++----------
 4 files changed, 27 insertions(+), 19 deletions(-)

drivers/net/ethernet/mellanox/mlx5/core/en.h +8 -2

index 979fc56205e1..90de40521029 100644
@@ -232,16 +232,22 @@ struct mlx5e_rx_wqe_cyc {
 	DECLARE_FLEX_ARRAY(struct mlx5_wqe_data_seg, data);
 };
 
-struct mlx5e_umr_wqe {
+struct mlx5e_umr_wqe_hdr {
 	struct mlx5_wqe_ctrl_seg       ctrl;
 	struct mlx5_wqe_umr_ctrl_seg   uctrl;
 	struct mlx5_mkey_seg           mkc;
+};
+
+struct mlx5e_umr_wqe {
+	struct mlx5e_umr_wqe_hdr hdr;
 	union {
 		DECLARE_FLEX_ARRAY(struct mlx5_mtt, inline_mtts);
 		DECLARE_FLEX_ARRAY(struct mlx5_klm, inline_klms);
 		DECLARE_FLEX_ARRAY(struct mlx5_ksm, inline_ksms);
 	};
 };
+static_assert(offsetof(struct mlx5e_umr_wqe, inline_mtts) == sizeof(struct mlx5e_umr_wqe_hdr),
+	      "struct members should be included in struct mlx5e_umr_wqe_hdr, not in struct mlx5e_umr_wqe");
 
 enum mlx5e_priv_flag {
 	MLX5E_PFLAG_RX_CQE_BASED_MODER,

@@ -660,7 +666,7 @@ struct mlx5e_rq {
 		} wqe;
 		struct {
 			struct mlx5_wq_ll      wq;
-			struct mlx5e_umr_wqe   umr_wqe;
+			struct mlx5e_umr_wqe_hdr umr_wqe;
 			struct mlx5e_mpw_info *info;
 			mlx5e_fp_skb_from_cqe_mpwrq skb_from_cqe_mpwrq;
 			__be32                 umr_mkey_be;

drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c +3 -3

index 1b7132fa70de..2b05536d564a 100644
@@ -123,7 +123,7 @@ int mlx5e_xsk_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 	bitmap_zero(wi->skip_release_bitmap, rq->mpwqe.pages_per_wqe);
 	wi->consumed_strides = 0;
 
-	umr_wqe->ctrl.opmod_idx_opcode =
+	umr_wqe->hdr.ctrl.opmod_idx_opcode =
 		cpu_to_be32((icosq->pc << MLX5_WQE_CTRL_WQE_INDEX_SHIFT) | MLX5_OPCODE_UMR);
 
 	/* Optimized for speed: keep in sync with mlx5e_mpwrq_umr_entry_size. */

@@ -134,7 +134,7 @@ int mlx5e_xsk_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 		offset = offset * sizeof(struct mlx5_klm) * 2 / MLX5_OCTWORD;
 	else if (unlikely(rq->mpwqe.umr_mode == MLX5E_MPWRQ_UMR_MODE_TRIPLE))
 		offset = offset * sizeof(struct mlx5_ksm) * 4 / MLX5_OCTWORD;
-	umr_wqe->uctrl.xlt_offset = cpu_to_be16(offset);
+	umr_wqe->hdr.uctrl.xlt_offset = cpu_to_be16(offset);
 
 	icosq->db.wqe_info[pi] = (struct mlx5e_icosq_wqe_info) {
 		.wqe_type = MLX5E_ICOSQ_WQE_UMR_RX,

@@ -144,7 +144,7 @@ int mlx5e_xsk_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 
 	icosq->pc += rq->mpwqe.umr_wqebbs;
 
-	icosq->doorbell_cseg = &umr_wqe->ctrl;
+	icosq->doorbell_cseg = &umr_wqe->hdr.ctrl;
 
 	return 0;

drivers/net/ethernet/mellanox/mlx5/core/en_main.c +5 -3

index 2fdc86432ac0..9abc6ce13ac9 100644
@@ -311,8 +311,8 @@ static inline void mlx5e_build_umr_wqe(struct mlx5e_rq *rq,
 				       struct mlx5e_icosq *sq,
 				       struct mlx5e_umr_wqe *wqe)
 {
-	struct mlx5_wqe_ctrl_seg      *cseg = &wqe->ctrl;
-	struct mlx5_wqe_umr_ctrl_seg *ucseg = &wqe->uctrl;
+	struct mlx5_wqe_ctrl_seg      *cseg = &wqe->hdr.ctrl;
+	struct mlx5_wqe_umr_ctrl_seg *ucseg = &wqe->hdr.uctrl;
 	u16 octowords;
 	u8 ds_cnt;

@@ -393,7 +393,9 @@ static int mlx5e_rq_alloc_mpwqe_info(struct mlx5e_rq *rq, int node)
 		bitmap_fill(wi->skip_release_bitmap, rq->mpwqe.pages_per_wqe);
 	}
 
-	mlx5e_build_umr_wqe(rq, rq->icosq, &rq->mpwqe.umr_wqe);
+	mlx5e_build_umr_wqe(rq, rq->icosq,
+			    container_of(&rq->mpwqe.umr_wqe,
+					 struct mlx5e_umr_wqe, hdr));
 
 	return 0;
 }

drivers/net/ethernet/mellanox/mlx5/core/en_rx.c +11 ~~-11~~

index 1963bc5adb18..5fd70b4d55be 100644
@@ -631,16 +631,16 @@ static void build_ksm_umr(struct mlx5e_icosq *sq, struct mlx5e_umr_wqe *umr_wqe,
 			  __be32 key, u16 offset, u16 ksm_len)
 {
 	memset(umr_wqe, 0, offsetof(struct mlx5e_umr_wqe, inline_ksms));
-	umr_wqe->ctrl.opmod_idx_opcode =
+	umr_wqe->hdr.ctrl.opmod_idx_opcode =
 		cpu_to_be32((sq->pc << MLX5_WQE_CTRL_WQE_INDEX_SHIFT) |
 			     MLX5_OPCODE_UMR);
-	umr_wqe->ctrl.umr_mkey = key;
-	umr_wqe->ctrl.qpn_ds = cpu_to_be32((sq->sqn << MLX5_WQE_CTRL_QPN_SHIFT)
+	umr_wqe->hdr.ctrl.umr_mkey = key;
+	umr_wqe->hdr.ctrl.qpn_ds = cpu_to_be32((sq->sqn << MLX5_WQE_CTRL_QPN_SHIFT)
 					    | MLX5E_KSM_UMR_DS_CNT(ksm_len));
-	umr_wqe->uctrl.flags = MLX5_UMR_TRANSLATION_OFFSET_EN | MLX5_UMR_INLINE;
-	umr_wqe->uctrl.xlt_offset = cpu_to_be16(offset);
-	umr_wqe->uctrl.xlt_octowords = cpu_to_be16(ksm_len);
-	umr_wqe->uctrl.mkey_mask     = cpu_to_be64(MLX5_MKEY_MASK_FREE);
+	umr_wqe->hdr.uctrl.flags = MLX5_UMR_TRANSLATION_OFFSET_EN | MLX5_UMR_INLINE;
+	umr_wqe->hdr.uctrl.xlt_offset = cpu_to_be16(offset);
+	umr_wqe->hdr.uctrl.xlt_octowords = cpu_to_be16(ksm_len);
+	umr_wqe->hdr.uctrl.mkey_mask     = cpu_to_be64(MLX5_MKEY_MASK_FREE);
 }
 
 static struct mlx5e_frag_page *mlx5e_shampo_hd_to_frag_page(struct mlx5e_rq *rq, int header_index)

@@ -704,7 +704,7 @@ static int mlx5e_build_shampo_hd_umr(struct mlx5e_rq *rq,
 
 	shampo->pi = (shampo->pi + ksm_entries) & (shampo->hd_per_wq - 1);
 	sq->pc += wqe_bbs;
-	sq->doorbell_cseg = &umr_wqe->ctrl;
+	sq->doorbell_cseg = &umr_wqe->hdr.ctrl;
 
 	return 0;

@@ -814,12 +814,12 @@ static int mlx5e_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 	bitmap_zero(wi->skip_release_bitmap, rq->mpwqe.pages_per_wqe);
 	wi->consumed_strides = 0;
 
-	umr_wqe->ctrl.opmod_idx_opcode =
+	umr_wqe->hdr.ctrl.opmod_idx_opcode =
 		cpu_to_be32((sq->pc << MLX5_WQE_CTRL_WQE_INDEX_SHIFT) |
 			    MLX5_OPCODE_UMR);
 
 	offset = (ix * rq->mpwqe.mtts_per_wqe) * sizeof(struct mlx5_mtt) / MLX5_OCTWORD;
-	umr_wqe->uctrl.xlt_offset = cpu_to_be16(offset);
+	umr_wqe->hdr.uctrl.xlt_offset = cpu_to_be16(offset);
 
 	sq->db.wqe_info[pi] = (struct mlx5e_icosq_wqe_info) {
 		.wqe_type   = MLX5E_ICOSQ_WQE_UMR_RX,

@@ -829,7 +829,7 @@ static int mlx5e_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 
 	sq->pc += rq->mpwqe.umr_wqebbs;
 
-	sq->doorbell_cseg = &umr_wqe->ctrl;
+	sq->doorbell_cseg = &umr_wqe->hdr.ctrl;
 
 	return 0;

Signature · 1 line

2.43.0

Saeed Mahameed

2025-02-26 18:49 UTC #1 lore raw

Quoted 64 lines · Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.
So, in this particular case, we create a new struct mlx5e_umr_wqe_hdr to enclose the header part of flexible structure struct mlx5e_umr_wqe. This is, all the members except the flexible arrays inline_mtts, inline_klms and inline_ksms in the anonymous union. We then replace the header part with struct mlx5e_umr_wqe_hdr hdr; in struct mlx5e_umr_wqe, and change the type of the object currently causing trouble umr_wqe from struct mlx5e_umr_wqe to struct mlx5e_umr_wqe_hdr --this last bit gets rid of the flex-array-in-the-middle part and avoid the warnings.
Also, no new members should be added to struct mlx5e_umr_wqe, instead any new members must be included in the header structure struct mlx5e_umr_wqe_hdr. To enforce this, we use static_assert(), ensuring that the memory layout of both the flexible structure and the newly created header struct remain consistent.
The next step is to refactor the rest of the related code accordingly, which means adding a bunch of hdr. wherever needed.
Lastly, we use container_of() whenever we need to retrieve a pointer to the flexible structure struct mlx5e_umr_wqe.
So, with these changes, fix 125 of the following warnings:
drivers/net/ethernet/mellanox/mlx5/core/en.h:664:48: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
Changes in v3:
- Update error message in the assertion.
- Also, this part is intentionally left as-is (to keep the assertion as close as possible to the flex struct):
| CHECK: Please use a blank line after function/struct/union/enum declarations
| #63: FILE: drivers/net/ethernet/mellanox/mlx5/core/en.h:249:
| };
| +static_assert(offsetof(struct mlx5e_umr_wqe, inline_mtts) == sizeof(struct mlx5e_umr_wqe_hdr),
Changes in v2:
- Split the header members of struct mlx5e_umr_wqe into a separate struct mlx5e_umr_wqe_hdr, and refactor the code accordingly. (Jakub)
- Update the changelog text.
- Link: https://lore.kernel.org/linux-hardening/Z76FE8oZO2Ssuj9T@kspp/
v1:
- Link: https://lore.kernel.org/linux-hardening/Z6GCJY8G9EzASrwQ@kspp/
drivers/net/ethernet/mellanox/mlx5/core/en.h  | 10 +++++++--
.../ethernet/mellanox/mlx5/core/en/xsk/rx.c   |  6 ++---
.../net/ethernet/mellanox/mlx5/core/en_main.c |  8 ++++---
.../net/ethernet/mellanox/mlx5/core/en_rx.c   | 22 +++++++++----------
4 files changed, 27 insertions(+), 19 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en.h +0 -0
index 979fc56205e1..90de40521029 100644
@@ -232,16 +232,22 @@ struct mlx5e_rx_wqe_cyc {
 	DECLARE_FLEX_ARRAY(struct mlx5_wqe_data_seg, data);
 };

drivers/net/ethernet/mellanox/mlx5/core/en.h struct mlx5e_rx_wqe_cyc { · line 242

Quoted 10 lines

drivers/net/ethernet/mellanox/mlx5/core/en.h +5 -1

 
-struct mlx5e_umr_wqe {
+struct mlx5e_umr_wqe_hdr {
 	struct mlx5_wqe_ctrl_seg       ctrl;
 	struct mlx5_wqe_umr_ctrl_seg   uctrl;
 	struct mlx5_mkey_seg           mkc;
+};
+
+struct mlx5e_umr_wqe {
+	struct mlx5e_umr_wqe_hdr hdr;

You missed or ignored my comment on v0, anyway:

Can we have struct mlx5e_umr_wq_hdr defined anonymously within mlx5e_umr_wqe? Let's avoid namespace pollution.

Jakub Kicinski

maintainer

2025-02-27 01:25 UTC #2 lore raw

Quoted 13 lines · On Wed, 26 Feb 2025 10:49:35 -0800 Saeed Mahameed wrote:

drivers/net/ethernet/mellanox/mlx5/core/en.h struct mlx5e_rx_wqe_cyc { · line 242
Quoted 9 lines · Gustavo A. R. Silva
drivers/net/ethernet/mellanox/mlx5/core/en.h +1 -1
-struct mlx5e_umr_wqe {
+struct mlx5e_umr_wqe_hdr {
	struct mlx5_wqe_ctrl_seg       ctrl;
	struct mlx5_wqe_umr_ctrl_seg   uctrl;
	struct mlx5_mkey_seg           mkc;
drivers/net/ethernet/mellanox/mlx5/core/en.h +4 -0
+};
+
+struct mlx5e_umr_wqe {
+	struct mlx5e_umr_wqe_hdr hdr;  
You missed or ignored my comment on v0, anyway:

Quoted 2 lines

Can we have struct mlx5e_umr_wq_hdr defined anonymously within mlx5e_umr_wqe? Let's avoid namespace pollution.

It's also used in struct mlx5e_rq, I don't think it can be anonymous?

Saeed Mahameed

2025-02-27 22:27 UTC #3 lore raw

Quoted 14 lines · Jakub Kicinski

Quoted 13 lines · On Wed, 26 Feb 2025 10:49:35 -0800 Saeed Mahameed wrote:
drivers/net/ethernet/mellanox/mlx5/core/en.h struct mlx5e_rx_wqe_cyc { · line 242
Quoted 9 lines · Gustavo A. R. Silva
drivers/net/ethernet/mellanox/mlx5/core/en.h +1 -1
-struct mlx5e_umr_wqe {
+struct mlx5e_umr_wqe_hdr {
	struct mlx5_wqe_ctrl_seg       ctrl;
	struct mlx5_wqe_umr_ctrl_seg   uctrl;
	struct mlx5_mkey_seg           mkc;
drivers/net/ethernet/mellanox/mlx5/core/en.h +4 -0
+};
+
+struct mlx5e_umr_wqe {
+	struct mlx5e_umr_wqe_hdr hdr;
You missed or ignored my comment on v0, anyway:

Quoted 4 lines

Quoted 2 lines
Can we have struct mlx5e_umr_wq_hdr defined anonymously within mlx5e_umr_wqe? Let's avoid namespace pollution.
It's also used in struct mlx5e_rq, I don't think it can be anonymous?

Yep, I see now, Thanks!.

Gustavo A. R. Silva

2025-02-27 01:36 UTC #4 lore raw

In reply to Saeed Mahameed

Quoted 14 lines

drivers/net/ethernet/mellanox/mlx5/core/en.h struct mlx5e_rx_wqe_cyc { · line 242
Quoted 11 lines
drivers/net/ethernet/mellanox/mlx5/core/en.h +1 -1
-struct mlx5e_umr_wqe {
+struct mlx5e_umr_wqe_hdr {
    struct mlx5_wqe_ctrl_seg       ctrl;
    struct mlx5_wqe_umr_ctrl_seg   uctrl;
    struct mlx5_mkey_seg           mkc;
drivers/net/ethernet/mellanox/mlx5/core/en.h +4 -0
+};
+
+struct mlx5e_umr_wqe {
+    struct mlx5e_umr_wqe_hdr hdr;
You missed or ignored my comment on v0, anyway:

Quoted 2 lines

Can we have struct mlx5e_umr_wq_hdr defined anonymously within mlx5e_umr_wqe? Let's avoid namespace pollution.

I thought your comment was directed to Jabuk.

I don't see how to avoid that and at the same time changing the type of the conflicting object and fix the warnings:

- struct mlx5e_umr_wqe umr_wqe;
+ struct mlx5e_umr_wqe_hdr umr_wqe;

My first patch avoids the need to introduce a bunch of hdr. changes. However, hdr is introduced as an identifier for the members grouped in the new type struct mlx5e_umr_wqe_hdr.

Of course struct_group_tagged() also creates an anonymous struct, which is why we can avoid all those hdr. changes in v1.

--
Gustavo

Saeed Mahameed

Reviewed-by

2025-02-27 22:29 UTC #5 lore raw

Quoted 25 lines · Gustavo A. R. Silva

Quoted 14 lines
drivers/net/ethernet/mellanox/mlx5/core/en.h struct mlx5e_rx_wqe_cyc { · line 242
Quoted 11 lines
drivers/net/ethernet/mellanox/mlx5/core/en.h +1 -1
-struct mlx5e_umr_wqe {
+struct mlx5e_umr_wqe_hdr {
    struct mlx5_wqe_ctrl_seg       ctrl;
    struct mlx5_wqe_umr_ctrl_seg   uctrl;
    struct mlx5_mkey_seg           mkc;
drivers/net/ethernet/mellanox/mlx5/core/en.h +4 -0
+};
+
+struct mlx5e_umr_wqe {
+    struct mlx5e_umr_wqe_hdr hdr;
You missed or ignored my comment on v0, anyway:
Quoted 2 lines
Can we have struct mlx5e_umr_wq_hdr defined anonymously within mlx5e_umr_wqe? Let's avoid namespace pollution.
I thought your comment was directed to Jabuk.
I don't see how to avoid that and at the same time changing the type of the conflicting object and fix the warnings:
drivers/net/ethernet/mellanox/mlx5/core/en.h +1 -1
-			struct mlx5e_umr_wqe   umr_wqe;
+			struct mlx5e_umr_wqe_hdr umr_wqe;

Quoted 7 lines

My first patch avoids the need to introduce a bunch of hdr. changes. However, hdr is introduced as an identifier for the members grouped in the new type struct mlx5e_umr_wqe_hdr.
Of course struct_group_tagged() also creates an anonymous struct, which is why we can avoid all those hdr. changes in v1.

I missed the fact that it was used part of another struct that needed only the header.

This patch is alright, no need to change anything.

Reviewed-by Saeed Mahameed <[email protected]>

Quoted 2 lines

--
Gustavo

[email protected]

CI Applied

2025-02-28 01:28 UTC #6 lore raw

In reply to Gustavo A. R. Silva

This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 13 lines · On Wed, 26 Feb 2025 13:47:32 +1030 you wrote:

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.
So, in this particular case, we create a new struct mlx5e_umr_wqe_hdr to enclose the header part of flexible structure struct mlx5e_umr_wqe. This is, all the members except the flexible arrays inline_mtts, inline_klms and inline_ksms in the anonymous union. We then replace the header part with struct mlx5e_umr_wqe_hdr hdr; in struct mlx5e_umr_wqe, and change the type of the object currently causing trouble umr_wqe from struct mlx5e_umr_wqe to struct mlx5e_umr_wqe_hdr --this last bit gets rid of the flex-array-in-the-middle part and avoid the warnings.

Quoted 1 line

[...]

Here is the summary with links:
- [v3,next] net/mlx5e: Avoid a hundred -Wflex-array-member-not-at-end warnings

    https://git.kernel.org/netdev/net-next/c/bf08fd32cc55

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Gustavo A. R. Silva

2025-02-28 01:38 UTC #7 lore raw

Quoted 1 line

This patch was applied to netdev/net-next.git (main)

Awesome. :)

Thank you, guys.
--
Gustavo