diff mbox series

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

Message ID 20231221005721.186607-8-saeed@kernel.org (mailing list archive)
State Accepted
Commit c82d360325112ccc512fc11a3b68cdcdf04a1478
Delegated to: Netdev Maintainers
Headers show
Series [net-next,01/15] net/mlx5e: Use the correct lag ports number when creating TISes | expand

Checks

Context Check Description
netdev/series_format success Pull request is its own cover letter
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1115 this patch: 1115
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/build_clang fail Errors and warnings before: 12 this patch: 12
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1142 this patch: 1142
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 39 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Saeed Mahameed Dec. 21, 2023, 12:57 a.m. UTC 
From: Tariq Toukan <tariqt@nvidia.com>

Print to kernel log when an SD group moves from/to ready state.

Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/lib/sd.c  | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Jiri Pirko Jan. 5, 2024, 12:12 p.m. UTC | #1 
Thu, Dec 21, 2023 at 01:57:13AM CET, saeed@kernel.org wrote:
>From: Tariq Toukan <tariqt@nvidia.com>
>
>Print to kernel log when an SD group moves from/to ready state.
>
>Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>Signed-off-by: Saeed Mahameed <saeedm@nvidia.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.


>+	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
>
>
Tariq Toukan Jan. 25, 2024, 7:42 a.m. UTC | #2 
On 05/01/2024 14:12, Jiri Pirko wrote:
> Thu, Dec 21, 2023 at 01:57:13AM CET, saeed@kernel.org wrote:
>> From: Tariq Toukan <tariqt@nvidia.com>
>>
>> Print to kernel log when an SD group moves from/to ready state.
>>
>> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>> Signed-off-by: Saeed Mahameed <saeedm@nvidia.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.
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
>>
>>
Jiri Pirko Jan. 29, 2024, 9:20 a.m. UTC | #3 
Thu, Jan 25, 2024 at 08:42:41AM CET, ttoukan.linux@gmail.com wrote:
>
>
>On 05/01/2024 14:12, Jiri Pirko wrote:
>> Thu, Dec 21, 2023 at 01:57:13AM CET, saeed@kernel.org wrote:
>> > From: Tariq Toukan <tariqt@nvidia.com>
>> > 
>> > Print to kernel log when an SD group moves from/to ready state.
>> > 
>> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>> > Signed-off-by: Saeed Mahameed <saeedm@nvidia.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
>> > 
>> >
diff mbox series

Patch

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));
+	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);