At 2026-08-13 07:34:27, "Jakub Kicinski" <[email protected]> wrote:
>On Wed, 12 Aug 2026 09:22:54 +0800 Longjun Tang wrote:
>> From: tanglongjun <[email protected]>
>>
>> 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
