Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH] virtio_net: add rx-alloc-fail counter
@ 2026-08-11  9:10 Longjun Tang
  2026-08-11 15:40 ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Longjun Tang @ 2026-08-11  9:10 UTC (permalink / raw)
  To: mst, kuba, jasowangio; +Cc: xuanzhuo, virtualization, tanglongjun, lange_tang

From: tanglongjun <tanglongjun@kylinos.cn>

Count buffer and skb allocation failures on the rx queue and
report them via rx-alloc-fail in netdev qstats.

$ ./tools/net/ynl/ynltool/ynltool qstats show --json scope queue
[{"ifname":"ens3",
"ifindex":2,
"queue-type":"rx",
"queue-id":0,
"rx":{"packets":3722,"bytes":374806,"alloc-fail":0}},
{"ifname":"ens3",
"ifindex":2,
"queue-type":"tx",
"queue-id":0,
"tx":{"packets":622,"bytes":77495,"stop":0,"wake":0}}]

Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
---
 drivers/net/virtio_net.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3e2a5876c6c8..f9d15c618d3b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -125,6 +125,7 @@ struct virtnet_rq_stats {
 	u64_stats_t packets;
 	u64_stats_t bytes;
 	u64_stats_t drops;
+	u64_stats_t alloc_fail;
 	u64_stats_t xdp_packets;
 	u64_stats_t xdp_tx;
 	u64_stats_t xdp_redirects;
@@ -173,8 +174,9 @@ static const struct virtnet_stat_desc virtnet_sq_stats_desc_qstat[] = {
 };
 
 static const struct virtnet_stat_desc virtnet_rq_stats_desc_qstat[] = {
-	VIRTNET_RQ_STAT_QSTAT("packets", packets),
-	VIRTNET_RQ_STAT_QSTAT("bytes",   bytes),
+	VIRTNET_RQ_STAT_QSTAT("packets",    packets),
+	VIRTNET_RQ_STAT_QSTAT("bytes",      bytes),
+	VIRTNET_RQ_STAT_QSTAT("alloc_fail", alloc_fail),
 };
 
 #define VIRTNET_STATS_DESC_CQ(name) \
@@ -1917,8 +1919,10 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
 	}
 
 	skb = virtnet_build_skb(buf, buflen, xdp.data - buf, len);
-	if (unlikely(!skb))
+	if (unlikely(!skb)) {
+		u64_stats_inc(&stats->alloc_fail);
 		goto err;
+	}
 
 	if (metasize)
 		skb_metadata_set(skb, metasize);
@@ -1985,6 +1989,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
 		return skb;
 	}
 
+	u64_stats_inc(&stats->alloc_fail);
 err:
 	u64_stats_inc(&stats->drops);
 	page_pool_put_page(rq->page_pool, page, -1, true);
@@ -2016,8 +2021,10 @@ static struct sk_buff *receive_big(struct net_device *dev,
 
 	skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE, 0);
 	u64_stats_add(&stats->bytes, len - vi->hdr_len);
-	if (unlikely(!skb))
+	if (unlikely(!skb)) {
+		u64_stats_inc(&stats->alloc_fail);
 		goto err;
+	}
 
 	return skb;
 
@@ -2298,8 +2305,10 @@ static struct sk_buff *receive_mergeable_xdp(struct net_device *dev,
 	switch (act) {
 	case XDP_PASS:
 		head_skb = build_skb_from_xdp_buff(dev, vi, &xdp, xdp_frags_truesz);
-		if (unlikely(!head_skb))
+		if (unlikely(!head_skb)) {
+			u64_stats_inc(&stats->alloc_fail);
 			break;
+		}
 
 		skb_mark_for_recycle(head_skb);
 		return head_skb;
@@ -2414,8 +2423,10 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 	head_skb = page_to_skb(vi, rq, page, offset, len, truesize, headroom);
 	curr_skb = head_skb;
 
-	if (unlikely(!curr_skb))
+	if (unlikely(!curr_skb)) {
+		u64_stats_inc(&stats->alloc_fail);
 		goto err_skb;
+	}
 
 	skb_mark_for_recycle(head_skb);
 	while (--num_buf) {
@@ -2444,8 +2455,10 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 		truesize = mergeable_ctx_to_truesize(ctx);
 		curr_skb  = virtnet_skb_append_frag(rq, head_skb, curr_skb, page,
 						    buf, len, truesize);
-		if (!curr_skb)
+		if (!curr_skb) {
+			u64_stats_inc(&stats->alloc_fail);
 			goto err_skb;
+		}
 	}
 
 	ewma_pkt_len_add(&rq->mrg_avg_pkt_len, head_skb->len);
@@ -2928,12 +2941,14 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 
 	u64_stats_set(&stats.packets, packets);
 	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
-		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
+		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
 			/* We need to retry refilling in the next NAPI poll so
 			 * we must return budget to make sure the NAPI is
 			 * repolled.
 			 */
 			packets = budget;
+			u64_stats_inc(&stats.alloc_fail);
+		}
 	}
 
 	u64_stats_update_begin(&rq->stats.syncp);
@@ -2948,6 +2963,7 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 
 	u64_stats_add(&rq->stats.packets, u64_stats_read(&stats.packets));
 	u64_stats_add(&rq->stats.bytes, u64_stats_read(&stats.bytes));
+	u64_stats_add(&rq->stats.alloc_fail, u64_stats_read(&stats.alloc_fail));
 
 	u64_stats_update_end(&rq->stats.syncp);
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] virtio_net: add rx-alloc-fail counter
  2026-08-11  9:10 Longjun Tang
@ 2026-08-11 15:40 ` Jakub Kicinski
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-08-11 15:40 UTC (permalink / raw)
  To: Longjun Tang; +Cc: mst, jasowangio, xuanzhuo, virtualization, tanglongjun

On Tue, 11 Aug 2026 17:10:45 +0800 Longjun Tang wrote:
> Count buffer and skb allocation failures on the rx queue and
> report them via rx-alloc-fail in netdev qstats.

please repost and CC netdev@ (if you haven't already)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] virtio_net: add rx-alloc-fail counter
@ 2026-08-12  1:22 Longjun Tang
  2026-08-12 23:34 ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Longjun Tang @ 2026-08-12  1:22 UTC (permalink / raw)
  To: mst, kuba, jasowangio
  Cc: xuanzhuo, virtualization, netdev, tanglongjun, lange_tang

From: tanglongjun <tanglongjun@kylinos.cn>

Count buffer and skb allocation failures on the rx queue and
report them via rx-alloc-fail in netdev qstats.

$ ./tools/net/ynl/ynltool/ynltool qstats show --json scope queue
[{"ifname":"ens3",
"ifindex":2,
"queue-type":"rx",
"queue-id":0,
"rx":{"packets":3722,"bytes":374806,"alloc-fail":0}},
{"ifname":"ens3",
"ifindex":2,
"queue-type":"tx",
"queue-id":0,
"tx":{"packets":622,"bytes":77495,"stop":0,"wake":0}}]

Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
---
 drivers/net/virtio_net.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3e2a5876c6c8..f9d15c618d3b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -125,6 +125,7 @@ struct virtnet_rq_stats {
 	u64_stats_t packets;
 	u64_stats_t bytes;
 	u64_stats_t drops;
+	u64_stats_t alloc_fail;
 	u64_stats_t xdp_packets;
 	u64_stats_t xdp_tx;
 	u64_stats_t xdp_redirects;
@@ -173,8 +174,9 @@ static const struct virtnet_stat_desc virtnet_sq_stats_desc_qstat[] = {
 };
 
 static const struct virtnet_stat_desc virtnet_rq_stats_desc_qstat[] = {
-	VIRTNET_RQ_STAT_QSTAT("packets", packets),
-	VIRTNET_RQ_STAT_QSTAT("bytes",   bytes),
+	VIRTNET_RQ_STAT_QSTAT("packets",    packets),
+	VIRTNET_RQ_STAT_QSTAT("bytes",      bytes),
+	VIRTNET_RQ_STAT_QSTAT("alloc_fail", alloc_fail),
 };
 
 #define VIRTNET_STATS_DESC_CQ(name) \
@@ -1917,8 +1919,10 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
 	}
 
 	skb = virtnet_build_skb(buf, buflen, xdp.data - buf, len);
-	if (unlikely(!skb))
+	if (unlikely(!skb)) {
+		u64_stats_inc(&stats->alloc_fail);
 		goto err;
+	}
 
 	if (metasize)
 		skb_metadata_set(skb, metasize);
@@ -1985,6 +1989,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
 		return skb;
 	}
 
+	u64_stats_inc(&stats->alloc_fail);
 err:
 	u64_stats_inc(&stats->drops);
 	page_pool_put_page(rq->page_pool, page, -1, true);
@@ -2016,8 +2021,10 @@ static struct sk_buff *receive_big(struct net_device *dev,
 
 	skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE, 0);
 	u64_stats_add(&stats->bytes, len - vi->hdr_len);
-	if (unlikely(!skb))
+	if (unlikely(!skb)) {
+		u64_stats_inc(&stats->alloc_fail);
 		goto err;
+	}
 
 	return skb;
 
@@ -2298,8 +2305,10 @@ static struct sk_buff *receive_mergeable_xdp(struct net_device *dev,
 	switch (act) {
 	case XDP_PASS:
 		head_skb = build_skb_from_xdp_buff(dev, vi, &xdp, xdp_frags_truesz);
-		if (unlikely(!head_skb))
+		if (unlikely(!head_skb)) {
+			u64_stats_inc(&stats->alloc_fail);
 			break;
+		}
 
 		skb_mark_for_recycle(head_skb);
 		return head_skb;
@@ -2414,8 +2423,10 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 	head_skb = page_to_skb(vi, rq, page, offset, len, truesize, headroom);
 	curr_skb = head_skb;
 
-	if (unlikely(!curr_skb))
+	if (unlikely(!curr_skb)) {
+		u64_stats_inc(&stats->alloc_fail);
 		goto err_skb;
+	}
 
 	skb_mark_for_recycle(head_skb);
 	while (--num_buf) {
@@ -2444,8 +2455,10 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 		truesize = mergeable_ctx_to_truesize(ctx);
 		curr_skb  = virtnet_skb_append_frag(rq, head_skb, curr_skb, page,
 						    buf, len, truesize);
-		if (!curr_skb)
+		if (!curr_skb) {
+			u64_stats_inc(&stats->alloc_fail);
 			goto err_skb;
+		}
 	}
 
 	ewma_pkt_len_add(&rq->mrg_avg_pkt_len, head_skb->len);
@@ -2928,12 +2941,14 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 
 	u64_stats_set(&stats.packets, packets);
 	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
-		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
+		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
 			/* We need to retry refilling in the next NAPI poll so
 			 * we must return budget to make sure the NAPI is
 			 * repolled.
 			 */
 			packets = budget;
+			u64_stats_inc(&stats.alloc_fail);
+		}
 	}
 
 	u64_stats_update_begin(&rq->stats.syncp);
@@ -2948,6 +2963,7 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 
 	u64_stats_add(&rq->stats.packets, u64_stats_read(&stats.packets));
 	u64_stats_add(&rq->stats.bytes, u64_stats_read(&stats.bytes));
+	u64_stats_add(&rq->stats.alloc_fail, u64_stats_read(&stats.alloc_fail));
 
 	u64_stats_update_end(&rq->stats.syncp);
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] virtio_net: add rx-alloc-fail counter
  2026-08-12  1:22 [PATCH] virtio_net: add rx-alloc-fail counter Longjun Tang
@ 2026-08-12 23:34 ` Jakub Kicinski
  2026-08-13  2:58   ` Lange Tang
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-08-12 23:34 UTC (permalink / raw)
  To: Longjun Tang
  Cc: mst, jasowangio, xuanzhuo, virtualization, netdev, tanglongjun

On Wed, 12 Aug 2026 09:22:54 +0800 Longjun Tang wrote:
> From: tanglongjun <tanglongjun@kylinos.cn>
> 
> Count buffer and skb allocation failures on the rx queue and
> report them via rx-alloc-fail in netdev qstats.

You are not plumbing the new stat into virtnet_get_base_stats()
This may be an entirely reasonable choice if you can't / don't 
want to retain the stats for the entire device (e.g. when queues
get disabled). But please explain the choice in the commit msg.

Please explain your motivation / what made you write this patch.
TBH the patches from KylinOS addresses are often slop so it would
be useful to understand whether your choices here are guided by
production experience, or not.

Last but not least, AI suggests:

> @@ -1917,8 +1919,10 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
>  	}
>  
>  	skb = virtnet_build_skb(buf, buflen, xdp.data - buf, len);
> -	if (unlikely(!skb))
> +	if (unlikely(!skb)) {
> +		u64_stats_inc(&stats->alloc_fail);
>  		goto err;
> +	}
>  
>  	if (metasize)
>  		skb_metadata_set(skb, metasize);
The commit message says the patch counts "buffer and skb allocation
failures on the rx queue", and the uAPI description in
Documentation/netlink/specs/netdev.yaml for rx-alloc-fail says "Number of
times skb or buffer allocation failed on the Rx datapath".  Can several
allocation failure sites on the same datapath still be missed?
In this same function the earlier buffer allocation failure is not counted:
	xdp_page = xdp_linearize_page(dev, rq, &num_buf, page,
				      offset, header_offset,
				      &tlen);
	if (!xdp_page)
		goto err_xdp;
and xdp_linearize_page() returns NULL exactly on allocation failure:
	page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
	if (!page)
		return NULL;
The mergeable XDP path has the same shape in receive_mergeable_xdp():
mergeable_xdp_get_buf() returns NULL when its page_pool_alloc_pages() call
fails, and that goes to err_xdp counting only xdp_drops/drops.
The AF_XDP receive path is not instrumented at all.  xsk_construct_skb(),
called from virtnet_receive_xsk_small() and virtnet_receive_xsk_merge(),
fails like this:
	skb = napi_alloc_skb(&rq->napi, size);
	if (unlikely(!skb)) {
		xsk_buff_free(xdp);
		return NULL;
	}
and xsk_append_merge_buffer() drops the remaining buffers when
napi_alloc_frag() or virtnet_skb_append_frag() fails, again without
touching alloc_fail.
So with an XSK pool bound to a queue, or with an XDP program that needs
linearization, can rx-alloc-fail stay at 0 while memory pressure is
actually causing the drops?
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re:Re: [PATCH] virtio_net: add rx-alloc-fail counter
  2026-08-12 23:34 ` Jakub Kicinski
@ 2026-08-13  2:58   ` Lange Tang
  2026-08-13 15:29     ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Lange Tang @ 2026-08-13  2:58 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: mst@redhat.com, jasowangio, xuanzhuo@linux.alibaba.com,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	Tang Longjun



At 2026-08-13 07:34:27, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Wed, 12 Aug 2026 09:22:54 +0800 Longjun Tang wrote:
>> From: tanglongjun <tanglongjun@kylinos.cn>
>> 
>> Count buffer and skb allocation failures on the rx queue and
>> report them via rx-alloc-fail in netdev qstats.
>
>You are not plumbing the new stat into virtnet_get_base_stats()
>This may be an entirely reasonable choice if you can't / don't 
>want to retain the stats for the entire device (e.g. when queues
>get disabled). But please explain the choice in the commit msg.
>
>Please explain your motivation / what made you write this patch.
>TBH the patches from KylinOS addresses are often slop so it would
>be useful to understand whether your choices here are guided by
>production experience, or not.

In my work, i once found that try_fill_recv returning ENOMEM led to
the queue having no available descriptors. To address this, i wanted
to provide some stats to inform user when it occurs. Fortunately, 
rx-alloc-fail in Documentation/netlink/specs/netdev.yaml fit the bill,

Regarding why i didn't into virtnet_get_base_stats, TBH i didn't give it
much thought. If you think it's necessary ,i can add it in the next version.


>Last but not least, AI suggests:
>
>> @@ -1917,8 +1919,10 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
>>  	}
>>  
>>  	skb = virtnet_build_skb(buf, buflen, xdp.data - buf, len);
>> -	if (unlikely(!skb))
>> +	if (unlikely(!skb)) {
>> +		u64_stats_inc(&stats->alloc_fail);
>>  		goto err;
>> +	}
>>  
>>  	if (metasize)
>>  		skb_metadata_set(skb, metasize);
>The commit message says the patch counts "buffer and skb allocation
>failures on the rx queue", and the uAPI description in
>Documentation/netlink/specs/netdev.yaml for rx-alloc-fail says "Number of
>times skb or buffer allocation failed on the Rx datapath".  Can several
>allocation failure sites on the same datapath still be missed?
>In this same function the earlier buffer allocation failure is not counted:
>	xdp_page = xdp_linearize_page(dev, rq, &num_buf, page,
>				      offset, header_offset,
>				      &tlen);
>	if (!xdp_page)
>		goto err_xdp;
>and xdp_linearize_page() returns NULL exactly on allocation failure:
>	page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
>	if (!page)
>		return NULL;
>The mergeable XDP path has the same shape in receive_mergeable_xdp():
>mergeable_xdp_get_buf() returns NULL when its page_pool_alloc_pages() call
>fails, and that goes to err_xdp counting only xdp_drops/drops.
>The AF_XDP receive path is not instrumented at all.  xsk_construct_skb(),
>called from virtnet_receive_xsk_small() and virtnet_receive_xsk_merge(),
>fails like this:
>	skb = napi_alloc_skb(&rq->napi, size);
>	if (unlikely(!skb)) {
>		xsk_buff_free(xdp);
>		return NULL;
>	}
>and xsk_append_merge_buffer() drops the remaining buffers when
>napi_alloc_frag() or virtnet_skb_append_frag() fails, again without
>touching alloc_fail.
>So with an XSK pool bound to a queue, or with an XDP program that needs
>linearization, can rx-alloc-fail stay at 0 while memory pressure is
>actually causing the drops?
>-- 
>pw-bot: cr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] virtio_net: add rx-alloc-fail counter
  2026-08-13  2:58   ` Lange Tang
@ 2026-08-13 15:29     ` Jakub Kicinski
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-08-13 15:29 UTC (permalink / raw)
  To: Lange Tang
  Cc: mst@redhat.com, jasowangio, xuanzhuo@linux.alibaba.com,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	Tang Longjun

On Thu, 13 Aug 2026 10:58:22 +0800 (CST) Lange Tang wrote:
> Regarding why i didn't into virtnet_get_base_stats, TBH i didn't give it
> much thought. If you think it's necessary ,i can add it in the next version.

It'd be ideal if you were the one doing the thinking if you are the one
sending the patches. Think about it and include what you concluded in
the commit msg for v2.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-13 15:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12  1:22 [PATCH] virtio_net: add rx-alloc-fail counter Longjun Tang
2026-08-12 23:34 ` Jakub Kicinski
2026-08-13  2:58   ` Lange Tang
2026-08-13 15:29     ` Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2026-08-11  9:10 Longjun Tang
2026-08-11 15:40 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox