netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 net-next] net: mellanox: use ethtool string helpers
@ 2024-11-12 21:17 Rosen Penev
  2024-11-13  9:37 ` Ido Schimmel
  0 siblings, 1 reply; 2+ messages in thread
From: Rosen Penev @ 2024-11-12 21:17 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Ido Schimmel, Petr Machata, Richard Cochran,
	open list

These are the preferred way to copy ethtool strings.

Avoids incrementing pointers all over the place.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 v3: also convert memcpy.
 v2: rebase to make it apply.
 .../mellanox/mlxbf_gige/mlxbf_gige_ethtool.c  |  5 +-
 .../mellanox/mlxsw/spectrum_ethtool.c         | 83 +++++++------------
 .../ethernet/mellanox/mlxsw/spectrum_ptp.c    |  7 +-
 3 files changed, 33 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c
index 8b63968bbee9..a430d35d4727 100644
--- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c
@@ -74,8 +74,9 @@ static void mlxbf_gige_get_strings(struct net_device *netdev, u32 stringset,
 {
 	if (stringset != ETH_SS_STATS)
 		return;
-	memcpy(buf, &mlxbf_gige_ethtool_stats_keys,
-	       sizeof(mlxbf_gige_ethtool_stats_keys));
+
+	for (int i = 0; i < ARRAY_SIZE(mlxbf_gige_ethtool_stats_keys); i++)
+		ethtool_puts(&buf, mlxbf_gige_ethtool_stats_keys[i].string);
 }
 
 static void mlxbf_gige_get_ethtool_stats(struct net_device *netdev,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index 2bed8c86b7cf..5189af0da1f4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -607,84 +607,57 @@ static void mlxsw_sp_port_get_prio_strings(u8 **p, int prio)
 {
 	int i;
 
-	for (i = 0; i < MLXSW_SP_PORT_HW_PRIO_STATS_LEN; i++) {
-		snprintf(*p, ETH_GSTRING_LEN, "%.29s_%.1d",
-			 mlxsw_sp_port_hw_prio_stats[i].str, prio);
-		*p += ETH_GSTRING_LEN;
-	}
+	for (i = 0; i < MLXSW_SP_PORT_HW_PRIO_STATS_LEN; i++)
+		ethtool_sprintf(p, "%.29s_%.1d",
+				mlxsw_sp_port_hw_prio_stats[i].str, prio);
 }
 
 static void mlxsw_sp_port_get_tc_strings(u8 **p, int tc)
 {
 	int i;
 
-	for (i = 0; i < MLXSW_SP_PORT_HW_TC_STATS_LEN; i++) {
-		snprintf(*p, ETH_GSTRING_LEN, "%.28s_%d",
-			 mlxsw_sp_port_hw_tc_stats[i].str, tc);
-		*p += ETH_GSTRING_LEN;
-	}
+	for (i = 0; i < MLXSW_SP_PORT_HW_TC_STATS_LEN; i++)
+		ethtool_sprintf(p, "%.28s_%d", mlxsw_sp_port_hw_tc_stats[i].str,
+				tc);
 }
 
 static void mlxsw_sp_port_get_strings(struct net_device *dev,
 				      u32 stringset, u8 *data)
 {
 	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
-	u8 *p = data;
 	int i;
 
-	switch (stringset) {
-	case ETH_SS_STATS:
-		for (i = 0; i < MLXSW_SP_PORT_HW_STATS_LEN; i++) {
-			memcpy(p, mlxsw_sp_port_hw_stats[i].str,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
+	if (stringset != ETH_SS_STATS)
+		return;
 
-		for (i = 0; i < MLXSW_SP_PORT_HW_RFC_2863_STATS_LEN; i++) {
-			memcpy(p, mlxsw_sp_port_hw_rfc_2863_stats[i].str,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
+	for (i = 0; i < MLXSW_SP_PORT_HW_STATS_LEN; i++)
+		ethtool_puts(&data, mlxsw_sp_port_hw_stats[i].str);
 
-		for (i = 0; i < MLXSW_SP_PORT_HW_RFC_2819_STATS_LEN; i++) {
-			memcpy(p, mlxsw_sp_port_hw_rfc_2819_stats[i].str,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
+	for (i = 0; i < MLXSW_SP_PORT_HW_RFC_2863_STATS_LEN; i++)
+		ethtool_puts(&data, mlxsw_sp_port_hw_rfc_2863_stats[i].str);
 
-		for (i = 0; i < MLXSW_SP_PORT_HW_RFC_3635_STATS_LEN; i++) {
-			memcpy(p, mlxsw_sp_port_hw_rfc_3635_stats[i].str,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
+	for (i = 0; i < MLXSW_SP_PORT_HW_RFC_2819_STATS_LEN; i++)
+		ethtool_puts(&data, mlxsw_sp_port_hw_rfc_2819_stats[i].str);
 
-		for (i = 0; i < MLXSW_SP_PORT_HW_EXT_STATS_LEN; i++) {
-			memcpy(p, mlxsw_sp_port_hw_ext_stats[i].str,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
+	for (i = 0; i < MLXSW_SP_PORT_HW_RFC_3635_STATS_LEN; i++)
+		ethtool_puts(&data, mlxsw_sp_port_hw_rfc_3635_stats[i].str);
 
-		for (i = 0; i < MLXSW_SP_PORT_HW_DISCARD_STATS_LEN; i++) {
-			memcpy(p, mlxsw_sp_port_hw_discard_stats[i].str,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
+	for (i = 0; i < MLXSW_SP_PORT_HW_EXT_STATS_LEN; i++)
+		ethtool_puts(&data, mlxsw_sp_port_hw_ext_stats[i].str);
 
-		for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++)
-			mlxsw_sp_port_get_prio_strings(&p, i);
+	for (i = 0; i < MLXSW_SP_PORT_HW_DISCARD_STATS_LEN; i++)
+		ethtool_puts(&data, mlxsw_sp_port_hw_discard_stats[i].str);
 
-		for (i = 0; i < TC_MAX_QUEUE; i++)
-			mlxsw_sp_port_get_tc_strings(&p, i);
+	for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++)
+		mlxsw_sp_port_get_prio_strings(&data, i);
 
-		mlxsw_sp_port->mlxsw_sp->ptp_ops->get_stats_strings(&p);
+	for (i = 0; i < TC_MAX_QUEUE; i++)
+		mlxsw_sp_port_get_tc_strings(&data, i);
 
-		for (i = 0; i < MLXSW_SP_PORT_HW_TRANSCEIVER_STATS_LEN; i++) {
-			memcpy(p, mlxsw_sp_port_transceiver_stats[i].str,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
-		break;
-	}
+	mlxsw_sp_port->mlxsw_sp->ptp_ops->get_stats_strings(&data);
+
+	for (i = 0; i < MLXSW_SP_PORT_HW_TRANSCEIVER_STATS_LEN; i++)
+		ethtool_puts(&data, mlxsw_sp_port_transceiver_stats[i].str);
 }
 
 static int mlxsw_sp_port_set_phys_id(struct net_device *dev,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index d94081c7658e..a8c0b84ffc28 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -1327,11 +1327,8 @@ void mlxsw_sp1_get_stats_strings(u8 **p)
 {
 	int i;
 
-	for (i = 0; i < MLXSW_SP_PTP_PORT_STATS_LEN; i++) {
-		memcpy(*p, mlxsw_sp_ptp_port_stats[i].str,
-		       ETH_GSTRING_LEN);
-		*p += ETH_GSTRING_LEN;
-	}
+	for (i = 0; i < MLXSW_SP_PTP_PORT_STATS_LEN; i++)
+		ethtool_puts(p, mlxsw_sp_ptp_port_stats[i].str);
 }
 
 void mlxsw_sp1_get_stats(struct mlxsw_sp_port *mlxsw_sp_port,
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCHv3 net-next] net: mellanox: use ethtool string helpers
  2024-11-12 21:17 [PATCHv3 net-next] net: mellanox: use ethtool string helpers Rosen Penev
@ 2024-11-13  9:37 ` Ido Schimmel
  0 siblings, 0 replies; 2+ messages in thread
From: Ido Schimmel @ 2024-11-13  9:37 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Petr Machata, Richard Cochran,
	open list

Thanks for the patch, but there are a few process issues.

On Tue, Nov 12, 2024 at 01:17:11PM -0800, Rosen Penev wrote:
> These are the preferred way to copy ethtool strings.
> 
> Avoids incrementing pointers all over the place.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>

I only tagged the mlxsw change. The mlxbf change is new.

> ---
>  v3: also convert memcpy.
>  v2: rebase to make it apply.
>  .../mellanox/mlxbf_gige/mlxbf_gige_ethtool.c  |  5 +-

Please split the mlxbf change to a different patch and copy the relevant
maintainers:

davthompson@nvidia.com
asmaa@nvidia.com

Use "mlxbf_gige:" prefix for the mlxbf patch and "mlxsw:" for the mlxsw
patch. You can keep my tag on the latter.


>  .../mellanox/mlxsw/spectrum_ethtool.c         | 83 +++++++------------
>  .../ethernet/mellanox/mlxsw/spectrum_ptp.c    |  7 +-
>  3 files changed, 33 insertions(+), 62 deletions(-)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-11-13  9:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-12 21:17 [PATCHv3 net-next] net: mellanox: use ethtool string helpers Rosen Penev
2024-11-13  9:37 ` Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).