diff mbox series

[PATCHv3,net-next,iwl-next] net: intel: use ethtool string helpers

Message ID 20241031211413.2219686-1-rosenp@gmail.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [PATCHv3,net-next,iwl-next] net: intel: use ethtool string helpers | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
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: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 7 this patch: 3
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: 4 this patch: 4
netdev/checkpatch warning CHECK: Please use a blank line after function/struct/union/enum declarations
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 99 this patch: 99
netdev/source_inline success Was 0 now: 0

Commit Message

Rosen Penev Oct. 31, 2024, 9:14 p.m. UTC 
The latter is the preferred way to copy ethtool strings.

Avoids manually incrementing the pointer. Cleans up the code quite well.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v3: change custom get_strings to u8** to make sure pointer increments
 get propagated.
 v2: add iwl-next tag. use inline int in for loops.
 .../net/ethernet/intel/e1000/e1000_ethtool.c  | 10 ++---
 drivers/net/ethernet/intel/e1000e/ethtool.c   | 14 +++---
 .../net/ethernet/intel/fm10k/fm10k_ethtool.c  | 10 ++---
 .../net/ethernet/intel/i40e/i40e_ethtool.c    |  6 +--
 drivers/net/ethernet/intel/ice/ice_ethtool.c  | 43 +++++++++++--------
 drivers/net/ethernet/intel/igb/igb_ethtool.c  | 35 ++++++++-------
 drivers/net/ethernet/intel/igbvf/ethtool.c    | 10 ++---
 drivers/net/ethernet/intel/igc/igc_ethtool.c  | 36 ++++++++--------
 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 32 +++++++-------
 drivers/net/ethernet/intel/ixgbevf/ethtool.c  | 36 ++++++----------
 10 files changed, 118 insertions(+), 114 deletions(-)

Comments

Przemek Kitszel Nov. 5, 2024, 5:47 a.m. UTC | #1 
On 10/31/24 22:14, Rosen Penev wrote:
> The latter is the preferred way to copy ethtool strings.
> 
> Avoids manually incrementing the pointer. Cleans up the code quite well.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>   v3: change custom get_strings to u8** to make sure pointer increments
>   get propagated.

I'm sorry for misleading you here, or perhaps not being clear enough.

Let me restate: I'm fine with double pointer, but single pointer is also
fine, no need to change if not used.

And my biggest corncern is that you change big chunks of the code for no
reason, please either drop those changes/those drivers, or adjust to
have only minimal changes.

please fine this complain embedded in the code inline for ice, igb, igc,
and ixgbe

>   v2: add iwl-next tag. use inline int in for loops.
>   .../net/ethernet/intel/e1000/e1000_ethtool.c  | 10 ++---
>   drivers/net/ethernet/intel/e1000e/ethtool.c   | 14 +++---
>   .../net/ethernet/intel/fm10k/fm10k_ethtool.c  | 10 ++---
>   .../net/ethernet/intel/i40e/i40e_ethtool.c    |  6 +--
>   drivers/net/ethernet/intel/ice/ice_ethtool.c  | 43 +++++++++++--------
>   drivers/net/ethernet/intel/igb/igb_ethtool.c  | 35 ++++++++-------
>   drivers/net/ethernet/intel/igbvf/ethtool.c    | 10 ++---
>   drivers/net/ethernet/intel/igc/igc_ethtool.c  | 36 ++++++++--------
>   .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 32 +++++++-------
>   drivers/net/ethernet/intel/ixgbevf/ethtool.c  | 36 ++++++----------
>   10 files changed, 118 insertions(+), 114 deletions(-)
> 


> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 2924ac61300d..81da126f83db 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -1478,51 +1478,56 @@ ice_self_test(struct net_device *netdev, struct ethtool_test *eth_test,
>   }
>   
>   static void
> -__ice_get_strings(struct net_device *netdev, u32 stringset, u8 *data,
> +__ice_get_strings(struct net_device *netdev, u32 stringset, u8 **data,
>   		  struct ice_vsi *vsi)
>   {
> +	const char *str;
>   	unsigned int i;
> -	u8 *p = data;
>   
>   	switch (stringset) {
>   	case ETH_SS_STATS:
> -		for (i = 0; i < ICE_VSI_STATS_LEN; i++)
> -			ethtool_puts(&p, ice_gstrings_vsi_stats[i].stat_string);
> +		for (i = 0; i < ICE_VSI_STATS_LEN; i++) {
> +			str = ice_gstrings_vsi_stats[i].stat_string;
> +			ethtool_puts(data, str);

please keep code to have "&p" where it is, instead of changing it to
data/&data

> +		}
>   
>   		if (ice_is_port_repr_netdev(netdev))
>   			return;
>   
>   		ice_for_each_alloc_txq(vsi, i) {
> -			ethtool_sprintf(&p, "tx_queue_%u_packets", i);
> -			ethtool_sprintf(&p, "tx_queue_%u_bytes", i);
> +			ethtool_sprintf(data, "tx_queue_%u_packets", i);
> +			ethtool_sprintf(data, "tx_queue_%u_bytes", i);

ditto

>   		}
>   
>   		ice_for_each_alloc_rxq(vsi, i) {
> -			ethtool_sprintf(&p, "rx_queue_%u_packets", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_bytes", i);
> +			ethtool_sprintf(data, "rx_queue_%u_packets", i);
> +			ethtool_sprintf(data, "rx_queue_%u_bytes", i);
>   		}
>   
>   		if (vsi->type != ICE_VSI_PF)
>   			return;
>   
> -		for (i = 0; i < ICE_PF_STATS_LEN; i++)
> -			ethtool_puts(&p, ice_gstrings_pf_stats[i].stat_string);
> +		for (i = 0; i < ICE_PF_STATS_LEN; i++) {
> +			str = ice_gstrings_pf_stats[i].stat_string;
> +			ethtool_puts(data, str);

tmp variable "str" makes this nicer, but is not worth changing in
otherwise big patch as this
for separate patch it will be too minor on the other hand

> +		}
>   
>   		for (i = 0; i < ICE_MAX_USER_PRIORITY; i++) {
> -			ethtool_sprintf(&p, "tx_priority_%u_xon.nic", i);
> -			ethtool_sprintf(&p, "tx_priority_%u_xoff.nic", i);
> +			ethtool_sprintf(data, "tx_priority_%u_xon.nic", i);
> +			ethtool_sprintf(data, "tx_priority_%u_xoff.nic", i);
>   		}
>   		for (i = 0; i < ICE_MAX_USER_PRIORITY; i++) {
> -			ethtool_sprintf(&p, "rx_priority_%u_xon.nic", i);
> -			ethtool_sprintf(&p, "rx_priority_%u_xoff.nic", i);
> +			ethtool_sprintf(data, "rx_priority_%u_xon.nic", i);
> +			ethtool_sprintf(data, "rx_priority_%u_xoff.nic", i);
>   		}
>   		break;
>   	case ETH_SS_TEST:
> -		memcpy(data, ice_gstrings_test, ICE_TEST_LEN * ETH_GSTRING_LEN);
> +		for (i = 0; i < ICE_TEST_LEN; i++)
> +			ethtool_puts(data, ice_gstrings_test[i]);
>   		break;
>   	case ETH_SS_PRIV_FLAGS:
>   		for (i = 0; i < ICE_PRIV_FLAG_ARRAY_SIZE; i++)
> -			ethtool_puts(&p, ice_gstrings_priv_flags[i].name);
> +			ethtool_puts(data, ice_gstrings_priv_flags[i].name);
>   		break;
>   	default:
>   		break;
> @@ -1533,7 +1538,7 @@ static void ice_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
>   {
>   	struct ice_netdev_priv *np = netdev_priv(netdev);
>   
> -	__ice_get_strings(netdev, stringset, data, np->vsi);
> +	__ice_get_strings(netdev, stringset, &data, np->vsi);

turns out that we gain nothing by double pointer, as @data here is
single one, I would rather revert it too

>   }
>   
>   static int
> @@ -4427,7 +4432,7 @@ ice_repr_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
>   	if (repr->ops.ready(repr) || stringset != ETH_SS_STATS)
>   		return;
>   
> -	__ice_get_strings(netdev, stringset, data, repr->src_vsi);
> +	__ice_get_strings(netdev, stringset, &data, repr->src_vsi);
>   }
>   
>   static void
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index ca6ccbc13954..c4a8712389af 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -123,7 +123,7 @@ static const char igb_gstrings_test[][ETH_GSTRING_LEN] = {
>   	[TEST_LOOP] = "Loopback test  (offline)",
>   	[TEST_LINK] = "Link test   (on/offline)"
>   };
> -#define IGB_TEST_LEN (sizeof(igb_gstrings_test) / ETH_GSTRING_LEN)
> +#define IGB_TEST_LEN ARRAY_SIZE(igb_gstrings_test)
>   
>   static const char igb_priv_flags_strings[][ETH_GSTRING_LEN] = {
>   #define IGB_PRIV_FLAGS_LEGACY_RX	BIT(0)
> @@ -2347,35 +2347,38 @@ static void igb_get_ethtool_stats(struct net_device *netdev,
>   static void igb_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
>   {
>   	struct igb_adapter *adapter = netdev_priv(netdev);
> -	u8 *p = data;
> +	const char *str;
>   	int i;
>   
>   	switch (stringset) {
>   	case ETH_SS_TEST:
> -		memcpy(data, igb_gstrings_test, sizeof(igb_gstrings_test));
> +		for (i = 0; i < IGB_TEST_LEN; i++)
> +			ethtool_puts(&data, igb_gstrings_test[i]);
>   		break;
>   	case ETH_SS_STATS:
>   		for (i = 0; i < IGB_GLOBAL_STATS_LEN; i++)
> -			ethtool_puts(&p, igb_gstrings_stats[i].stat_string);
> -		for (i = 0; i < IGB_NETDEV_STATS_LEN; i++)
> -			ethtool_puts(&p, igb_gstrings_net_stats[i].stat_string);
> +			ethtool_puts(&data, igb_gstrings_stats[i].stat_string);

same complains for igb as for ice

> +		for (i = 0; i < IGB_NETDEV_STATS_LEN; i++) {
> +			str = igb_gstrings_net_stats[i].stat_string;
> +			ethtool_puts(&data, str);
> +		}
>   		for (i = 0; i < adapter->num_tx_queues; i++) {
> -			ethtool_sprintf(&p, "tx_queue_%u_packets", i);
> -			ethtool_sprintf(&p, "tx_queue_%u_bytes", i);
> -			ethtool_sprintf(&p, "tx_queue_%u_restart", i);
> +			ethtool_sprintf(&data, "tx_queue_%u_packets", i);
> +			ethtool_sprintf(&data, "tx_queue_%u_bytes", i);
> +			ethtool_sprintf(&data, "tx_queue_%u_restart", i);
>   		}
>   		for (i = 0; i < adapter->num_rx_queues; i++) {
> -			ethtool_sprintf(&p, "rx_queue_%u_packets", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_bytes", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_drops", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_csum_err", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_alloc_failed", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_packets", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_bytes", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_drops", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_csum_err", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_alloc_failed", i);
>   		}
>   		/* BUG_ON(p - data != IGB_STATS_LEN * ETH_GSTRING_LEN); */
>   		break;
>   	case ETH_SS_PRIV_FLAGS:
> -		memcpy(data, igb_priv_flags_strings,
> -		       IGB_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
> +		for (i = 0; i < IGB_PRIV_FLAGS_STR_LEN; i++)
> +			ethtool_puts(&data, igb_priv_flags_strings[i]);
>   		break;
>   	}
>   }


> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> index 5b0c6f433767..7b118fb7097b 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> @@ -104,7 +104,7 @@ static const char igc_gstrings_test[][ETH_GSTRING_LEN] = {
>   	[TEST_LINK] = "Link test   (on/offline)"
>   };
>   
> -#define IGC_TEST_LEN (sizeof(igc_gstrings_test) / ETH_GSTRING_LEN)
> +#define IGC_TEST_LEN ARRAY_SIZE(igc_gstrings_test)
>   
>   #define IGC_GLOBAL_STATS_LEN	\
>   	(sizeof(igc_gstrings_stats) / sizeof(struct igc_stats))
> @@ -763,36 +763,38 @@ static void igc_ethtool_get_strings(struct net_device *netdev, u32 stringset,
>   				    u8 *data)
>   {
>   	struct igc_adapter *adapter = netdev_priv(netdev);
> -	u8 *p = data;
> +	const char *str;
>   	int i;
>   
>   	switch (stringset) {
>   	case ETH_SS_TEST:
> -		memcpy(data, *igc_gstrings_test,
> -		       IGC_TEST_LEN * ETH_GSTRING_LEN);
> +		for (i = 0; i < IGC_TEST_LEN; i++)
> +			ethtool_puts(&data, igc_gstrings_test[i]);
>   		break;
>   	case ETH_SS_STATS:
>   		for (i = 0; i < IGC_GLOBAL_STATS_LEN; i++)
> -			ethtool_puts(&p, igc_gstrings_stats[i].stat_string);
> -		for (i = 0; i < IGC_NETDEV_STATS_LEN; i++)
> -			ethtool_puts(&p, igc_gstrings_net_stats[i].stat_string);
> +			ethtool_puts(&data, igc_gstrings_stats[i].stat_string);
> +		for (i = 0; i < IGC_NETDEV_STATS_LEN; i++) {
> +			str = igc_gstrings_net_stats[i].stat_string;
> +			ethtool_puts(&data, str);
> +		}
>   		for (i = 0; i < adapter->num_tx_queues; i++) {
> -			ethtool_sprintf(&p, "tx_queue_%u_packets", i);
> -			ethtool_sprintf(&p, "tx_queue_%u_bytes", i);
> -			ethtool_sprintf(&p, "tx_queue_%u_restart", i);
> +			ethtool_sprintf(&data, "tx_queue_%u_packets", i);
> +			ethtool_sprintf(&data, "tx_queue_%u_bytes", i);
> +			ethtool_sprintf(&data, "tx_queue_%u_restart", i);

same complains for igc as for ice and igb

>   		}
>   		for (i = 0; i < adapter->num_rx_queues; i++) {
> -			ethtool_sprintf(&p, "rx_queue_%u_packets", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_bytes", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_drops", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_csum_err", i);
> -			ethtool_sprintf(&p, "rx_queue_%u_alloc_failed", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_packets", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_bytes", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_drops", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_csum_err", i);
> +			ethtool_sprintf(&data, "rx_queue_%u_alloc_failed", i);
>   		}
>   		/* BUG_ON(p - data != IGC_STATS_LEN * ETH_GSTRING_LEN); */
>   		break;
>   	case ETH_SS_PRIV_FLAGS:
> -		memcpy(data, igc_priv_flags_strings,
> -		       IGC_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
> +		for (i = 0; i < IGC_PRIV_FLAGS_STR_LEN; i++)
> +			ethtool_puts(&data, igc_priv_flags_strings[i]);
>   		break;
>   	}
>   }
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 9482e0cca8b7..b3b2e38c2ae6 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -129,7 +129,7 @@ static const char ixgbe_gstrings_test[][ETH_GSTRING_LEN] = {
>   	"Interrupt test (offline)", "Loopback test  (offline)",
>   	"Link test   (on/offline)"
>   };
> -#define IXGBE_TEST_LEN sizeof(ixgbe_gstrings_test) / ETH_GSTRING_LEN
> +#define IXGBE_TEST_LEN ARRAY_SIZE(ixgbe_gstrings_test)
>   
>   static const char ixgbe_priv_flags_strings[][ETH_GSTRING_LEN] = {
>   #define IXGBE_PRIV_FLAGS_LEGACY_RX	BIT(0)
> @@ -1409,38 +1409,40 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
>   static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
>   			      u8 *data)
>   {
> +	const char *str;
>   	unsigned int i;
> -	u8 *p = data;
>   
>   	switch (stringset) {
>   	case ETH_SS_TEST:
>   		for (i = 0; i < IXGBE_TEST_LEN; i++)
> -			ethtool_puts(&p, ixgbe_gstrings_test[i]);
> +			ethtool_puts(&data, ixgbe_gstrings_test[i]);

and same complains for ixgbe as the other three

[snip]
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index d06d29c6c037..33222fadb3b9 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -1839,18 +1839,18 @@  static void e1000_get_ethtool_stats(struct net_device *netdev,
 static void e1000_get_strings(struct net_device *netdev, u32 stringset,
 			      u8 *data)
 {
-	u8 *p = data;
+	const char *str;
 	int i;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, e1000_gstrings_test, sizeof(e1000_gstrings_test));
+		for (i = 0; i < E1000_TEST_LEN; i++)
+			ethtool_puts(&data, e1000_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
-			memcpy(p, e1000_gstrings_stats[i].stat_string,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
+			str = e1000_gstrings_stats[i].stat_string;
+			ethtool_puts(&data, str);
 		}
 		/* BUG_ON(p - data != E1000_STATS_LEN * ETH_GSTRING_LEN); */
 		break;
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 9364bc2b4eb1..ab590b69c14f 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -2075,23 +2075,23 @@  static void e1000_get_ethtool_stats(struct net_device *netdev,
 static void e1000_get_strings(struct net_device __always_unused *netdev,
 			      u32 stringset, u8 *data)
 {
-	u8 *p = data;
+	const char *str;
 	int i;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, e1000_gstrings_test, sizeof(e1000_gstrings_test));
+		for (i = 0; i < E1000_TEST_LEN; i++)
+			ethtool_puts(&data, e1000_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
-			memcpy(p, e1000_gstrings_stats[i].stat_string,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
+			str = e1000_gstrings_stats[i].stat_string;
+			ethtool_puts(&data, str);
 		}
 		break;
 	case ETH_SS_PRIV_FLAGS:
-		memcpy(data, e1000e_priv_flags_strings,
-		       E1000E_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < E1000E_PRIV_FLAGS_STR_LEN; i++)
+			ethtool_puts(&data, e1000e_priv_flags_strings[i]);
 		break;
 	}
 }
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index 1bc5b6c0b897..fb03bb30154a 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -122,7 +122,7 @@  static const char fm10k_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Mailbox test (on/offline)"
 };
 
-#define FM10K_TEST_LEN (sizeof(fm10k_gstrings_test) / ETH_GSTRING_LEN)
+#define FM10K_TEST_LEN ARRAY_SIZE(fm10k_gstrings_test)
 
 enum fm10k_self_test_types {
 	FM10K_TEST_MBX,
@@ -182,15 +182,15 @@  static void fm10k_get_strings(struct net_device *dev,
 {
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, fm10k_gstrings_test,
-		       FM10K_TEST_LEN * ETH_GSTRING_LEN);
+		for (int i = 0; i < FM10K_TEST_LEN; i++)
+			ethtool_puts(&data, fm10k_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		fm10k_get_stat_strings(dev, data);
 		break;
 	case ETH_SS_PRIV_FLAGS:
-		memcpy(data, fm10k_prv_flags,
-		       FM10K_PRV_FLAG_LEN * ETH_GSTRING_LEN);
+		for (int i = 0; i < FM10K_PRV_FLAG_LEN; i++)
+			ethtool_puts(&data, fm10k_prv_flags[i]);
 		break;
 	}
 }
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index f2506511bbff..90fc0c29fbd6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -426,7 +426,7 @@  static const char i40e_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Link test   (on/offline)"
 };
 
-#define I40E_TEST_LEN (sizeof(i40e_gstrings_test) / ETH_GSTRING_LEN)
+#define I40E_TEST_LEN ARRAY_SIZE(i40e_gstrings_test)
 
 struct i40e_priv_flags {
 	char flag_string[ETH_GSTRING_LEN];
@@ -2531,8 +2531,8 @@  static void i40e_get_strings(struct net_device *netdev, u32 stringset,
 {
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, i40e_gstrings_test,
-		       I40E_TEST_LEN * ETH_GSTRING_LEN);
+		for (int i = 0; i < I40E_TEST_LEN; i++)
+			ethtool_puts(&data, i40e_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		i40e_get_stat_strings(netdev, data);
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 2924ac61300d..81da126f83db 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -83,7 +83,7 @@  static const char ice_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Link test   (on/offline)",
 };
 
-#define ICE_TEST_LEN (sizeof(ice_gstrings_test) / ETH_GSTRING_LEN)
+#define ICE_TEST_LEN ARRAY_SIZE(ice_gstrings_test)
 
 /* These PF_STATs might look like duplicates of some NETDEV_STATs,
  * but they aren't. This device is capable of supporting multiple
@@ -1478,51 +1478,56 @@  ice_self_test(struct net_device *netdev, struct ethtool_test *eth_test,
 }
 
 static void
-__ice_get_strings(struct net_device *netdev, u32 stringset, u8 *data,
+__ice_get_strings(struct net_device *netdev, u32 stringset, u8 **data,
 		  struct ice_vsi *vsi)
 {
+	const char *str;
 	unsigned int i;
-	u8 *p = data;
 
 	switch (stringset) {
 	case ETH_SS_STATS:
-		for (i = 0; i < ICE_VSI_STATS_LEN; i++)
-			ethtool_puts(&p, ice_gstrings_vsi_stats[i].stat_string);
+		for (i = 0; i < ICE_VSI_STATS_LEN; i++) {
+			str = ice_gstrings_vsi_stats[i].stat_string;
+			ethtool_puts(data, str);
+		}
 
 		if (ice_is_port_repr_netdev(netdev))
 			return;
 
 		ice_for_each_alloc_txq(vsi, i) {
-			ethtool_sprintf(&p, "tx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "tx_queue_%u_bytes", i);
+			ethtool_sprintf(data, "tx_queue_%u_packets", i);
+			ethtool_sprintf(data, "tx_queue_%u_bytes", i);
 		}
 
 		ice_for_each_alloc_rxq(vsi, i) {
-			ethtool_sprintf(&p, "rx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "rx_queue_%u_bytes", i);
+			ethtool_sprintf(data, "rx_queue_%u_packets", i);
+			ethtool_sprintf(data, "rx_queue_%u_bytes", i);
 		}
 
 		if (vsi->type != ICE_VSI_PF)
 			return;
 
-		for (i = 0; i < ICE_PF_STATS_LEN; i++)
-			ethtool_puts(&p, ice_gstrings_pf_stats[i].stat_string);
+		for (i = 0; i < ICE_PF_STATS_LEN; i++) {
+			str = ice_gstrings_pf_stats[i].stat_string;
+			ethtool_puts(data, str);
+		}
 
 		for (i = 0; i < ICE_MAX_USER_PRIORITY; i++) {
-			ethtool_sprintf(&p, "tx_priority_%u_xon.nic", i);
-			ethtool_sprintf(&p, "tx_priority_%u_xoff.nic", i);
+			ethtool_sprintf(data, "tx_priority_%u_xon.nic", i);
+			ethtool_sprintf(data, "tx_priority_%u_xoff.nic", i);
 		}
 		for (i = 0; i < ICE_MAX_USER_PRIORITY; i++) {
-			ethtool_sprintf(&p, "rx_priority_%u_xon.nic", i);
-			ethtool_sprintf(&p, "rx_priority_%u_xoff.nic", i);
+			ethtool_sprintf(data, "rx_priority_%u_xon.nic", i);
+			ethtool_sprintf(data, "rx_priority_%u_xoff.nic", i);
 		}
 		break;
 	case ETH_SS_TEST:
-		memcpy(data, ice_gstrings_test, ICE_TEST_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < ICE_TEST_LEN; i++)
+			ethtool_puts(data, ice_gstrings_test[i]);
 		break;
 	case ETH_SS_PRIV_FLAGS:
 		for (i = 0; i < ICE_PRIV_FLAG_ARRAY_SIZE; i++)
-			ethtool_puts(&p, ice_gstrings_priv_flags[i].name);
+			ethtool_puts(data, ice_gstrings_priv_flags[i].name);
 		break;
 	default:
 		break;
@@ -1533,7 +1538,7 @@  static void ice_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 {
 	struct ice_netdev_priv *np = netdev_priv(netdev);
 
-	__ice_get_strings(netdev, stringset, data, np->vsi);
+	__ice_get_strings(netdev, stringset, &data, np->vsi);
 }
 
 static int
@@ -4427,7 +4432,7 @@  ice_repr_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 	if (repr->ops.ready(repr) || stringset != ETH_SS_STATS)
 		return;
 
-	__ice_get_strings(netdev, stringset, data, repr->src_vsi);
+	__ice_get_strings(netdev, stringset, &data, repr->src_vsi);
 }
 
 static void
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index ca6ccbc13954..c4a8712389af 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -123,7 +123,7 @@  static const char igb_gstrings_test[][ETH_GSTRING_LEN] = {
 	[TEST_LOOP] = "Loopback test  (offline)",
 	[TEST_LINK] = "Link test   (on/offline)"
 };
-#define IGB_TEST_LEN (sizeof(igb_gstrings_test) / ETH_GSTRING_LEN)
+#define IGB_TEST_LEN ARRAY_SIZE(igb_gstrings_test)
 
 static const char igb_priv_flags_strings[][ETH_GSTRING_LEN] = {
 #define IGB_PRIV_FLAGS_LEGACY_RX	BIT(0)
@@ -2347,35 +2347,38 @@  static void igb_get_ethtool_stats(struct net_device *netdev,
 static void igb_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
-	u8 *p = data;
+	const char *str;
 	int i;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, igb_gstrings_test, sizeof(igb_gstrings_test));
+		for (i = 0; i < IGB_TEST_LEN; i++)
+			ethtool_puts(&data, igb_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		for (i = 0; i < IGB_GLOBAL_STATS_LEN; i++)
-			ethtool_puts(&p, igb_gstrings_stats[i].stat_string);
-		for (i = 0; i < IGB_NETDEV_STATS_LEN; i++)
-			ethtool_puts(&p, igb_gstrings_net_stats[i].stat_string);
+			ethtool_puts(&data, igb_gstrings_stats[i].stat_string);
+		for (i = 0; i < IGB_NETDEV_STATS_LEN; i++) {
+			str = igb_gstrings_net_stats[i].stat_string;
+			ethtool_puts(&data, str);
+		}
 		for (i = 0; i < adapter->num_tx_queues; i++) {
-			ethtool_sprintf(&p, "tx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "tx_queue_%u_bytes", i);
-			ethtool_sprintf(&p, "tx_queue_%u_restart", i);
+			ethtool_sprintf(&data, "tx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "tx_queue_%u_bytes", i);
+			ethtool_sprintf(&data, "tx_queue_%u_restart", i);
 		}
 		for (i = 0; i < adapter->num_rx_queues; i++) {
-			ethtool_sprintf(&p, "rx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "rx_queue_%u_bytes", i);
-			ethtool_sprintf(&p, "rx_queue_%u_drops", i);
-			ethtool_sprintf(&p, "rx_queue_%u_csum_err", i);
-			ethtool_sprintf(&p, "rx_queue_%u_alloc_failed", i);
+			ethtool_sprintf(&data, "rx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "rx_queue_%u_bytes", i);
+			ethtool_sprintf(&data, "rx_queue_%u_drops", i);
+			ethtool_sprintf(&data, "rx_queue_%u_csum_err", i);
+			ethtool_sprintf(&data, "rx_queue_%u_alloc_failed", i);
 		}
 		/* BUG_ON(p - data != IGB_STATS_LEN * ETH_GSTRING_LEN); */
 		break;
 	case ETH_SS_PRIV_FLAGS:
-		memcpy(data, igb_priv_flags_strings,
-		       IGB_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < IGB_PRIV_FLAGS_STR_LEN; i++)
+			ethtool_puts(&data, igb_priv_flags_strings[i]);
 		break;
 	}
 }
diff --git a/drivers/net/ethernet/intel/igbvf/ethtool.c b/drivers/net/ethernet/intel/igbvf/ethtool.c
index 83b97989a6bd..2da95ea66718 100644
--- a/drivers/net/ethernet/intel/igbvf/ethtool.c
+++ b/drivers/net/ethernet/intel/igbvf/ethtool.c
@@ -412,18 +412,18 @@  static int igbvf_get_sset_count(struct net_device *dev, int stringset)
 static void igbvf_get_strings(struct net_device *netdev, u32 stringset,
 			      u8 *data)
 {
-	u8 *p = data;
+	const char *str;
 	int i;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, *igbvf_gstrings_test, sizeof(igbvf_gstrings_test));
+		for (i = 0; i < IGBVF_TEST_LEN; i++)
+			ethtool_puts(&data, igbvf_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		for (i = 0; i < IGBVF_GLOBAL_STATS_LEN; i++) {
-			memcpy(p, igbvf_gstrings_stats[i].stat_string,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
+			str = igbvf_gstrings_stats[i].stat_string;
+			ethtool_puts(&data, str);
 		}
 		break;
 	}
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 5b0c6f433767..7b118fb7097b 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -104,7 +104,7 @@  static const char igc_gstrings_test[][ETH_GSTRING_LEN] = {
 	[TEST_LINK] = "Link test   (on/offline)"
 };
 
-#define IGC_TEST_LEN (sizeof(igc_gstrings_test) / ETH_GSTRING_LEN)
+#define IGC_TEST_LEN ARRAY_SIZE(igc_gstrings_test)
 
 #define IGC_GLOBAL_STATS_LEN	\
 	(sizeof(igc_gstrings_stats) / sizeof(struct igc_stats))
@@ -763,36 +763,38 @@  static void igc_ethtool_get_strings(struct net_device *netdev, u32 stringset,
 				    u8 *data)
 {
 	struct igc_adapter *adapter = netdev_priv(netdev);
-	u8 *p = data;
+	const char *str;
 	int i;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, *igc_gstrings_test,
-		       IGC_TEST_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < IGC_TEST_LEN; i++)
+			ethtool_puts(&data, igc_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		for (i = 0; i < IGC_GLOBAL_STATS_LEN; i++)
-			ethtool_puts(&p, igc_gstrings_stats[i].stat_string);
-		for (i = 0; i < IGC_NETDEV_STATS_LEN; i++)
-			ethtool_puts(&p, igc_gstrings_net_stats[i].stat_string);
+			ethtool_puts(&data, igc_gstrings_stats[i].stat_string);
+		for (i = 0; i < IGC_NETDEV_STATS_LEN; i++) {
+			str = igc_gstrings_net_stats[i].stat_string;
+			ethtool_puts(&data, str);
+		}
 		for (i = 0; i < adapter->num_tx_queues; i++) {
-			ethtool_sprintf(&p, "tx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "tx_queue_%u_bytes", i);
-			ethtool_sprintf(&p, "tx_queue_%u_restart", i);
+			ethtool_sprintf(&data, "tx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "tx_queue_%u_bytes", i);
+			ethtool_sprintf(&data, "tx_queue_%u_restart", i);
 		}
 		for (i = 0; i < adapter->num_rx_queues; i++) {
-			ethtool_sprintf(&p, "rx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "rx_queue_%u_bytes", i);
-			ethtool_sprintf(&p, "rx_queue_%u_drops", i);
-			ethtool_sprintf(&p, "rx_queue_%u_csum_err", i);
-			ethtool_sprintf(&p, "rx_queue_%u_alloc_failed", i);
+			ethtool_sprintf(&data, "rx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "rx_queue_%u_bytes", i);
+			ethtool_sprintf(&data, "rx_queue_%u_drops", i);
+			ethtool_sprintf(&data, "rx_queue_%u_csum_err", i);
+			ethtool_sprintf(&data, "rx_queue_%u_alloc_failed", i);
 		}
 		/* BUG_ON(p - data != IGC_STATS_LEN * ETH_GSTRING_LEN); */
 		break;
 	case ETH_SS_PRIV_FLAGS:
-		memcpy(data, igc_priv_flags_strings,
-		       IGC_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < IGC_PRIV_FLAGS_STR_LEN; i++)
+			ethtool_puts(&data, igc_priv_flags_strings[i]);
 		break;
 	}
 }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 9482e0cca8b7..b3b2e38c2ae6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -129,7 +129,7 @@  static const char ixgbe_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Interrupt test (offline)", "Loopback test  (offline)",
 	"Link test   (on/offline)"
 };
-#define IXGBE_TEST_LEN sizeof(ixgbe_gstrings_test) / ETH_GSTRING_LEN
+#define IXGBE_TEST_LEN ARRAY_SIZE(ixgbe_gstrings_test)
 
 static const char ixgbe_priv_flags_strings[][ETH_GSTRING_LEN] = {
 #define IXGBE_PRIV_FLAGS_LEGACY_RX	BIT(0)
@@ -1409,38 +1409,40 @@  static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
 			      u8 *data)
 {
+	const char *str;
 	unsigned int i;
-	u8 *p = data;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
 		for (i = 0; i < IXGBE_TEST_LEN; i++)
-			ethtool_puts(&p, ixgbe_gstrings_test[i]);
+			ethtool_puts(&data, ixgbe_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
-		for (i = 0; i < IXGBE_GLOBAL_STATS_LEN; i++)
-			ethtool_puts(&p, ixgbe_gstrings_stats[i].stat_string);
+		for (i = 0; i < IXGBE_GLOBAL_STATS_LEN; i++) {
+			str = ixgbe_gstrings_stats[i].stat_string;
+			ethtool_puts(&data, str);
+		}
 		for (i = 0; i < netdev->num_tx_queues; i++) {
-			ethtool_sprintf(&p, "tx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "tx_queue_%u_bytes", i);
+			ethtool_sprintf(&data, "tx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "tx_queue_%u_bytes", i);
 		}
 		for (i = 0; i < IXGBE_NUM_RX_QUEUES; i++) {
-			ethtool_sprintf(&p, "rx_queue_%u_packets", i);
-			ethtool_sprintf(&p, "rx_queue_%u_bytes", i);
+			ethtool_sprintf(&data, "rx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "rx_queue_%u_bytes", i);
 		}
 		for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
-			ethtool_sprintf(&p, "tx_pb_%u_pxon", i);
-			ethtool_sprintf(&p, "tx_pb_%u_pxoff", i);
+			ethtool_sprintf(&data, "tx_pb_%u_pxon", i);
+			ethtool_sprintf(&data, "tx_pb_%u_pxoff", i);
 		}
 		for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
-			ethtool_sprintf(&p, "rx_pb_%u_pxon", i);
-			ethtool_sprintf(&p, "rx_pb_%u_pxoff", i);
+			ethtool_sprintf(&data, "rx_pb_%u_pxon", i);
+			ethtool_sprintf(&data, "rx_pb_%u_pxoff", i);
 		}
 		/* BUG_ON(p - data != IXGBE_STATS_LEN * ETH_GSTRING_LEN); */
 		break;
 	case ETH_SS_PRIV_FLAGS:
-		memcpy(data, ixgbe_priv_flags_strings,
-		       IXGBE_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < IXGBE_PRIV_FLAGS_STR_LEN; i++)
+			ethtool_puts(&data, ixgbe_priv_flags_strings[i]);
 	}
 }
 
diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index 7ac53171b041..f63a9f683e20 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -70,7 +70,7 @@  static const char ixgbe_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Link test   (on/offline)"
 };
 
-#define IXGBEVF_TEST_LEN (sizeof(ixgbe_gstrings_test) / ETH_GSTRING_LEN)
+#define IXGBEVF_TEST_LEN ARRAY_SIZE(ixgbe_gstrings_test)
 
 static const char ixgbevf_priv_flags_strings[][ETH_GSTRING_LEN] = {
 #define IXGBEVF_PRIV_FLAGS_LEGACY_RX	BIT(0)
@@ -504,43 +504,35 @@  static void ixgbevf_get_strings(struct net_device *netdev, u32 stringset,
 				u8 *data)
 {
 	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
-	char *p = (char *)data;
+	const char *str;
 	int i;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
-		memcpy(data, *ixgbe_gstrings_test,
-		       IXGBEVF_TEST_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < IXGBEVF_TEST_LEN; i++)
+			ethtool_puts(&data, ixgbe_gstrings_test[i]);
 		break;
 	case ETH_SS_STATS:
 		for (i = 0; i < IXGBEVF_GLOBAL_STATS_LEN; i++) {
-			memcpy(p, ixgbevf_gstrings_stats[i].stat_string,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
+			str = ixgbevf_gstrings_stats[i].stat_string;
+			ethtool_puts(&data, str);
 		}
-
 		for (i = 0; i < adapter->num_tx_queues; i++) {
-			sprintf(p, "tx_queue_%u_packets", i);
-			p += ETH_GSTRING_LEN;
-			sprintf(p, "tx_queue_%u_bytes", i);
-			p += ETH_GSTRING_LEN;
+			ethtool_sprintf(&data, "tx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "tx_queue_%u_bytes", i);
 		}
 		for (i = 0; i < adapter->num_xdp_queues; i++) {
-			sprintf(p, "xdp_queue_%u_packets", i);
-			p += ETH_GSTRING_LEN;
-			sprintf(p, "xdp_queue_%u_bytes", i);
-			p += ETH_GSTRING_LEN;
+			ethtool_sprintf(&data, "xdp_queue_%u_packets", i);
+			ethtool_sprintf(&data, "xdp_queue_%u_bytes", i);
 		}
 		for (i = 0; i < adapter->num_rx_queues; i++) {
-			sprintf(p, "rx_queue_%u_packets", i);
-			p += ETH_GSTRING_LEN;
-			sprintf(p, "rx_queue_%u_bytes", i);
-			p += ETH_GSTRING_LEN;
+			ethtool_sprintf(&data, "rx_queue_%u_packets", i);
+			ethtool_sprintf(&data, "rx_queue_%u_bytes", i);
 		}
 		break;
 	case ETH_SS_PRIV_FLAGS:
-		memcpy(data, ixgbevf_priv_flags_strings,
-		       IXGBEVF_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
+		for (i = 0; i < IXGBEVF_PRIV_FLAGS_STR_LEN; i++)
+			ethtool_puts(&data, ixgbevf_priv_flags_strings[i]);
 		break;
 	}
 }