From: Martin Habets <habetsm.xilinx@gmail.com>
To: Rosen Penev <rosenp@gmail.com>
Cc: netdev@vger.kernel.org, Edward Cree <ecree.xilinx@gmail.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Jesper Dangaard Brouer <hawk@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
"open list:SFC NETWORK DRIVER" <linux-net-drivers@amd.com>,
open list <linux-kernel@vger.kernel.org>,
"open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)"
<bpf@vger.kernel.org>
Subject: Re: [PATCH net-next] net: sfc: use ethtool string helpers
Date: Tue, 5 Nov 2024 09:48:55 +0000 [thread overview]
Message-ID: <20241105094855.GE595392@gmail.com> (raw)
In-Reply-To: <20241104202705.120939-1-rosenp@gmail.com>
On Mon, Nov 04, 2024 at 12:27:05PM -0800, 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>
> ---
> drivers/net/ethernet/sfc/ethtool_common.c | 34 +++++++------------
> drivers/net/ethernet/sfc/falcon/ethtool.c | 24 +++++--------
> drivers/net/ethernet/sfc/falcon/nic.c | 7 ++--
> drivers/net/ethernet/sfc/nic.c | 7 ++--
> .../net/ethernet/sfc/siena/ethtool_common.c | 34 +++++++------------
> drivers/net/ethernet/sfc/siena/nic.c | 7 ++--
> 6 files changed, 40 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
> index ae32e08540fa..d46972f45ec1 100644
> --- a/drivers/net/ethernet/sfc/ethtool_common.c
> +++ b/drivers/net/ethernet/sfc/ethtool_common.c
> @@ -403,24 +403,19 @@ static size_t efx_describe_per_queue_stats(struct efx_nic *efx, u8 *strings)
> efx_for_each_channel(channel, efx) {
> if (efx_channel_has_tx_queues(channel)) {
> n_stats++;
> - if (strings != NULL) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "tx-%u.tx_packets",
> - channel->tx_queue[0].queue /
> - EFX_MAX_TXQ_PER_CHANNEL);
> -
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(
> + &strings, "tx-%u.tx_packets",
This still fits after the opening parentheses above within 80 characters.
I would prefer that style.
Martin
> + channel->tx_queue[0].queue /
> + EFX_MAX_TXQ_PER_CHANNEL);
> }
> }
> efx_for_each_channel(channel, efx) {
> if (efx_channel_has_rx_queue(channel)) {
> n_stats++;
> - if (strings != NULL) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "rx-%d.rx_packets", channel->channel);
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(&strings, "rx-%d.rx_packets",
> + channel->channel);
> }
> }
> if (efx->xdp_tx_queue_count && efx->xdp_tx_queues) {
> @@ -428,11 +423,10 @@ static size_t efx_describe_per_queue_stats(struct efx_nic *efx, u8 *strings)
>
> for (xdp = 0; xdp < efx->xdp_tx_queue_count; xdp++) {
> n_stats++;
> - if (strings) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "tx-xdp-cpu-%hu.tx_packets", xdp);
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(&strings,
> + "tx-xdp-cpu-%hu.tx_packets",
> + xdp);
> }
> }
>
> @@ -467,9 +461,7 @@ void efx_ethtool_get_strings(struct net_device *net_dev,
> strings += (efx->type->describe_stats(efx, strings) *
> ETH_GSTRING_LEN);
> for (i = 0; i < EFX_ETHTOOL_SW_STAT_COUNT; i++)
> - strscpy(strings + i * ETH_GSTRING_LEN,
> - efx_sw_stat_desc[i].name, ETH_GSTRING_LEN);
> - strings += EFX_ETHTOOL_SW_STAT_COUNT * ETH_GSTRING_LEN;
> + ethtool_puts(&strings, efx_sw_stat_desc[i].name);
> strings += (efx_describe_per_queue_stats(efx, strings) *
> ETH_GSTRING_LEN);
> efx_ptp_describe_stats(efx, strings);
> diff --git a/drivers/net/ethernet/sfc/falcon/ethtool.c b/drivers/net/ethernet/sfc/falcon/ethtool.c
> index f4db683b80f7..41bd63d0c40c 100644
> --- a/drivers/net/ethernet/sfc/falcon/ethtool.c
> +++ b/drivers/net/ethernet/sfc/falcon/ethtool.c
> @@ -361,24 +361,18 @@ static size_t ef4_describe_per_queue_stats(struct ef4_nic *efx, u8 *strings)
> ef4_for_each_channel(channel, efx) {
> if (ef4_channel_has_tx_queues(channel)) {
> n_stats++;
> - if (strings != NULL) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "tx-%u.tx_packets",
> - channel->tx_queue[0].queue /
> - EF4_TXQ_TYPES);
> -
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(&strings, "tx-%u.tx_packets",
> + channel->tx_queue[0].queue /
> + EF4_TXQ_TYPES);
> }
> }
> ef4_for_each_channel(channel, efx) {
> if (ef4_channel_has_rx_queue(channel)) {
> n_stats++;
> - if (strings != NULL) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "rx-%d.rx_packets", channel->channel);
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(&strings, "rx-%d.rx_packets",
> + channel->channel);
> }
> }
> return n_stats;
> @@ -412,9 +406,7 @@ static void ef4_ethtool_get_strings(struct net_device *net_dev,
> strings += (efx->type->describe_stats(efx, strings) *
> ETH_GSTRING_LEN);
> for (i = 0; i < EF4_ETHTOOL_SW_STAT_COUNT; i++)
> - strscpy(strings + i * ETH_GSTRING_LEN,
> - ef4_sw_stat_desc[i].name, ETH_GSTRING_LEN);
> - strings += EF4_ETHTOOL_SW_STAT_COUNT * ETH_GSTRING_LEN;
> + ethtool_puts(&strings, ef4_sw_stat_desc[i].name);
> strings += (ef4_describe_per_queue_stats(efx, strings) *
> ETH_GSTRING_LEN);
> break;
> diff --git a/drivers/net/ethernet/sfc/falcon/nic.c b/drivers/net/ethernet/sfc/falcon/nic.c
> index 78c851b5a56f..a7f0caa8710f 100644
> --- a/drivers/net/ethernet/sfc/falcon/nic.c
> +++ b/drivers/net/ethernet/sfc/falcon/nic.c
> @@ -451,11 +451,8 @@ size_t ef4_nic_describe_stats(const struct ef4_hw_stat_desc *desc, size_t count,
>
> for_each_set_bit(index, mask, count) {
> if (desc[index].name) {
> - if (names) {
> - strscpy(names, desc[index].name,
> - ETH_GSTRING_LEN);
> - names += ETH_GSTRING_LEN;
> - }
> + if (names)
> + ethtool_puts(&names, desc[index].name);
> ++visible;
> }
> }
> diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
> index a33ed473cc8a..51c975cff4fe 100644
> --- a/drivers/net/ethernet/sfc/nic.c
> +++ b/drivers/net/ethernet/sfc/nic.c
> @@ -306,11 +306,8 @@ size_t efx_nic_describe_stats(const struct efx_hw_stat_desc *desc, size_t count,
>
> for_each_set_bit(index, mask, count) {
> if (desc[index].name) {
> - if (names) {
> - strscpy(names, desc[index].name,
> - ETH_GSTRING_LEN);
> - names += ETH_GSTRING_LEN;
> - }
> + if (names)
> + ethtool_puts(&names, desc[index].name);
> ++visible;
> }
> }
> diff --git a/drivers/net/ethernet/sfc/siena/ethtool_common.c b/drivers/net/ethernet/sfc/siena/ethtool_common.c
> index 075fef64de68..53b1cdf872d8 100644
> --- a/drivers/net/ethernet/sfc/siena/ethtool_common.c
> +++ b/drivers/net/ethernet/sfc/siena/ethtool_common.c
> @@ -403,24 +403,19 @@ static size_t efx_describe_per_queue_stats(struct efx_nic *efx, u8 *strings)
> efx_for_each_channel(channel, efx) {
> if (efx_channel_has_tx_queues(channel)) {
> n_stats++;
> - if (strings != NULL) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "tx-%u.tx_packets",
> - channel->tx_queue[0].queue /
> - EFX_MAX_TXQ_PER_CHANNEL);
> -
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(
> + &strings, "tx-%u.tx_packets",
> + channel->tx_queue[0].queue /
> + EFX_MAX_TXQ_PER_CHANNEL);
> }
> }
> efx_for_each_channel(channel, efx) {
> if (efx_channel_has_rx_queue(channel)) {
> n_stats++;
> - if (strings != NULL) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "rx-%d.rx_packets", channel->channel);
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(&strings, "rx-%d.rx_packets",
> + channel->channel);
> }
> }
> if (efx->xdp_tx_queue_count && efx->xdp_tx_queues) {
> @@ -428,11 +423,10 @@ static size_t efx_describe_per_queue_stats(struct efx_nic *efx, u8 *strings)
>
> for (xdp = 0; xdp < efx->xdp_tx_queue_count; xdp++) {
> n_stats++;
> - if (strings) {
> - snprintf(strings, ETH_GSTRING_LEN,
> - "tx-xdp-cpu-%hu.tx_packets", xdp);
> - strings += ETH_GSTRING_LEN;
> - }
> + if (strings)
> + ethtool_sprintf(&strings,
> + "tx-xdp-cpu-%hu.tx_packets",
> + xdp);
> }
> }
>
> @@ -467,9 +461,7 @@ void efx_siena_ethtool_get_strings(struct net_device *net_dev,
> strings += (efx->type->describe_stats(efx, strings) *
> ETH_GSTRING_LEN);
> for (i = 0; i < EFX_ETHTOOL_SW_STAT_COUNT; i++)
> - strscpy(strings + i * ETH_GSTRING_LEN,
> - efx_sw_stat_desc[i].name, ETH_GSTRING_LEN);
> - strings += EFX_ETHTOOL_SW_STAT_COUNT * ETH_GSTRING_LEN;
> + ethtool_puts(&strings, efx_sw_stat_desc[i].name);
> strings += (efx_describe_per_queue_stats(efx, strings) *
> ETH_GSTRING_LEN);
> efx_siena_ptp_describe_stats(efx, strings);
> diff --git a/drivers/net/ethernet/sfc/siena/nic.c b/drivers/net/ethernet/sfc/siena/nic.c
> index 0ea0433a6230..06b97218b490 100644
> --- a/drivers/net/ethernet/sfc/siena/nic.c
> +++ b/drivers/net/ethernet/sfc/siena/nic.c
> @@ -457,11 +457,8 @@ size_t efx_siena_describe_stats(const struct efx_hw_stat_desc *desc, size_t coun
>
> for_each_set_bit(index, mask, count) {
> if (desc[index].name) {
> - if (names) {
> - strscpy(names, desc[index].name,
> - ETH_GSTRING_LEN);
> - names += ETH_GSTRING_LEN;
> - }
> + if (names)
> + ethtool_puts(&names, desc[index].name);
> ++visible;
> }
> }
> --
> 2.47.0
>
next prev parent reply other threads:[~2024-11-05 9:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-04 20:27 [PATCH net-next] net: sfc: use ethtool string helpers Rosen Penev
2024-11-05 9:48 ` Martin Habets [this message]
2024-11-05 23:20 ` Rosen Penev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241105094855.GE595392@gmail.com \
--to=habetsm.xilinx@gmail.com \
--cc=andrew+netdev@lunn.ch \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=ecree.xilinx@gmail.com \
--cc=edumazet@google.com \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-net-drivers@amd.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=rosenp@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.