netdev - Re: [net-next 07/15] net/mlx5: SD, Add informative prints in kernel log

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: <Zbdt1vy9WLRqMqGK@nanopsycho>
Date: Mon, 29 Jan 2024 10:20:22 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Tariq Toukan <ttoukan.linux@...il.com>
Cc: Saeed Mahameed <saeed@...nel.org>,
	"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>
Subject: Re: [net-next 07/15] net/mlx5: SD, Add informative prints in kernel
 log

Thu, Jan 25, 2024 at 08:42:41AM CET, ttoukan.linux@...il.com wrote:
>
>
>On 05/01/2024 14:12, Jiri Pirko wrote:
>> Thu, Dec 21, 2023 at 01:57:13AM CET, saeed@...nel.org wrote:
>> > From: Tariq Toukan <tariqt@...dia.com>
>> > 
>> > Print to kernel log when an SD group moves from/to ready state.
>> > 
>> > Signed-off-by: Tariq Toukan <tariqt@...dia.com>
>> > Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
>> > ---
>> > .../net/ethernet/mellanox/mlx5/core/lib/sd.c  | 21 +++++++++++++++++++
>> > 1 file changed, 21 insertions(+)
>> > 
>> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
>> > index 3309f21d892e..f68942277c62 100644
>> > --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
>> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
>> > @@ -373,6 +373,21 @@ static void sd_cmd_unset_secondary(struct mlx5_core_dev *secondary)
>> > 	mlx5_fs_cmd_set_l2table_entry_silent(secondary, 0);
>> > }
>> > 
>> > +static void sd_print_group(struct mlx5_core_dev *primary)
>> > +{
>> > +	struct mlx5_sd *sd = mlx5_get_sd(primary);
>> > +	struct mlx5_core_dev *pos;
>> > +	int i;
>> > +
>> > +	sd_info(primary, "group id %#x, primary %s, vhca %u\n",
>> > +		sd->group_id, pci_name(primary->pdev),
>> > +		MLX5_CAP_GEN(primary, vhca_id));
>> > +	mlx5_sd_for_each_secondary(i, primary, pos)
>> > +		sd_info(primary, "group id %#x, secondary#%d %s, vhca %u\n",
>> > +			sd->group_id, i - 1, pci_name(pos->pdev),
>> > +			MLX5_CAP_GEN(pos, vhca_id));
>> > +}
>> > +
>> > int mlx5_sd_init(struct mlx5_core_dev *dev)
>> > {
>> > 	struct mlx5_core_dev *primary, *pos, *to;
>> > @@ -410,6 +425,10 @@ int mlx5_sd_init(struct mlx5_core_dev *dev)
>> > 			goto err_unset_secondaries;
>> > 	}
>> > 
>> > +	sd_info(primary, "group id %#x, size %d, combined\n",
>> > +		sd->group_id, mlx5_devcom_comp_get_size(sd->devcom));
>> 
>> Can't you rather expose this over sysfs or debugfs? I mean, dmesg print
>> does not seem like a good idea.
>> 
>> 
>
>I think that the events of netdev combine/uncombine are important enough to
>be logged in the kernel dmesg.

Why? I believe that the best amount od dmesg log is exactly 0. You
should find proper interfaces. Definitelly for new features. Why do you
keep asking user to look for random messages in dmesg? Does not make any
sense :/



>I can implement a debugfs as an addition, not replacing the print.
>
>> > +	sd_print_group(primary);
>> > +
>> > 	return 0;
>> > 
>> > err_unset_secondaries:
>> > @@ -440,6 +459,8 @@ void mlx5_sd_cleanup(struct mlx5_core_dev *dev)
>> > 	mlx5_sd_for_each_secondary(i, primary, pos)
>> > 		sd_cmd_unset_secondary(pos);
>> > 	sd_cmd_unset_primary(primary);
>> > +
>> > +	sd_info(primary, "group id %#x, uncombined\n", sd->group_id);
>> > out:
>> > 	sd_unregister(dev);
>> > 	sd_cleanup(dev);
>> > -- 
>> > 2.43.0
>> > 
>> >