| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7ce8d318-584f-42c2-b88a-2597acd67029@embeddedor.com>
Date: Tue, 18 Feb 2025 18:44:48 +1030
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Tariq Toukan <ttoukan.linux@...il.com>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>,
Saeed Mahameed <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>,
Tariq Toukan <tariqt@...dia.com>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] net/mlx5e: Avoid a hundred
-Wflex-array-member-not-at-end warnings
Hi all,
Friendly ping: who can take this, please?
Thanks
--
Gustavo
On 06/02/25 17:55, Tariq Toukan wrote:
>
>
> On 06/02/2025 6:37, Gustavo A. R. Silva wrote:
>> 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@...nel.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`:
>>
>
> Thanks for your patch.
>
> The change with the struct_group_tagged() uses advanced tag, and keeps code cleaner.
> I prefer it.
>
> Reviewed-by: Tariq Toukan <tariqt@...dia.com>
>
> Regards,
> Tariq
>
>> 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
>>
>
>
Powered by blists - more mailing lists