netdev - Re: [net-next 13/15] net/mlx5: DR, Change SWS usage to debug fs seq

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20240129205529.GS401354@kernel.org>
Date: Mon, 29 Jan 2024 20:55:29 +0000
From: Simon Horman <horms@...nel.org>
To: Saeed Mahameed <saeed@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org,
	Tariq Toukan <tariqt@...dia.com>, Gal Pressman <gal@...dia.com>,
	Leon Romanovsky <leonro@...dia.com>,
	Hamdan Igbaria <hamdani@...dia.com>,
	Yevgeny Kliteynik <kliteyn@...dia.com>
Subject: Re: [net-next 13/15] net/mlx5: DR, Change SWS usage to debug fs
 seq_file interface

On Fri, Jan 26, 2024 at 02:36:14PM -0800, Saeed Mahameed wrote:
> From: Hamdan Igbaria <hamdani@...dia.com>
> 
> In current SWS debug dump mechanism we implement the seq_file interface,
> but we only implement the 'show' callback to dump the whole steering DB
> with a single call to this callback.
> 
> However, for large data size the seq_printf function will fail to
> allocate a buffer with the adequate capacity to hold such data.
> 
> This patch solves this problem by utilizing the seq_file interface
> mechanism in the following way:
>  - when the user triggers a dump procedure, we will allocate a list of
>    buffers that hold the whole data dump (in the start callback)
>  - using the start, next, show and stop callbacks of the seq_file
>    API we iterate through the list and dump the whole data
> 
> Signed-off-by: Hamdan Igbaria <hamdani@...dia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@...dia.com>
> Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
> ---
>  .../mellanox/mlx5/core/steering/dr_dbg.c      | 726 ++++++++++++++----
>  .../mellanox/mlx5/core/steering/dr_dbg.h      |  20 +
>  2 files changed, 615 insertions(+), 131 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c

...

> @@ -568,24 +927,41 @@ static int
>  dr_dump_domain_info_caps(struct seq_file *file, struct mlx5dr_cmd_caps *caps,
>  			 const u64 domain_id)
>  {
> +	char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];

...

Hi Saeed and Hamdan,

I am seeing some warnings which I think relate to stack usage like the above,
combined with inlining.

clang-17 W=1 build on x86_64 says:

 .../dr_dbg.c:1071:1: warning: stack frame size (2552) exceeds limit (2048) in 'dr_dump_start' [-Wframe-larger-than]
  1071 | dr_dump_start(struct seq_file *file, loff_t *pos)
       | ^
 .../dr_dbg.c:703:1: warning: stack frame size (2136) exceeds limit (2048) in 'dr_dump_matcher_rx_tx' [-Wframe-larger-than]
   703 | dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
       | ^

gcc-13 W=1 build on x86_64 says:

 .../dr_dbg.c: In function 'dr_dump_domain':
 .../dr_dbg.c:1044:1: warning: the frame size of 2096 bytes is larger than 2048 bytes [-Wframe-larger-than=]
  1044 | }
       | ^

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.h
> index def6cf853eea..13511716cdbc 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.h
> @@ -1,10 +1,30 @@
>  /* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
>  /* Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
>  
> +#define MLX5DR_DEBUG_DUMP_BUFF_SIZE (64 * 1024 * 1024)
> +#define MLX5DR_DEBUG_DUMP_BUFF_LENGTH 1024

...