netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] net: bnxt: use ethtool string helpers
@ 2024-10-21  2:29 Rosen Penev
  2024-10-21 16:58 ` Simon Horman
  2024-10-21 20:25 ` Michael Chan
  0 siblings, 2 replies; 5+ messages in thread
From: Rosen Penev @ 2024-10-21  2:29 UTC (permalink / raw)
  To: netdev
  Cc: Michael Chan, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, open list

Avoids having to use manual pointer manipulation.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 110 ++++++++----------
 1 file changed, 50 insertions(+), 60 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index f71cc8188b4e..84d468ad3c8e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -713,18 +713,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 		for (i = 0; i < bp->cp_nr_rings; i++) {
 			if (is_rx_ring(bp, i)) {
 				num_str = NUM_RING_RX_HW_STATS;
-				for (j = 0; j < num_str; j++) {
-					sprintf(buf, "[%d]: %s", i,
+				for (j = 0; j < num_str; j++)
+					ethtool_sprintf(
+						&buf, "[%d]: %s", i,
 						bnxt_ring_rx_stats_str[j]);
-					buf += ETH_GSTRING_LEN;
-				}
 			}
 			if (is_tx_ring(bp, i)) {
 				num_str = NUM_RING_TX_HW_STATS;
 				for (j = 0; j < num_str; j++) {
-					sprintf(buf, "[%d]: %s", i,
+					ethtool_sprintf(
+						&buf, "[%d]: %s", i,
 						bnxt_ring_tx_stats_str[j]);
-					buf += ETH_GSTRING_LEN;
 				}
 			}
 			num_str = bnxt_get_num_tpa_ring_stats(bp);
@@ -736,81 +735,72 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 			else
 				str = bnxt_ring_tpa_stats_str;
 
-			for (j = 0; j < num_str; j++) {
-				sprintf(buf, "[%d]: %s", i, str[j]);
-				buf += ETH_GSTRING_LEN;
-			}
+			for (j = 0; j < num_str; j++)
+				ethtool_sprintf(&buf, "[%d]: %s", i, str[j]);
 skip_tpa_stats:
 			if (is_rx_ring(bp, i)) {
 				num_str = NUM_RING_RX_SW_STATS;
-				for (j = 0; j < num_str; j++) {
-					sprintf(buf, "[%d]: %s", i,
+				for (j = 0; j < num_str; j++)
+					ethtool_sprintf(
+						&buf, "[%d]: %s", i,
 						bnxt_rx_sw_stats_str[j]);
-					buf += ETH_GSTRING_LEN;
-				}
 			}
 			num_str = NUM_RING_CMN_SW_STATS;
-			for (j = 0; j < num_str; j++) {
-				sprintf(buf, "[%d]: %s", i,
-					bnxt_cmn_sw_stats_str[j]);
-				buf += ETH_GSTRING_LEN;
-			}
-		}
-		for (i = 0; i < BNXT_NUM_RING_ERR_STATS; i++) {
-			strscpy(buf, bnxt_ring_err_stats_arr[i], ETH_GSTRING_LEN);
-			buf += ETH_GSTRING_LEN;
+			for (j = 0; j < num_str; j++)
+				ethtool_sprintf(&buf, "[%d]: %s", i,
+						bnxt_cmn_sw_stats_str[j]);
 		}
+		for (i = 0; i < BNXT_NUM_RING_ERR_STATS; i++)
+			ethtool_puts(&buf, bnxt_ring_err_stats_arr[i]);
+
+		if (bp->flags & BNXT_FLAG_PORT_STATS)
+			for (i = 0; i < BNXT_NUM_PORT_STATS; i++)
+				ethtool_puts(&buf,
+					     bnxt_port_stats_arr[i].string);
 
-		if (bp->flags & BNXT_FLAG_PORT_STATS) {
-			for (i = 0; i < BNXT_NUM_PORT_STATS; i++) {
-				strcpy(buf, bnxt_port_stats_arr[i].string);
-				buf += ETH_GSTRING_LEN;
-			}
-		}
 		if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) {
 			u32 len;
 
 			len = min_t(u32, bp->fw_rx_stats_ext_size,
 				    ARRAY_SIZE(bnxt_port_stats_ext_arr));
-			for (i = 0; i < len; i++) {
-				strcpy(buf, bnxt_port_stats_ext_arr[i].string);
-				buf += ETH_GSTRING_LEN;
-			}
+			for (i = 0; i < len; i++)
+				ethtool_puts(&buf,
+					     bnxt_port_stats_ext_arr[i].string);
+
 			len = min_t(u32, bp->fw_tx_stats_ext_size,
 				    ARRAY_SIZE(bnxt_tx_port_stats_ext_arr));
-			for (i = 0; i < len; i++) {
-				strcpy(buf,
-				       bnxt_tx_port_stats_ext_arr[i].string);
-				buf += ETH_GSTRING_LEN;
-			}
+			for (i = 0; i < len; i++)
+				ethtool_puts(
+					&buf,
+					bnxt_tx_port_stats_ext_arr[i].string);
+
 			if (bp->pri2cos_valid) {
-				for (i = 0; i < 8; i++) {
-					strcpy(buf,
-					       bnxt_rx_bytes_pri_arr[i].string);
-					buf += ETH_GSTRING_LEN;
-				}
-				for (i = 0; i < 8; i++) {
-					strcpy(buf,
-					       bnxt_rx_pkts_pri_arr[i].string);
-					buf += ETH_GSTRING_LEN;
-				}
-				for (i = 0; i < 8; i++) {
-					strcpy(buf,
-					       bnxt_tx_bytes_pri_arr[i].string);
-					buf += ETH_GSTRING_LEN;
-				}
-				for (i = 0; i < 8; i++) {
-					strcpy(buf,
-					       bnxt_tx_pkts_pri_arr[i].string);
-					buf += ETH_GSTRING_LEN;
-				}
+				for (i = 0; i < 8; i++)
+					ethtool_puts(
+						&buf,
+						bnxt_rx_bytes_pri_arr[i].string);
+
+				for (i = 0; i < 8; i++)
+					ethtool_puts(
+						&buf,
+						bnxt_rx_pkts_pri_arr[i].string);
+
+				for (i = 0; i < 8; i++)
+					ethtool_puts(
+						&buf,
+						bnxt_tx_bytes_pri_arr[i].string);
+
+				for (i = 0; i < 8; i++)
+					ethtool_puts(
+						&buf,
+						bnxt_tx_pkts_pri_arr[i].string);
 			}
 		}
 		break;
 	case ETH_SS_TEST:
 		if (bp->num_tests)
-			memcpy(buf, bp->test_info->string,
-			       bp->num_tests * ETH_GSTRING_LEN);
+			for (i = 0; i < bp->num_tests; i++)
+				ethtool_puts(&buf, bp->test_info->string[i]);
 		break;
 	default:
 		netdev_err(bp->dev, "bnxt_get_strings invalid request %x\n",
-- 
2.47.0


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

* Re: [PATCH net-next] net: bnxt: use ethtool string helpers
  2024-10-21  2:29 [PATCH net-next] net: bnxt: use ethtool string helpers Rosen Penev
@ 2024-10-21 16:58 ` Simon Horman
  2024-10-21 20:25 ` Michael Chan
  1 sibling, 0 replies; 5+ messages in thread
From: Simon Horman @ 2024-10-21 16:58 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, Michael Chan, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, open list

On Sun, Oct 20, 2024 at 07:29:01PM -0700, Rosen Penev wrote:
> Avoids having to use manual pointer manipulation.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [PATCH net-next] net: bnxt: use ethtool string helpers
  2024-10-21  2:29 [PATCH net-next] net: bnxt: use ethtool string helpers Rosen Penev
  2024-10-21 16:58 ` Simon Horman
@ 2024-10-21 20:25 ` Michael Chan
  2024-10-21 21:05   ` Rosen Penev
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Chan @ 2024-10-21 20:25 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, open list

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

On Sun, Oct 20, 2024 at 7:29 PM Rosen Penev <rosenp@gmail.com> wrote:
>
> Avoids having to use manual pointer manipulation.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 110 ++++++++----------
>  1 file changed, 50 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index f71cc8188b4e..84d468ad3c8e 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -713,18 +713,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
>                 for (i = 0; i < bp->cp_nr_rings; i++) {
>                         if (is_rx_ring(bp, i)) {
>                                 num_str = NUM_RING_RX_HW_STATS;
> -                               for (j = 0; j < num_str; j++) {
> -                                       sprintf(buf, "[%d]: %s", i,
> +                               for (j = 0; j < num_str; j++)
> +                                       ethtool_sprintf(
> +                                               &buf, "[%d]: %s", i,

If you combine this line with the line above, I think it will look better:

ethtool_sprintf(&buf, "[%d]: %s", i,

checkpatch.pl may warn because the next line won't line up, but I
think it will look better and it reduces the number of lines.  The
same comment applies to other similar spots in the patch.  Thanks.

>                                                 bnxt_ring_rx_stats_str[j]);
> -                                       buf += ETH_GSTRING_LEN;
> -                               }

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next] net: bnxt: use ethtool string helpers
  2024-10-21 20:25 ` Michael Chan
@ 2024-10-21 21:05   ` Rosen Penev
  2024-10-21 22:19     ` Michael Chan
  0 siblings, 1 reply; 5+ messages in thread
From: Rosen Penev @ 2024-10-21 21:05 UTC (permalink / raw)
  To: Michael Chan
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, open list

On Mon, Oct 21, 2024 at 1:26 PM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Sun, Oct 20, 2024 at 7:29 PM Rosen Penev <rosenp@gmail.com> wrote:
> >
> > Avoids having to use manual pointer manipulation.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 110 ++++++++----------
> >  1 file changed, 50 insertions(+), 60 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > index f71cc8188b4e..84d468ad3c8e 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > @@ -713,18 +713,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> >                 for (i = 0; i < bp->cp_nr_rings; i++) {
> >                         if (is_rx_ring(bp, i)) {
> >                                 num_str = NUM_RING_RX_HW_STATS;
> > -                               for (j = 0; j < num_str; j++) {
> > -                                       sprintf(buf, "[%d]: %s", i,
> > +                               for (j = 0; j < num_str; j++)
> > +                                       ethtool_sprintf(
> > +                                               &buf, "[%d]: %s", i,
>
> If you combine this line with the line above, I think it will look better:
>
> ethtool_sprintf(&buf, "[%d]: %s", i,
>
> checkpatch.pl may warn because the next line won't line up, but I
> think it will look better and it reduces the number of lines.  The
> same comment applies to other similar spots in the patch.  Thanks.
I use git-clang-format for this stuff.

The only real way to satisfy everything is to use a temporary variable like

for (i = 0; i < BNXT_NUM_PORT_STATS; i++) {
  const char *str = bnxt_port_stats_arr[i].string;

  ethtool_puts(&buf, str);
  }

to make everything small.
>
> >                                                 bnxt_ring_rx_stats_str[j]);
> > -                                       buf += ETH_GSTRING_LEN;
> > -                               }

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

* Re: [PATCH net-next] net: bnxt: use ethtool string helpers
  2024-10-21 21:05   ` Rosen Penev
@ 2024-10-21 22:19     ` Michael Chan
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Chan @ 2024-10-21 22:19 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, open list

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

On Mon, Oct 21, 2024 at 2:05 PM Rosen Penev <rosenp@gmail.com> wrote:
> The only real way to satisfy everything is to use a temporary variable like
>
> for (i = 0; i < BNXT_NUM_PORT_STATS; i++) {
>   const char *str = bnxt_port_stats_arr[i].string;
>
>   ethtool_puts(&buf, str);
>   }
>
> to make everything small.

This looks fine too.  The actual function call looks clean on one line.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

end of thread, other threads:[~2024-10-21 22:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-21  2:29 [PATCH net-next] net: bnxt: use ethtool string helpers Rosen Penev
2024-10-21 16:58 ` Simon Horman
2024-10-21 20:25 ` Michael Chan
2024-10-21 21:05   ` Rosen Penev
2024-10-21 22:19     ` Michael Chan

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).