| 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
| ||
|
Message-ID: <597a68d6-2314-47a0-af9f-138e46387978@trager.us>
Date: Fri, 25 Oct 2024 16:33:45 -0700
From: Lee Trager <lee@...ger.us>
To: Rosen Penev <rosenp@...il.com>, netdev@...r.kernel.org
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
"moderated list:INTEL ETHERNET DRIVERS" <intel-wired-lan@...ts.osuosl.org>,
open list <linux-kernel@...r.kernel.org>,
"open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)"
<bpf@...r.kernel.org>
Subject: Re: [PATCHv2 net-next] net: freescale: use ethtool string helpers
Cleans things up nicely :)
Reviewed-by: Lee Trager <lee@...ger.us>
On 10/25/24 1:37 PM, 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@...il.com>
> ---
> v2: fix wrong variable in for loop
> .../ethernet/freescale/dpaa/dpaa_ethtool.c | 40 ++++++-------------
> .../ethernet/freescale/dpaa2/dpaa2-ethtool.c | 15 +++----
> .../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 9 ++---
> .../net/ethernet/freescale/dpaa2/dpaa2-mac.h | 2 +-
> .../freescale/dpaa2/dpaa2-switch-ethtool.c | 9 ++---
> .../ethernet/freescale/enetc/enetc_ethtool.c | 35 +++++-----------
> .../net/ethernet/freescale/gianfar_ethtool.c | 8 ++--
> .../net/ethernet/freescale/ucc_geth_ethtool.c | 21 +++++-----
> 8 files changed, 51 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
> index b0060cf96090..9986f6e1f587 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
> @@ -243,38 +243,24 @@ static void dpaa_get_ethtool_stats(struct net_device *net_dev,
> static void dpaa_get_strings(struct net_device *net_dev, u32 stringset,
> u8 *data)
> {
> - unsigned int i, j, num_cpus, size;
> - char string_cpu[ETH_GSTRING_LEN];
> - u8 *strings;
> + unsigned int i, j, num_cpus;
>
> - memset(string_cpu, 0, sizeof(string_cpu));
> - strings = data;
> - num_cpus = num_online_cpus();
> - size = DPAA_STATS_GLOBAL_LEN * ETH_GSTRING_LEN;
> + num_cpus = num_online_cpus();
>
> for (i = 0; i < DPAA_STATS_PERCPU_LEN; i++) {
> - for (j = 0; j < num_cpus; j++) {
> - snprintf(string_cpu, ETH_GSTRING_LEN, "%s [CPU %d]",
> - dpaa_stats_percpu[i], j);
> - memcpy(strings, string_cpu, ETH_GSTRING_LEN);
> - strings += ETH_GSTRING_LEN;
> - }
> - snprintf(string_cpu, ETH_GSTRING_LEN, "%s [TOTAL]",
> - dpaa_stats_percpu[i]);
> - memcpy(strings, string_cpu, ETH_GSTRING_LEN);
> - strings += ETH_GSTRING_LEN;
> - }
> - for (j = 0; j < num_cpus; j++) {
> - snprintf(string_cpu, ETH_GSTRING_LEN,
> - "bpool [CPU %d]", j);
> - memcpy(strings, string_cpu, ETH_GSTRING_LEN);
> - strings += ETH_GSTRING_LEN;
> + for (j = 0; j < num_cpus; j++)
> + ethtool_sprintf(&data, "%s [CPU %d]",
> + dpaa_stats_percpu[i], j);
> +
> + ethtool_sprintf(&data, "%s [TOTAL]", dpaa_stats_percpu[i]);
> }
> - snprintf(string_cpu, ETH_GSTRING_LEN, "bpool [TOTAL]");
> - memcpy(strings, string_cpu, ETH_GSTRING_LEN);
> - strings += ETH_GSTRING_LEN;
> + for (i = 0; i < num_cpus; i++)
> + ethtool_sprintf(&data, "bpool [CPU %d]", i);
> +
> + ethtool_puts(&data, "bpool [TOTAL]");
>
> - memcpy(strings, dpaa_stats_global, size);
> + for (i = 0; i < DPAA_STATS_GLOBAL_LEN; i++)
> + ethtool_puts(&data, dpaa_stats_global[i]);
> }
>
> static int dpaa_get_hash_opts(struct net_device *dev,
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> index 7f476519b7ad..74ef77cb7078 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> @@ -217,20 +217,15 @@ static int dpaa2_eth_set_pauseparam(struct net_device *net_dev,
> static void dpaa2_eth_get_strings(struct net_device *netdev, u32 stringset,
> u8 *data)
> {
> - u8 *p = data;
> int i;
>
> switch (stringset) {
> case ETH_SS_STATS:
> - for (i = 0; i < DPAA2_ETH_NUM_STATS; i++) {
> - strscpy(p, dpaa2_ethtool_stats[i], ETH_GSTRING_LEN);
> - p += ETH_GSTRING_LEN;
> - }
> - for (i = 0; i < DPAA2_ETH_NUM_EXTRA_STATS; i++) {
> - strscpy(p, dpaa2_ethtool_extras[i], ETH_GSTRING_LEN);
> - p += ETH_GSTRING_LEN;
> - }
> - dpaa2_mac_get_strings(p);
> + for (i = 0; i < DPAA2_ETH_NUM_STATS; i++)
> + ethtool_puts(&data, dpaa2_ethtool_stats[i]);
> + for (i = 0; i < DPAA2_ETH_NUM_EXTRA_STATS; i++)
> + ethtool_puts(&data, dpaa2_ethtool_extras[i]);
> + dpaa2_mac_get_strings(&data);
> break;
> }
> }
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> index a69bb22c37ea..422ce13a7c94 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> @@ -558,15 +558,12 @@ int dpaa2_mac_get_sset_count(void)
> return DPAA2_MAC_NUM_STATS;
> }
>
> -void dpaa2_mac_get_strings(u8 *data)
> +void dpaa2_mac_get_strings(u8 **data)
> {
> - u8 *p = data;
> int i;
>
> - for (i = 0; i < DPAA2_MAC_NUM_STATS; i++) {
> - strscpy(p, dpaa2_mac_ethtool_stats[i], ETH_GSTRING_LEN);
> - p += ETH_GSTRING_LEN;
> - }
> + for (i = 0; i < DPAA2_MAC_NUM_STATS; i++)
> + ethtool_puts(data, dpaa2_mac_ethtool_stats[i]);
> }
>
> void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data)
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> index c1ec9efd413a..53f8d106d11e 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> @@ -49,7 +49,7 @@ void dpaa2_mac_disconnect(struct dpaa2_mac *mac);
>
> int dpaa2_mac_get_sset_count(void);
>
> -void dpaa2_mac_get_strings(u8 *data);
> +void dpaa2_mac_get_strings(u8 **data);
>
> void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data);
>
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c
> index 6bc1988be311..a888f6e6e9b0 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c
> @@ -170,17 +170,16 @@ dpaa2_switch_ethtool_get_sset_count(struct net_device *netdev, int sset)
> static void dpaa2_switch_ethtool_get_strings(struct net_device *netdev,
> u32 stringset, u8 *data)
> {
> - u8 *p = data;
> + const char *str;
> int i;
>
> switch (stringset) {
> case ETH_SS_STATS:
> for (i = 0; i < DPAA2_SWITCH_NUM_COUNTERS; i++) {
> - memcpy(p, dpaa2_switch_ethtool_counters[i].name,
> - ETH_GSTRING_LEN);
> - p += ETH_GSTRING_LEN;
> + str = dpaa2_switch_ethtool_counters[i].name;
> + ethtool_puts(&data, str);
> }
> - dpaa2_mac_get_strings(p);
> + dpaa2_mac_get_strings(&data);
> break;
> }
> }
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> index 2563eb8ac7b6..e1745b89362d 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> @@ -247,38 +247,25 @@ static int enetc_get_sset_count(struct net_device *ndev, int sset)
> static void enetc_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
> {
> struct enetc_ndev_priv *priv = netdev_priv(ndev);
> - u8 *p = data;
> int i, j;
>
> switch (stringset) {
> case ETH_SS_STATS:
> - for (i = 0; i < ARRAY_SIZE(enetc_si_counters); i++) {
> - strscpy(p, enetc_si_counters[i].name, ETH_GSTRING_LEN);
> - p += ETH_GSTRING_LEN;
> - }
> - for (i = 0; i < priv->num_tx_rings; i++) {
> - for (j = 0; j < ARRAY_SIZE(tx_ring_stats); j++) {
> - snprintf(p, ETH_GSTRING_LEN, tx_ring_stats[j],
> - i);
> - p += ETH_GSTRING_LEN;
> - }
> - }
> - for (i = 0; i < priv->num_rx_rings; i++) {
> - for (j = 0; j < ARRAY_SIZE(rx_ring_stats); j++) {
> - snprintf(p, ETH_GSTRING_LEN, rx_ring_stats[j],
> - i);
> - p += ETH_GSTRING_LEN;
> - }
> - }
> + for (i = 0; i < ARRAY_SIZE(enetc_si_counters); i++)
> + ethtool_puts(&data, enetc_si_counters[i].name);
> + for (i = 0; i < priv->num_tx_rings; i++)
> + for (j = 0; j < ARRAY_SIZE(tx_ring_stats); j++)
> + ethtool_sprintf(&data, tx_ring_stats[j], i);
> + for (i = 0; i < priv->num_rx_rings; i++)
> + for (j = 0; j < ARRAY_SIZE(rx_ring_stats); j++)
> + ethtool_sprintf(&data, rx_ring_stats[j], i);
>
> if (!enetc_si_is_pf(priv->si))
> break;
>
> - for (i = 0; i < ARRAY_SIZE(enetc_port_counters); i++) {
> - strscpy(p, enetc_port_counters[i].name,
> - ETH_GSTRING_LEN);
> - p += ETH_GSTRING_LEN;
> - }
> + for (i = 0; i < ARRAY_SIZE(enetc_port_counters); i++)
> + ethtool_puts(&data, enetc_port_counters[i].name);
> +
> break;
> }
> }
> diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> index a99b95c4bcfb..781d92e703cb 100644
> --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
> +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> @@ -115,12 +115,14 @@ static const char stat_gstrings[][ETH_GSTRING_LEN] = {
> static void gfar_gstrings(struct net_device *dev, u32 stringset, u8 * buf)
> {
> struct gfar_private *priv = netdev_priv(dev);
> + int i;
>
> if (priv->device_flags & FSL_GIANFAR_DEV_HAS_RMON)
> - memcpy(buf, stat_gstrings, GFAR_STATS_LEN * ETH_GSTRING_LEN);
> + for (i = 0; i < GFAR_STATS_LEN; i++)
> + ethtool_puts(&buf, stat_gstrings[i]);
> else
> - memcpy(buf, stat_gstrings,
> - GFAR_EXTRA_STATS_LEN * ETH_GSTRING_LEN);
> + for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
> + ethtool_puts(&buf, stat_gstrings[i]);
> }
>
> /* Fill in an array of 64-bit statistics from various sources.
> diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
> index 601beb93d3b3..699f346faf5c 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
> @@ -287,20 +287,17 @@ static void uec_get_strings(struct net_device *netdev, u32 stringset, u8 *buf)
> {
> struct ucc_geth_private *ugeth = netdev_priv(netdev);
> u32 stats_mode = ugeth->ug_info->statisticsMode;
> + int i;
>
> - if (stats_mode & UCC_GETH_STATISTICS_GATHERING_MODE_HARDWARE) {
> - memcpy(buf, hw_stat_gstrings, UEC_HW_STATS_LEN *
> - ETH_GSTRING_LEN);
> - buf += UEC_HW_STATS_LEN * ETH_GSTRING_LEN;
> - }
> - if (stats_mode & UCC_GETH_STATISTICS_GATHERING_MODE_FIRMWARE_TX) {
> - memcpy(buf, tx_fw_stat_gstrings, UEC_TX_FW_STATS_LEN *
> - ETH_GSTRING_LEN);
> - buf += UEC_TX_FW_STATS_LEN * ETH_GSTRING_LEN;
> - }
> + if (stats_mode & UCC_GETH_STATISTICS_GATHERING_MODE_HARDWARE)
> + for (i = 0; i < UEC_HW_STATS_LEN; i++)
> + ethtool_puts(&buf, hw_stat_gstrings[i]);
> + if (stats_mode & UCC_GETH_STATISTICS_GATHERING_MODE_FIRMWARE_TX)
> + for (i = 0; i < UEC_TX_FW_STATS_LEN; i++)
> + ethtool_puts(&buf, tx_fw_stat_gstrings[i]);
> if (stats_mode & UCC_GETH_STATISTICS_GATHERING_MODE_FIRMWARE_RX)
> - memcpy(buf, rx_fw_stat_gstrings, UEC_RX_FW_STATS_LEN *
> - ETH_GSTRING_LEN);
> + for (i = 0; i < UEC_RX_FW_STATS_LEN; i++)
> + ethtool_puts(&buf, rx_fw_stat_gstrings[i]);
> }
>
> static void uec_get_ethtool_stats(struct net_device *netdev,
Powered by blists - more mailing lists