.../net/ethernet/broadcom/genet/bcmgenet.c | 89 +++++++++++++++---- .../net/ethernet/broadcom/genet/bcmgenet.h | 10 +++ 2 files changed, 83 insertions(+), 16 deletions(-)
From: Zak Kemble <zakkemble@gmail.com>
This patch exposes more statistics counters in ethtool and tidies up the
counters so that they are all per-queue. The netdev counters are now only
updated synchronously in bcmgenet_get_stats instead of a mix of sync/async
throughout the driver. Hardware discarded packets are now counted in their
own missed stat instead of being lumped in with general errors.
Signed-off-by: Zak Kemble <zakkemble@gmail.com>
---
.../net/ethernet/broadcom/genet/bcmgenet.c | 89 +++++++++++++++----
.../net/ethernet/broadcom/genet/bcmgenet.h | 10 +++
2 files changed, 83 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 73d78dcb7..c395a071a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1018,6 +1018,8 @@ struct bcmgenet_stats {
tx_rings[num].packets), \
STAT_GENET_SOFT_MIB("txq" __stringify(num) "_bytes", \
tx_rings[num].bytes), \
+ STAT_GENET_SOFT_MIB("txq" __stringify(num) "_errors", \
+ tx_rings[num].errors), \
STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_bytes", \
rx_rings[num].bytes), \
STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_packets", \
@@ -1025,7 +1027,23 @@ struct bcmgenet_stats {
STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_errors", \
rx_rings[num].errors), \
STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_dropped", \
- rx_rings[num].dropped)
+ rx_rings[num].dropped), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_multicast", \
+ rx_rings[num].multicast), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_missed", \
+ rx_rings[num].missed), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_length_errors", \
+ rx_rings[num].length_errors), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_over_errors", \
+ rx_rings[num].over_errors), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_crc_errors", \
+ rx_rings[num].crc_errors), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_frame_errors", \
+ rx_rings[num].frame_errors), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_fragmented_errors", \
+ rx_rings[num].fragmented_errors), \
+ STAT_GENET_SOFT_MIB("rxq" __stringify(num) "_broadcast", \
+ rx_rings[num].broadcast)
/* There is a 0xC gap between the end of RX and beginning of TX stats and then
* between the end of TX stats and the beginning of the RX RUNT
@@ -1046,6 +1064,11 @@ static const struct bcmgenet_stats bcmgenet_gstrings_stats[] = {
STAT_NETDEV(rx_dropped),
STAT_NETDEV(tx_dropped),
STAT_NETDEV(multicast),
+ STAT_NETDEV(rx_missed_errors),
+ STAT_NETDEV(rx_length_errors),
+ STAT_NETDEV(rx_over_errors),
+ STAT_NETDEV(rx_crc_errors),
+ STAT_NETDEV(rx_frame_errors),
/* UniMAC RSV counters */
STAT_GENET_MIB_RX("rx_64_octets", mib.rx.pkt_cnt.cnt_64),
STAT_GENET_MIB_RX("rx_65_127_oct", mib.rx.pkt_cnt.cnt_127),
@@ -1983,7 +2006,8 @@ static void bcmgenet_tx_reclaim_all(struct net_device *dev)
* the transmit checksum offsets in the descriptors
*/
static struct sk_buff *bcmgenet_add_tsb(struct net_device *dev,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ struct bcmgenet_tx_ring *ring)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
struct status_64 *status = NULL;
@@ -2001,7 +2025,7 @@ static struct sk_buff *bcmgenet_add_tsb(struct net_device *dev,
if (!new_skb) {
dev_kfree_skb_any(skb);
priv->mib.tx_realloc_tsb_failed++;
- dev->stats.tx_dropped++;
+ ring->dropped++;
return NULL;
}
dev_consume_skb_any(skb);
@@ -2089,7 +2113,7 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
GENET_CB(skb)->bytes_sent = skb->len;
/* add the Transmit Status Block */
- skb = bcmgenet_add_tsb(dev, skb);
+ skb = bcmgenet_add_tsb(dev, skb, ring);
if (!skb) {
ret = NETDEV_TX_OK;
goto out;
@@ -2253,7 +2277,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
DMA_P_INDEX_DISCARD_CNT_MASK;
if (discards > ring->old_discards) {
discards = discards - ring->old_discards;
- ring->errors += discards;
+ ring->missed += discards;
ring->old_discards += discards;
/* Clear HW register when we reach 75% of maximum 0xFFFF */
@@ -2306,8 +2330,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
if (unlikely(len > RX_BUF_LENGTH)) {
netif_err(priv, rx_status, dev, "oversized packet\n");
- dev->stats.rx_length_errors++;
- dev->stats.rx_errors++;
+ ring->length_errors++;
dev_kfree_skb_any(skb);
goto next;
}
@@ -2315,7 +2338,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
if (unlikely(!(dma_flag & DMA_EOP) || !(dma_flag & DMA_SOP))) {
netif_err(priv, rx_status, dev,
"dropping fragmented packet!\n");
- ring->errors++;
+ ring->fragmented_errors++;
dev_kfree_skb_any(skb);
goto next;
}
@@ -2329,14 +2352,19 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
netif_err(priv, rx_status, dev, "dma_flag=0x%x\n",
(unsigned int)dma_flag);
if (dma_flag & DMA_RX_CRC_ERROR)
- dev->stats.rx_crc_errors++;
+ ring->crc_errors++;
if (dma_flag & DMA_RX_OV)
- dev->stats.rx_over_errors++;
+ ring->over_errors++;
if (dma_flag & DMA_RX_NO)
- dev->stats.rx_frame_errors++;
+ ring->frame_errors++;
if (dma_flag & DMA_RX_LG)
- dev->stats.rx_length_errors++;
- dev->stats.rx_errors++;
+ ring->length_errors++;
+ if ((dma_flag & (DMA_RX_CRC_ERROR |
+ DMA_RX_OV |
+ DMA_RX_NO |
+ DMA_RX_LG |
+ DMA_RX_RXER)) == DMA_RX_RXER)
+ ring->errors++;
dev_kfree_skb_any(skb);
goto next;
} /* error packet */
@@ -2359,7 +2387,9 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
ring->packets++;
ring->bytes += len;
if (dma_flag & DMA_RX_MULT)
- dev->stats.multicast++;
+ ring->multicast++;
+ else if (dma_flag & DMA_RX_BRDCAST)
+ ring->broadcast++;
/* Notify kernel */
napi_gro_receive(&ring->napi, skb);
@@ -3420,7 +3450,7 @@ static void bcmgenet_timeout(struct net_device *dev, unsigned int txqueue)
netif_trans_update(dev);
- dev->stats.tx_errors++;
+ priv->tx_rings[txqueue].errors++;
netif_tx_wake_all_queues(dev);
}
@@ -3513,8 +3543,13 @@ static struct net_device_stats *bcmgenet_get_stats(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
unsigned long tx_bytes = 0, tx_packets = 0;
+ unsigned long tx_errors = 0, tx_dropped = 0;
unsigned long rx_bytes = 0, rx_packets = 0;
unsigned long rx_errors = 0, rx_dropped = 0;
+ unsigned long rx_missed = 0, rx_length_errors = 0;
+ unsigned long rx_over_errors = 0, rx_crc_errors = 0;
+ unsigned long rx_frame_errors = 0, rx_fragmented_errors = 0;
+ unsigned long multicast = 0, broadcast = 0;
struct bcmgenet_tx_ring *tx_ring;
struct bcmgenet_rx_ring *rx_ring;
unsigned int q;
@@ -3523,6 +3558,8 @@ static struct net_device_stats *bcmgenet_get_stats(struct net_device *dev)
tx_ring = &priv->tx_rings[q];
tx_bytes += tx_ring->bytes;
tx_packets += tx_ring->packets;
+ tx_errors += tx_ring->errors;
+ tx_dropped += tx_ring->dropped;
}
for (q = 0; q <= priv->hw_params->rx_queues; q++) {
@@ -3532,15 +3569,35 @@ static struct net_device_stats *bcmgenet_get_stats(struct net_device *dev)
rx_packets += rx_ring->packets;
rx_errors += rx_ring->errors;
rx_dropped += rx_ring->dropped;
+ rx_missed += rx_ring->missed;
+ rx_length_errors += rx_ring->length_errors;
+ rx_over_errors += rx_ring->over_errors;
+ rx_crc_errors += rx_ring->crc_errors;
+ rx_frame_errors += rx_ring->frame_errors;
+ rx_fragmented_errors += rx_ring->fragmented_errors;
+ multicast += rx_ring->multicast;
+ broadcast += rx_ring->broadcast;
}
+ rx_errors += rx_length_errors;
+ rx_errors += rx_crc_errors;
+ rx_errors += rx_frame_errors;
+ rx_errors += rx_fragmented_errors;
+
dev->stats.tx_bytes = tx_bytes;
dev->stats.tx_packets = tx_packets;
+ dev->stats.tx_errors = tx_errors;
+ dev->stats.tx_dropped = tx_dropped;
dev->stats.rx_bytes = rx_bytes;
dev->stats.rx_packets = rx_packets;
dev->stats.rx_errors = rx_errors;
- dev->stats.rx_missed_errors = rx_errors;
dev->stats.rx_dropped = rx_dropped;
+ dev->stats.rx_missed_errors = rx_missed;
+ dev->stats.rx_length_errors = rx_length_errors;
+ dev->stats.rx_over_errors = rx_over_errors;
+ dev->stats.rx_crc_errors = rx_crc_errors;
+ dev->stats.rx_frame_errors = rx_frame_errors;
+ dev->stats.multicast = multicast;
return &dev->stats;
}
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 10c631bbe..429b63cc6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -517,6 +517,8 @@ struct bcmgenet_tx_ring {
struct napi_struct napi; /* NAPI per tx queue */
unsigned long packets;
unsigned long bytes;
+ unsigned long errors;
+ unsigned long dropped;
unsigned int index; /* ring index */
struct enet_cb *cbs; /* tx ring buffer control block*/
unsigned int size; /* size of each tx ring */
@@ -544,6 +546,14 @@ struct bcmgenet_rx_ring {
unsigned long packets;
unsigned long errors;
unsigned long dropped;
+ unsigned long multicast;
+ unsigned long missed;
+ unsigned long length_errors;
+ unsigned long over_errors;
+ unsigned long crc_errors;
+ unsigned long frame_errors;
+ unsigned long fragmented_errors;
+ unsigned long broadcast;
unsigned int index; /* Rx ring index */
struct enet_cb *cbs; /* Rx ring buffer control block */
unsigned int size; /* Rx ring size */
--
2.39.5On 5/11/2025 11:40 PM, zakkemble@gmail.com wrote: > From: Zak Kemble <zakkemble@gmail.com> > > This patch exposes more statistics counters in ethtool and tidies up the > counters so that they are all per-queue. The netdev counters are now only > updated synchronously in bcmgenet_get_stats instead of a mix of sync/async > throughout the driver. Hardware discarded packets are now counted in their > own missed stat instead of being lumped in with general errors. > > Signed-off-by: Zak Kemble <zakkemble@gmail.com> If you are making changes to the driver around statistics, I would rather you modernize the driver to report statistics according to how it should be done, that is following what bcmasp2 does. Thank you. -- Florian
Heya, I've split up the changes and updated the stats reporting to use ndo_get_stats64 etc. here - https://lore.kernel.org/all/20250513144107.1989-1-zakkemble@gmail.com/ On Mon, 12 May 2025 at 15:35, Florian Fainelli <florian.fainelli@broadcom.com> wrote: > > > > On 5/11/2025 11:40 PM, zakkemble@gmail.com wrote: > > From: Zak Kemble <zakkemble@gmail.com> > > > > This patch exposes more statistics counters in ethtool and tidies up the > > counters so that they are all per-queue. The netdev counters are now only > > updated synchronously in bcmgenet_get_stats instead of a mix of sync/async > > throughout the driver. Hardware discarded packets are now counted in their > > own missed stat instead of being lumped in with general errors. > > > > Signed-off-by: Zak Kemble <zakkemble@gmail.com> > > If you are making changes to the driver around statistics, I would > rather you modernize the driver to report statistics according to how it > should be done, that is following what bcmasp2 does. Thank you. > -- > Florian >
On Sun, May 11, 2025 at 10:40:36PM +0100, zakkemble@gmail.com wrote:
> From: Zak Kemble <zakkemble@gmail.com>
>
> This patch exposes more statistics counters in ethtool and tidies up the
> counters so that they are all per-queue. The netdev counters are now only
> updated synchronously in bcmgenet_get_stats instead of a mix of sync/async
> throughout the driver. Hardware discarded packets are now counted in their
> own missed stat instead of being lumped in with general errors.
Hi Zak
That sounds like a list. When i see a list in a commit message, i
think the patch needs splitting up into smaller pieces.
Andrew
---
pw-bot: cr
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
[also build test WARNING on net/main linus/master v6.15-rc6 next-20250509]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/zakkemble-gmail-com/net-bcmgenet-tidy-up-stats-expose-more-stats-in-ethtool/20250512-054217
base: net-next/main
patch link: https://lore.kernel.org/r/20250511214037.2805-1-zakkemble%40gmail.com
patch subject: [PATCH] net: bcmgenet: tidy up stats, expose more stats in ethtool
config: x86_64-buildonly-randconfig-003-20250512 (https://download.01.org/0day-ci/archive/20250512/202505121424.5NKdmAPP-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250512/202505121424.5NKdmAPP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505121424.5NKdmAPP-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/broadcom/genet/bcmgenet.c:3552:31: warning: variable 'broadcast' set but not used [-Wunused-but-set-variable]
3552 | unsigned long multicast = 0, broadcast = 0;
| ^
1 warning generated.
vim +/broadcast +3552 drivers/net/ethernet/broadcom/genet/bcmgenet.c
3541
3542 static struct net_device_stats *bcmgenet_get_stats(struct net_device *dev)
3543 {
3544 struct bcmgenet_priv *priv = netdev_priv(dev);
3545 unsigned long tx_bytes = 0, tx_packets = 0;
3546 unsigned long tx_errors = 0, tx_dropped = 0;
3547 unsigned long rx_bytes = 0, rx_packets = 0;
3548 unsigned long rx_errors = 0, rx_dropped = 0;
3549 unsigned long rx_missed = 0, rx_length_errors = 0;
3550 unsigned long rx_over_errors = 0, rx_crc_errors = 0;
3551 unsigned long rx_frame_errors = 0, rx_fragmented_errors = 0;
> 3552 unsigned long multicast = 0, broadcast = 0;
3553 struct bcmgenet_tx_ring *tx_ring;
3554 struct bcmgenet_rx_ring *rx_ring;
3555 unsigned int q;
3556
3557 for (q = 0; q <= priv->hw_params->tx_queues; q++) {
3558 tx_ring = &priv->tx_rings[q];
3559 tx_bytes += tx_ring->bytes;
3560 tx_packets += tx_ring->packets;
3561 tx_errors += tx_ring->errors;
3562 tx_dropped += tx_ring->dropped;
3563 }
3564
3565 for (q = 0; q <= priv->hw_params->rx_queues; q++) {
3566 rx_ring = &priv->rx_rings[q];
3567
3568 rx_bytes += rx_ring->bytes;
3569 rx_packets += rx_ring->packets;
3570 rx_errors += rx_ring->errors;
3571 rx_dropped += rx_ring->dropped;
3572 rx_missed += rx_ring->missed;
3573 rx_length_errors += rx_ring->length_errors;
3574 rx_over_errors += rx_ring->over_errors;
3575 rx_crc_errors += rx_ring->crc_errors;
3576 rx_frame_errors += rx_ring->frame_errors;
3577 rx_fragmented_errors += rx_ring->fragmented_errors;
3578 multicast += rx_ring->multicast;
3579 broadcast += rx_ring->broadcast;
3580 }
3581
3582 rx_errors += rx_length_errors;
3583 rx_errors += rx_crc_errors;
3584 rx_errors += rx_frame_errors;
3585 rx_errors += rx_fragmented_errors;
3586
3587 dev->stats.tx_bytes = tx_bytes;
3588 dev->stats.tx_packets = tx_packets;
3589 dev->stats.tx_errors = tx_errors;
3590 dev->stats.tx_dropped = tx_dropped;
3591 dev->stats.rx_bytes = rx_bytes;
3592 dev->stats.rx_packets = rx_packets;
3593 dev->stats.rx_errors = rx_errors;
3594 dev->stats.rx_dropped = rx_dropped;
3595 dev->stats.rx_missed_errors = rx_missed;
3596 dev->stats.rx_length_errors = rx_length_errors;
3597 dev->stats.rx_over_errors = rx_over_errors;
3598 dev->stats.rx_crc_errors = rx_crc_errors;
3599 dev->stats.rx_frame_errors = rx_frame_errors;
3600 dev->stats.multicast = multicast;
3601 return &dev->stats;
3602 }
3603
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.