[PATCH net-next] net: dlink: count dropped packets on skb allocation failure

[PATCH net-next] net: dlink: count dropped packets on skb allocation failure

[Yeounsu Moon]

Track dropped packet statistics when skb allocation fails
in the receive path.

Tested-on: D-Link DGE-550T Rev-A3
Signed-off-by: Yeounsu Moon <yyyynoom@gmail.com>
---
 drivers/net/ethernet/dlink/dl2k.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index 6bbf6e5584e5..47d9eef2e725 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -1009,6 +1009,7 @@ receive_packet (struct net_device *dev)
 			skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
 			if (skb == NULL) {
 				np->rx_ring[entry].fraginfo = 0;
+				dev->stats.rx_dropped++;
 				printk (KERN_INFO
 					"%s: receive_packet: "
 					"Unable to re-allocate Rx skbuff.#%d\n",
-- 
2.51.0

Re: [PATCH net-next] net: dlink: count dropped packets on skb allocation failure

[Simon Horman]

On Wed, Sep 10, 2025 at 02:48:37PM +0900, Yeounsu Moon wrote:
> Track dropped packet statistics when skb allocation fails
> in the receive path.
> 
> Tested-on: D-Link DGE-550T Rev-A3
> Signed-off-by: Yeounsu Moon <yyyynoom@gmail.com>
> ---
>  drivers/net/ethernet/dlink/dl2k.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
> index 6bbf6e5584e5..47d9eef2e725 100644
> --- a/drivers/net/ethernet/dlink/dl2k.c
> +++ b/drivers/net/ethernet/dlink/dl2k.c
> @@ -1009,6 +1009,7 @@ receive_packet (struct net_device *dev)
>  			skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
>  			if (skb == NULL) {
>  				np->rx_ring[entry].fraginfo = 0;
> +				dev->stats.rx_dropped++;

Although new users of dev->stats are discoraged (see ndetdevice.h).
That is not the case here, as the driver already uses dev-stats.
So I believe this is change is good.

>  				printk (KERN_INFO
>  					"%s: receive_packet: "
>  					"Unable to re-allocate Rx skbuff.#%d\n",

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

Re: [PATCH net-next] net: dlink: count dropped packets on skb allocation failure

[Jakub Kicinski]

On Wed, 10 Sep 2025 14:48:37 +0900 Yeounsu Moon wrote:
> Track dropped packet statistics when skb allocation fails
> in the receive path.

I'm not sure that failing to allocate a buffer results in dropping
one packet in this driver. The statistics have specific meaning, if
you're just trying to use dropped to mean "buffer allocation failures"
that's not allowed. If I'm misreading the code please explain in more
detail in the commit message and repost.

> diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
> index 6bbf6e5584e5..47d9eef2e725 100644
> --- a/drivers/net/ethernet/dlink/dl2k.c
> +++ b/drivers/net/ethernet/dlink/dl2k.c
> @@ -1009,6 +1009,7 @@ receive_packet (struct net_device *dev)
>  			skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
>  			if (skb == NULL) {
>  				np->rx_ring[entry].fraginfo = 0;
> +				dev->stats.rx_dropped++;
>  				printk (KERN_INFO
>  					"%s: receive_packet: "
>  					"Unable to re-allocate Rx skbuff.#%d\n",

Re: [PATCH net-next] net: dlink: count dropped packets on skb allocation failure

[Yeounsu Moon]

On Fri Sep 12, 2025 at 2:02 AM KST, Simon Horman wrote:
> Although new users of dev->stats are discoraged (see ndetdevice.h).
> That is not the case here, as the driver already uses dev-stats.
> So I believe this is change is good.

Thank you for pointing that out! I also plan to rework statistics code
to rtnl-based statistics. However, that change affects a large amount
of code, so I'd like to introduce it gradually and gracefully.

>
>>  				printk (KERN_INFO
>>  					"%s: receive_packet: "
>>  					"Unable to re-allocate Rx skbuff.#%d\n",
>
> Reviewed-by: Simon Horman <horms@kernel.org>

Thank you again for reviewing my patch, Simon :)

	Yeounsu Moon

Re: [PATCH net-next] net: dlink: count dropped packets on skb allocation failure

[Yeounsu Moon]

On Fri Sep 12, 2025 at 9:08 AM KST, Jakub Kicinski wrote:
> On Wed, 10 Sep 2025 14:48:37 +0900 Yeounsu Moon wrote:
>> Track dropped packet statistics when skb allocation fails
>> in the receive path.
>
> I'm not sure that failing to allocate a buffer results in dropping
> one packet in this driver. The statistics have specific meaning, if
> you're just trying to use dropped to mean "buffer allocation failures"
> that's not allowed. If I'm misreading the code please explain in more
> detail in the commit message and repost.
>

I think you understand the code better than I do.
Your insights are always surprising to me.

I believed that when `netdev_alloc_skb()` fails, it leads to dropping packets.
I also found many cases where `rx_dropped` was incremented when
`netdev_alloc_skb()` failed.

However, I'm not entirely sure whether such a failure actually results
in a misisng packet. I'll resend the patch after verifying whether the packet
is really dropped.

Thank you for reviewing!

	Yeounsu Moon

Re: [PATCH net-next] net: dlink: count dropped packets on skb allocation failure

[Jakub Kicinski]

On Fri, 12 Sep 2025 19:03:46 +0900 Yeounsu Moon wrote:
> > I'm not sure that failing to allocate a buffer results in dropping
> > one packet in this driver. The statistics have specific meaning, if
> > you're just trying to use dropped to mean "buffer allocation failures"
> > that's not allowed. If I'm misreading the code please explain in more
> > detail in the commit message and repost.
> 
> I think you understand the code better than I do.
> Your insights are always surprising to me.
> 
> I believed that when `netdev_alloc_skb()` fails, it leads to dropping packets.
> I also found many cases where `rx_dropped` was incremented when
> `netdev_alloc_skb()` failed.
> 
> However, I'm not entirely sure whether such a failure actually results
> in a misisng packet. I'll resend the patch after verifying whether the packet
> is really dropped.

There's a ring of outstanding Rx buffers. If the ring becomes
completely empty we'll start dropping. But that's not the same
as one allocation failure == one packet drop.