| Message ID | 20231026013231.53271-1-michael.chan@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9cfe8cf5027bf4354e752d40c784219225f74004 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] bnxt_en: Fix 2 stray ethtool -S counters | expand |
On 26.10.2023 03:32, Michael Chan wrote: > The recent firmware interface change has added 2 counters in struct > rx_port_stats_ext. This caused 2 stray ethtool counters to be > displayed. > > Since new counters are added from time to time, fix it so that the > ethtool logic will only display up to the maximum known counters. > These 2 counters are not used by production firmware yet. > > Fixes: 754fbf604ff6 ("bnxt_en: Update firmware interface to 1.10.2.171") If this is a fix than the target should be "net" not "net-next". > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> > Signed-off-by: Michael Chan <michael.chan@broadcom.com> > --- > .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 28 +++++++++++++++---- > 1 file changed, 22 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c > index 53442aaabe5e..f3f384773ac0 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c > @@ -535,6 +535,7 @@ static int bnxt_get_num_ring_stats(struct bnxt *bp) > static int bnxt_get_num_stats(struct bnxt *bp) > { > int num_stats = bnxt_get_num_ring_stats(bp); > + int len; > > num_stats += BNXT_NUM_RING_ERR_STATS; > > @@ -542,8 +543,12 @@ static int bnxt_get_num_stats(struct bnxt *bp) > num_stats += BNXT_NUM_PORT_STATS; > > if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) { > - num_stats += bp->fw_rx_stats_ext_size + > - bp->fw_tx_stats_ext_size; > + len = min_t(int, bp->fw_rx_stats_ext_size, > + ARRAY_SIZE(bnxt_port_stats_ext_arr)); You don't need "len" var. Why not just: num_stats += min_t(int, bp->fw_rx_stats_ext_size, ARRAY_SIZE(bnxt_port_stats_ext_arr)); > + num_stats += len; > + len = min_t(int, bp->fw_tx_stats_ext_size, > + ARRAY_SIZE(bnxt_tx_port_stats_ext_arr)); > + num_stats += len; > if (bp->pri2cos_valid) > num_stats += BNXT_NUM_STATS_PRI; > } > @@ -653,12 +658,17 @@ static void bnxt_get_ethtool_stats(struct net_device *dev, > if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) { > u64 *rx_port_stats_ext = bp->rx_port_stats_ext.sw_stats; > u64 *tx_port_stats_ext = bp->tx_port_stats_ext.sw_stats; > + u32 len; > > - for (i = 0; i < bp->fw_rx_stats_ext_size; i++, j++) { > + len = min_t(u32, bp->fw_rx_stats_ext_size, > + ARRAY_SIZE(bnxt_port_stats_ext_arr)); > + for (i = 0; i < len; i++, j++) { > buf[j] = *(rx_port_stats_ext + > bnxt_port_stats_ext_arr[i].offset); > } > - for (i = 0; i < bp->fw_tx_stats_ext_size; i++, j++) { > + len = min_t(u32, bp->fw_tx_stats_ext_size, > + ARRAY_SIZE(bnxt_tx_port_stats_ext_arr)); > + for (i = 0; i < len; i++, j++) { > buf[j] = *(tx_port_stats_ext + > bnxt_tx_port_stats_ext_arr[i].offset); > } > @@ -757,11 +767,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf) > } > } > if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) { > - for (i = 0; i < bp->fw_rx_stats_ext_size; i++) { > + 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 < bp->fw_tx_stats_ext_size; i++) { > + 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;
On Thu, 26 Oct 2023 12:36:37 +0200 Wojciech Drewek wrote: > > Fixes: 754fbf604ff6 ("bnxt_en: Update firmware interface to 1.10.2.171") > > If this is a fix than the target should be "net" not "net-next". The commit in question hasn't reached net yet, this is the second time you're going this incorrect feedback: https://lore.kernel.org/all/20231023093256.0dd8f145@kernel.org/ > You don't need "len" var. > Why not just: > num_stats += min_t(int, bp->fw_rx_stats_ext_size, > ARRAY_SIZE(bnxt_port_stats_ext_arr)); I think it's needed to make sure lines don't go over 80 chars.
On 26.10.2023 16:28, Jakub Kicinski wrote: > On Thu, 26 Oct 2023 12:36:37 +0200 Wojciech Drewek wrote: >>> Fixes: 754fbf604ff6 ("bnxt_en: Update firmware interface to 1.10.2.171") >> >> If this is a fix than the target should be "net" not "net-next". > > The commit in question hasn't reached net yet, this is the second time > you're going this incorrect feedback: > > https://lore.kernel.org/all/20231023093256.0dd8f145@kernel.org/ Sorry, I somehow missed your comment. > >> You don't need "len" var. >> Why not just: >> num_stats += min_t(int, bp->fw_rx_stats_ext_size, >> ARRAY_SIZE(bnxt_port_stats_ext_arr)); > > I think it's needed to make sure lines don't go over 80 chars. Makes sense
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 25 Oct 2023 18:32:31 -0700 you wrote: > The recent firmware interface change has added 2 counters in struct > rx_port_stats_ext. This caused 2 stray ethtool counters to be > displayed. > > Since new counters are added from time to time, fix it so that the > ethtool logic will only display up to the maximum known counters. > These 2 counters are not used by production firmware yet. > > [...] Here is the summary with links: - [net-next] bnxt_en: Fix 2 stray ethtool -S counters https://git.kernel.org/netdev/net-next/c/9cfe8cf5027b You are awesome, thank you!
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c index 53442aaabe5e..f3f384773ac0 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -535,6 +535,7 @@ static int bnxt_get_num_ring_stats(struct bnxt *bp) static int bnxt_get_num_stats(struct bnxt *bp) { int num_stats = bnxt_get_num_ring_stats(bp); + int len; num_stats += BNXT_NUM_RING_ERR_STATS; @@ -542,8 +543,12 @@ static int bnxt_get_num_stats(struct bnxt *bp) num_stats += BNXT_NUM_PORT_STATS; if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) { - num_stats += bp->fw_rx_stats_ext_size + - bp->fw_tx_stats_ext_size; + len = min_t(int, bp->fw_rx_stats_ext_size, + ARRAY_SIZE(bnxt_port_stats_ext_arr)); + num_stats += len; + len = min_t(int, bp->fw_tx_stats_ext_size, + ARRAY_SIZE(bnxt_tx_port_stats_ext_arr)); + num_stats += len; if (bp->pri2cos_valid) num_stats += BNXT_NUM_STATS_PRI; } @@ -653,12 +658,17 @@ static void bnxt_get_ethtool_stats(struct net_device *dev, if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) { u64 *rx_port_stats_ext = bp->rx_port_stats_ext.sw_stats; u64 *tx_port_stats_ext = bp->tx_port_stats_ext.sw_stats; + u32 len; - for (i = 0; i < bp->fw_rx_stats_ext_size; i++, j++) { + len = min_t(u32, bp->fw_rx_stats_ext_size, + ARRAY_SIZE(bnxt_port_stats_ext_arr)); + for (i = 0; i < len; i++, j++) { buf[j] = *(rx_port_stats_ext + bnxt_port_stats_ext_arr[i].offset); } - for (i = 0; i < bp->fw_tx_stats_ext_size; i++, j++) { + len = min_t(u32, bp->fw_tx_stats_ext_size, + ARRAY_SIZE(bnxt_tx_port_stats_ext_arr)); + for (i = 0; i < len; i++, j++) { buf[j] = *(tx_port_stats_ext + bnxt_tx_port_stats_ext_arr[i].offset); } @@ -757,11 +767,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf) } } if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) { - for (i = 0; i < bp->fw_rx_stats_ext_size; i++) { + 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 < bp->fw_tx_stats_ext_size; i++) { + 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;
The recent firmware interface change has added 2 counters in struct rx_port_stats_ext. This caused 2 stray ethtool counters to be displayed. Since new counters are added from time to time, fix it so that the ethtool logic will only display up to the maximum known counters. These 2 counters are not used by production firmware yet. Fixes: 754fbf604ff6 ("bnxt_en: Update firmware interface to 1.10.2.171") Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Michael Chan <michael.chan@broadcom.com> --- .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-)