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

[Michal Swiatkowski]

On Tue, Feb 04, 2025 at 01:27:41PM +1030, 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 order to avoid ending up with a flexible-array member in the
> middle of other structs, we use the `struct_group_tagged()` helper
> to create a new tagged `struct mlx5e_umr_wqe_hdr`. This structure
> groups together all the members of the flexible `struct mlx5e_umr_wqe`
> except the flexible array.
> 
> As a result, the array is effectively separated from the rest of the
> members without modifying the memory layout of the flexible structure.
> We then change the type of the middle struct member currently causing
> trouble from `struct mlx5e_umr_wqe` to `struct mlx5e_umr_wqe_hdr`.
> 
> We also want to ensure that when new members need to be added to the
> flexible structure, they are always included within the newly created
> tagged struct. For this, we use `static_assert()`. This ensures that the
> memory layout for both the flexible structure and the new tagged struct
> is the same after any changes.
> 
> This approach avoids having to implement `struct mlx5e_umr_wqe_hdr` as
> a completely separate structure, thus preventing having to maintain two
> independent but basically identical structures, closing the door to
> potential bugs in the future.
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure, through which we can access the flexible-array
> member, if necessary.
> 
> So, with these changes, fix 124 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@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h      | 13 +++++++++----
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c |  4 +++-
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 979fc56205e1..c30c64eb346f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -233,15 +233,20 @@ struct mlx5e_rx_wqe_cyc {
>  };
>  
>  struct mlx5e_umr_wqe {
> -	struct mlx5_wqe_ctrl_seg       ctrl;
> -	struct mlx5_wqe_umr_ctrl_seg   uctrl;
> -	struct mlx5_mkey_seg           mkc;
> +	/* New members MUST be added within the struct_group() macro below. */
> +	struct_group_tagged(mlx5e_umr_wqe_hdr, hdr,
> +		struct mlx5_wqe_ctrl_seg       ctrl;
> +		struct mlx5_wqe_umr_ctrl_seg   uctrl;
> +		struct mlx5_mkey_seg           mkc;
> +	);
>  	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 member likely outside of struct_group_tagged()");
>  
>  enum mlx5e_priv_flag {
>  	MLX5E_PFLAG_RX_CQE_BASED_MODER,
> @@ -660,7 +665,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;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index bd41b75d246e..4ff4ff2342cf 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -373,6 +373,8 @@ static void mlx5e_rq_shampo_hd_info_free(struct mlx5e_rq *rq)
>  
>  static int mlx5e_rq_alloc_mpwqe_info(struct mlx5e_rq *rq, int node)
>  {
> +	struct mlx5e_umr_wqe *umr_wqe =
> +		container_of(&rq->mpwqe.umr_wqe, struct mlx5e_umr_wqe, hdr);
>  	int wq_sz = mlx5_wq_ll_get_size(&rq->mpwqe.wq);
>  	size_t alloc_size;
>  
> @@ -393,7 +395,7 @@ 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, umr_wqe);
>  
>  	return 0;

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@xxxxxxxxxxxxxxx>

>  }
> -- 
> 2.43.0