Add a call to dev_dstats_rx_dropped_add() in nsim_queue_free() to
account for the number of packets dropped when purging the skb queue.

This improves the accuracy of RX drop statistics reported by
netdevsim.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/netdevsim/netdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index de309ff69e43e..5d0b801e81129 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -632,9 +632,10 @@ static struct nsim_rq *nsim_queue_alloc(void)
 	return rq;
 }
 
-static void nsim_queue_free(struct nsim_rq *rq)
+static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq)
 {
 	hrtimer_cancel(&rq->napi_timer);
+	dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
 	skb_queue_purge_reason(&rq->skb_queue, SKB_DROP_REASON_QUEUE_PURGE);
 	kfree(rq);
 }
@@ -681,7 +682,7 @@ nsim_queue_mem_alloc(struct net_device *dev, void *per_queue_mem, int idx)
 	return 0;
 
 err_free:
-	nsim_queue_free(qmem->rq);
+	nsim_queue_free(dev, qmem->rq);
 	return err;
 }
 
@@ -695,7 +696,7 @@ static void nsim_queue_mem_free(struct net_device *dev, void *per_queue_mem)
 		if (!ns->rq_reset_mode)
 			netif_napi_del_locked(&qmem->rq->napi);
 		page_pool_destroy(qmem->rq->page_pool);
-		nsim_queue_free(qmem->rq);
+		nsim_queue_free(dev, qmem->rq);
 	}
 }
 
@@ -913,7 +914,7 @@ static void nsim_queue_uninit(struct netdevsim *ns)
 	int i;
 
 	for (i = 0; i < dev->num_rx_queues; i++)
-		nsim_queue_free(ns->rq[i]);
+		nsim_queue_free(dev, ns->rq[i]);
 
 	kfree(ns->rq);
 	ns->rq = NULL;

-- 
2.47.1

On Tue, 17 Jun 2025 01:19:00 -0700 Breno Leitao wrote:
> -static void nsim_queue_free(struct nsim_rq *rq)
> +static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq)
>  {
>  	hrtimer_cancel(&rq->napi_timer);
> +	dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);

here we are in process context and debug checks complain about the use
of this_cpu_ptr(). Let's wrap this in local_bh_disable() / enable() ?

>  	skb_queue_purge_reason(&rq->skb_queue, SKB_DROP_REASON_QUEUE_PURGE);
>  	kfree(rq);
>  }
-- 
pw-bot: cr

On Tue, Jun 17, 2025 at 05:59:34AM -0700, Jakub Kicinski wrote:
> On Tue, 17 Jun 2025 01:19:00 -0700 Breno Leitao wrote:
> > -static void nsim_queue_free(struct nsim_rq *rq)
> > +static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq)
> >  {
> >  	hrtimer_cancel(&rq->napi_timer);
> > +	dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
> 
> here we are in process context and debug checks complain about the use
> of this_cpu_ptr(). Let's wrap this in local_bh_disable() / enable() ?

Thanks. I was able to reproduce it. Your suggestion avoids the complain.
I suppose we should just wrap dev_dstats_rx_dropped_add(), right?

	diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
	index 5d0b801e81129..07171cf8b07ee 100644
	--- a/drivers/net/netdevsim/netdev.c
	+++ b/drivers/net/netdevsim/netdev.c
	@@ -635,7 +635,9 @@ static struct nsim_rq *nsim_queue_alloc(void)
	static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq)
	{
		hrtimer_cancel(&rq->napi_timer);
	+	local_bh_disable();
		dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen);
	+	local_bh_enable();
		skb_queue_purge_reason(&rq->skb_queue, SKB_DROP_REASON_QUEUE_PURGE);
		kfree(rq);
	}

On Tue, 17 Jun 2025 06:24:23 -0700 Breno Leitao wrote:
> > here we are in process context and debug checks complain about the use
> > of this_cpu_ptr(). Let's wrap this in local_bh_disable() / enable() ?  
> 
> Thanks. I was able to reproduce it. Your suggestion avoids the complain.
> I suppose we should just wrap dev_dstats_rx_dropped_add(), right?

I think so. I hope that makes the preempt checker happy.
The reason for bh_disable rather than preempt_disable() is that we 
should only update the stats from one context, the NAPI poll context.
So we also need to prevent races with another NAPI running on the same
core while we free an old queue.