[PATCH net-next 2/2] netdevsim: collect statistics at RX side

Breno Leitao posted 2 patches 4 months ago
There is a newer version of this series
[PATCH net-next 2/2] netdevsim: collect statistics at RX side
Posted by Breno Leitao 4 months ago
When the RX side of netdevsim was added, the RX statistics were missing,
making the driver unusable for GenerateTraffic() test framework.

This patch adds proper statistics tracking on RX side, complementing the
TX path.

Signed-off-by: Breno Leitao <Leitao@debian.org>
---
 drivers/net/netdevsim/netdev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 67871d31252fe..590cb5bb0d20b 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -39,12 +39,16 @@ MODULE_IMPORT_NS("NETDEV_INTERNAL");
 
 static int nsim_napi_rx(struct nsim_rq *rq, struct sk_buff *skb)
 {
+	struct net_device *dev = rq->napi.dev;
+
 	if (skb_queue_len(&rq->skb_queue) > NSIM_RING_SIZE) {
 		dev_kfree_skb_any(skb);
+		dev_dstats_rx_dropped(dev);
 		return NET_RX_DROP;
 	}
 
 	skb_queue_tail(&rq->skb_queue, skb);
+	dev_dstats_rx_add(dev, skb->len);
 	return NET_RX_SUCCESS;
 }
 

-- 
2.47.1
Re: [PATCH net-next 2/2] netdevsim: collect statistics at RX side
Posted by Joe Damato 4 months ago
On Wed, Jun 11, 2025 at 08:06:20AM -0700, Breno Leitao wrote:
> When the RX side of netdevsim was added, the RX statistics were missing,
> making the driver unusable for GenerateTraffic() test framework.
> 
> This patch adds proper statistics tracking on RX side, complementing the
> TX path.
> 
> Signed-off-by: Breno Leitao <Leitao@debian.org>
> ---
>  drivers/net/netdevsim/netdev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index 67871d31252fe..590cb5bb0d20b 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -39,12 +39,16 @@ MODULE_IMPORT_NS("NETDEV_INTERNAL");
>  
>  static int nsim_napi_rx(struct nsim_rq *rq, struct sk_buff *skb)
>  {
> +	struct net_device *dev = rq->napi.dev;
> +
>  	if (skb_queue_len(&rq->skb_queue) > NSIM_RING_SIZE) {
>  		dev_kfree_skb_any(skb);
> +		dev_dstats_rx_dropped(dev);
>  		return NET_RX_DROP;
>  	}
>  
>  	skb_queue_tail(&rq->skb_queue, skb);
> +	dev_dstats_rx_add(dev, skb->len);
>  	return NET_RX_SUCCESS;
>  }

Hm, I was wondering: is it maybe better to add the stats code to nsim_poll or
nsim_rcv instead?

It "feels" like other drivers would be bumping RX stats in their NAPI poll
function (which is nsim_poll, the naming of the functions is a bit confusing
here), so it seems like netdevsim maybe should too?
Re: [PATCH net-next 2/2] netdevsim: collect statistics at RX side
Posted by Jakub Kicinski 4 months ago
On Thu, 12 Jun 2025 17:25:52 +0300 Joe Damato wrote:
> It "feels" like other drivers would be bumping RX stats in their NAPI poll
> function (which is nsim_poll, the naming of the functions is a bit confusing
> here), so it seems like netdevsim maybe should too?

Good point, and then we should probably add the queue length to
rx_dropped in nsim_queue_free() before we call purge?
-- 
pw-bot: cr
Re: [PATCH net-next 2/2] netdevsim: collect statistics at RX side
Posted by Breno Leitao 4 months ago
On Thu, Jun 12, 2025 at 07:45:03AM -0700, Jakub Kicinski wrote:
> On Thu, 12 Jun 2025 17:25:52 +0300 Joe Damato wrote:
> > It "feels" like other drivers would be bumping RX stats in their NAPI poll
> > function (which is nsim_poll, the naming of the functions is a bit confusing
> > here), so it seems like netdevsim maybe should too?
> 
> Good point, and then we should probably add the queue length to
> rx_dropped in nsim_queue_free() before we call purge?

Thanks Joe and Jakub, and after a review, I agree it might be a better
approach. I will update this patch with this new approach
and send a v2 tomorrow.

Thanks for the review,
--breno