Re: [PATCH net] net: natsemi: fix `rx_dropped` double accounting on `netif_rx()` failure
From: Eric Dumazet <edumazet@google.com>
Date: 2025-09-12 14:19:13
Also in:
lkml
On Wed, Sep 10, 2025 at 10:35 PM Yeounsu Moon [off-list ref] wrote:
`netif_rx()` already increments `rx_dropped` core stat when it fails.
The driver was also updating `ndev->stats.rx_dropped` in the same path.
Since both are reported together via `ip -s -s` command, this resulted
in drops being counted twice in user-visible stats.
Keep the driver update on `skb_put()` failure, but skip it after
`netif_rx()` errors.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I do not think this Fixes: is correct.
I think core networking got this accounting in netif_rx() in 2010
commit caf586e5f23c (" net: add a core netdev->rx_dropped counter")
quoted hunk ↗ jump to hunk
Signed-off-by: Yeounsu Moon <redacted> --- drivers/net/ethernet/natsemi/ns83820.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c index 56d5464222d9..cdbf82affa7b 100644 --- a/drivers/net/ethernet/natsemi/ns83820.c +++ b/drivers/net/ethernet/natsemi/ns83820.c@@ -820,7 +820,7 @@ static void rx_irq(struct net_device *ndev) struct ns83820 *dev = PRIV(ndev); struct rx_info *info = &dev->rx_info; unsigned next_rx; - int rx_rc, len; + int len; u32 cmdsts; __le32 *desc; unsigned long flags;@@ -881,8 +881,10 @@ static void rx_irq(struct net_device *ndev) if (likely(CMDSTS_OK & cmdsts)) { #endif skb_put(skb, len); - if (unlikely(!skb))
I doubt this driver is used. Notice that this test about skb being NULL or not happens after skb_put(skb, len) which would have crashed anyway if skb was NULL.
quoted hunk ↗ jump to hunk
+ if (unlikely(!skb)) { + ndev->stats.rx_dropped++; goto netdev_mangle_me_harder_failed; + } if (cmdsts & CMDSTS_DEST_MULTI) ndev->stats.multicast++; ndev->stats.rx_packets++;@@ -901,15 +903,12 @@ static void rx_irq(struct net_device *ndev) __vlan_hwaccel_put_tag(skb, htons(ETH_P_IPV6), tag); } #endif - rx_rc = netif_rx(skb); - if (NET_RX_DROP == rx_rc) { -netdev_mangle_me_harder_failed: - ndev->stats.rx_dropped++; - } + netif_rx(skb); } else { dev_kfree_skb_irq(skb); } +netdev_mangle_me_harder_failed: nr++; next_rx = info->next_rx; desc = info->descs + (DESC_SIZE * next_rx); --2.51.0