Re: [PATCH v3][next] net/mlx5e: Avoid a hundred -Wflex-array-member-not-at-end warnings

[Saeed Mahameed <saeed@kernel.org>]

On 26 Feb 13:47, Gustavo A. R. Silva 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.
>
>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 <gustavoars@kernel.org>
>---
>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(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>index 979fc56205e1..90de40521029 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>@@ -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;

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.