netdev - Re: [PATCH net-next 05/12] net/mlx5: fs, add mlx5_fs

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <101cfa96-ee51-4a68-a11c-4f1c93f7c79c@nvidia.com>
Date: Wed, 18 Dec 2024 10:21:51 +0200
From: Moshe Shemesh <moshe@...dia.com>
To: Kees Cook <kees@...nel.org>, <morbo@...gle.com>, <qing.zhao@...cle.com>
CC: Simon Horman <horms@...nel.org>, Tariq Toukan <tariqt@...dia.com>, "David
 S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, "Paolo
 Abeni" <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, Leon Romanovsky <leonro@...dia.com>,
	<netdev@...r.kernel.org>, Saeed Mahameed <saeedm@...dia.com>, Gal Pressman
	<gal@...dia.com>, <linux-rdma@...r.kernel.org>, Yevgeny Kliteynik
	<kliteyn@...dia.com>, Mark Bloch <mbloch@...dia.com>,
	<linux-hardening@...r.kernel.org>
Subject: Re: [PATCH net-next 05/12] net/mlx5: fs, add mlx5_fs_pool API



On 12/18/2024 7:22 AM, Kees Cook wrote:
> 
> On Sun, Dec 15, 2024 at 03:39:11PM +0200, Moshe Shemesh wrote:
>>
>>
>> On 12/12/2024 7:23 PM, Simon Horman wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Wed, Dec 11, 2024 at 03:42:16PM +0200, Tariq Toukan wrote:
>>>> From: Moshe Shemesh <moshe@...dia.com>
>>>>
>>>> Refactor fc_pool API to create generic fs_pool API, as HW steering has
>>>> more flow steering elements which can take advantage of the same pool of
>>>> bulks API. Change fs_counters code to use the fs_pool API.
>>>>
>>>> Signed-off-by: Moshe Shemesh <moshe@...dia.com>
>>>> Reviewed-by: Yevgeny Kliteynik <kliteyn@...dia.com>
>>>> Reviewed-by: Mark Bloch <mbloch@...dia.com>
>>>> Signed-off-by: Tariq Toukan <tariqt@...dia.com>
>>>
>>> ...
>>>
>>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c
>>>
>>> ...
>>>
>>>> @@ -447,11 +437,9 @@ void mlx5_fc_update_sampling_interval(struct mlx5_core_dev *dev,
>>>>    /* Flow counter bluks */
>>>>
>>>>    struct mlx5_fc_bulk {
>>>> -     struct list_head pool_list;
>>>> +     struct mlx5_fs_bulk fs_bulk;
>>>>         u32 base_id;
>>>> -     int bulk_len;
>>>> -     unsigned long *bitmask;
>>>> -     struct mlx5_fc fcs[] __counted_by(bulk_len);
>>>> +     struct mlx5_fc fcs[] __counted_by(fs_bulk.bulk_len);
>>>>    };
>>>
>>> Unfortunately it seems that clang-19 doesn't know how to handle
>>> __counted_by() when used like this:
>>>
>>> drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c:442:36: error: 'counted_by' argument must be a simple declaration reference
>>>     442 |         struct mlx5_fc fcs[] __counted_by(fs_bulk.bulk_len);
>>
>> Thanks Simon, from code perspective, bulk_len should be now in the inner
>> struct fs_bulk.
>>
>> Keen Cook, is that going to be supported in the future? for now I will just
>> remove __counted_by() from this struct.
> 
> I am expecting this will be supported in the future, yes, but there isn't
> an ETA for it yet. Neither GCC 15 nor Clang are currently supporting
> sub-struct members -- the counted_by member needs to be at the same
> "level" in the struct as the annotated flexible array.
> 
> Is it possible to move "base_id" above "fs_bulk" and move "fcs" into
> the of end struct mlx5_fs_bulk?
> 

fcs is specific for mlx5_fc_bulk and so can't be part of fs_bulk.
In follow up patches for hws feature I am using mlx5_fs_bulk in other 
type of bulks, fcs is specific for mlx5_fc_bulk.
Unfortunately, I don't see a nice way to have bulk_len and fcs on the 
same struct.

> -Kees
> 
> --
> Kees Cook