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

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

Hi,

On 04/02/25 13:27, 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@kernel.org>
> ---
>   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;
>   }

Here is another alternative for this.  And similarly to the struct_group_tagged()
change above, no struct members should be added before or after `struct
mlx5e_umr_wqe_hdr hdr;` in `struct mlx5e_umr_wqe`:

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 979fc56205e1..912b97eeb4d6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -232,10 +232,13 @@ 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);
@@ -660,7 +663,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/xsk/rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
index 1b7132fa70de..2b05536d564a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
@@ -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;

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index a814b63ed97e..bbd0b888d237 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -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;
  }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 1963bc5adb18..5fd70b4d55be 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -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;


Thanks
--
Gustavo