[PATCH net v2 0/2] net: do not run out of frags in alloc_skb_with_frags()

[PATCH net v2 0/2] net: do not run out of frags in alloc_skb_with_frags()

[Haishuang Yan]

alloc_skb_with_frags() can fail with -ENOBUFS for a data_len that passed
its own size check: it splits the tail into one frag per bit set in its
page count, and when a high-order allocation fails it lowers the order
without checking that the frags left can still hold the rest. With
vhost-net + tun, a guest that sends a GSO packet above 64K can then make
the vhost worker retry that packet forever (100% CPU, device dead).
Patch 1 fixes both cases; patch 2 adds a KUnit test for the tail case.

The issues were found with the help of an AI coding assistant.

Changes in v2:
- Patch 1: also fix the fallback after a failed high-order allocation:
  only lower the order while the frags left can hold the rest, else
  retry the same order once with direct reclaim (reported by the netdev
  AI review of v1). Changelog and test notes updated, including fault
  injection results.
- Patch 1: reword the comment as suggested by Narcisa, and compute the
  page count with DIV_ROUND_UP() to keep the line within 80 columns.
- Patch 1: say in the changelog that this only fixes the frag accounting;
  larger frames failing in tun_alloc_skb() are a separate problem.
- Patch 2: skip the test instead of failing it when no order-3 page can
  be had, and name the fix by its subject. Dropped Narcisa's Reviewed-by
  since the test changed.

v1: https://lore.kernel.org/netdev/20261108100617.23840-1-yanhaishuang@cmss.chinamobile.com/

Haishuang Yan (2):
  net: do not run out of frags in alloc_skb_with_frags()
  net: add a KUnit test for the alloc_skb_with_frags() tail split

 net/core/net_test.c | 35 +++++++++++++++++++++++++++++++++++
 net/core/skbuff.c   | 30 ++++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 2 deletions(-)


base-commit: a45dbf182137275f44ad7c8bc8acd23f7af6b2cf
-- 
2.43.0

[PATCH net v2 1/2] net: do not run out of frags in alloc_skb_with_frags()

[Haishuang Yan]

alloc_skb_with_frags() rejects data_len above
MAX_SKB_FRAGS * (PAGE_SIZE << order) up front, but two things can then
make it need more than MAX_SKB_FRAGS frags, so it fails with -ENOBUFS
for a data_len that passed the check:

- It rounds the order down for the tail, so a tail of N pages is split
  into one frag per bit set in N. With order 3 and 4K pages, a 516536
  byte packet is 15 * 32K plus a 25016 byte tail, which becomes
  16K + 8K + 4K, so 18 frags are needed although every page allocation
  succeeded. Sizes between about 480K and 540K can be affected.

- When a high-order allocation fails, it lowers the order for this and
  every later chunk without checking that the frags left can still hold
  the rest. With order 3, a failure on the first chunk leaves room for
  at most 17 * 16K, so anything above 278528 bytes fails. These
  allocations are made without direct reclaim, so they fail whenever no
  order-3 page is free at that moment.

Both came with the commit below. Before it, the limit was MAX_SKB_FRAGS
pages, which fit at any order.

tun hits this when vhost-net hands it a GSO packet of such a size from a
guest. Nothing on the host limits the size of a guest's GSO packet to
64K: virtio_net_hdr has no total length, and tun_get_user() sizes the
skb from the descriptor chain. Today's virtio-net drivers stay at 64K,
but a guest can send bigger packets by raising its own TSO limit, and
the host must not depend on the guest for that. Worse, vhost-net treats
-ENOBUFS from sendmsg() as transient: handle_tx_copy() puts the
descriptor back and retries it, so the vhost worker spins at 100% on
the same packet and the device stops passing traffic in both
directions. On an unmodified host, tun_get_user() returned -ENOBUFS
about 620,000 times per second, the tap's rx_dropped counter climbed at
the same rate, and this went on with no traffic until the VM was
destroyed.

This patch only fixes the frag accounting for packets that pass the
data_len check. A larger frame from a guest can still make
tun_alloc_skb() fail in alloc_skb() and vhost-net retry it; that is a
separate problem in tun.

Keep rounding the tail down when the remaining frags allow it, and use
one page rounded up for the whole tail otherwise. This is never larger
than the full-size chunks already used. When a high-order allocation
fails, only lower the order if the frags left can still hold the rest.
If they can't, try the same order once more with the caller's direct
reclaim (with __GFP_NORETRY, so no OOM killer), and fail only if that
fails too. An allocation that succeeds today takes the same path; the
extra attempt only happens where it would have failed.

The tail case was triggered while prototyping >64K TSO for virtio-net,
with a one-line guest virtio_net change raising its TSO limit; the host
was unmodified. The fallback case was pointed out in review of v1. The
fix was tested with that reproducer, with page allocation fault
injection on the vhost worker, and with the KUnit test in the next
patch.

Fixes: 09c2c90705bb ("net: allow alloc_skb_with_frags() to allocate bigger packets")
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
Base: net a45dbf182137 ("ipvlan: make outbound skb headers writable").
The tests below were run on net-next 303d91a15925, where
alloc_skb_with_frags() is identical. KUnit on net 30c71476d842: net_core
20/20 (kunit.py, UML x86_64); with only patch 2 applied, the new test fails
with err -105. allmodconfig W=1 build of net/core/ (arm64) on net
a45dbf182137 with no warnings.

Reproducer: x86_64 KVM host (i7-14700) running net-next 303d91a15925 with no
local changes; vhost-net, a tap NIC with mrg_rxbuf=on and rx_queue_size=1024.
The guest (net-next too) has a local one-line virtio_net change setting
netif_set_tso_max_size(dev, GSO_MAX_SIZE), plus
`ip link set dev ethX gso_max_size 524280 gso_ipv4_max_size 524280`; then
a 10 s iperf3 guest -> host.

Tail case, without this patch: iperf3 fails, one vhost worker stays at
99.9% CPU, tun_get_user() returns -105 (31014 times in 50 ms, kretprobe),
the tap's rx_dropped goes up by 34M during the run and keeps climbing at
~690k/s with no traffic, and the guest can no longer ping the host until
the VM is destroyed. With this patch: ~100 Gbit/s, no drops, and the vhost
worker is idle afterwards; tun_get_user() handled 246783 packets, 237937
of them between 480K and 540K, and none failed.

Fallback case: CONFIG_FAIL_PAGE_ALLOC with min-order=3, ignore-gfp-wait=Y
and task-filter set on the vhost workers, so only the order-3 attempts
without direct reclaim fail. Failures of tun_get_user() during the run:

  probability   tail fix only               this patch
     0%          99.0 Gbit/s, 0              96.0 Gbit/s, 0
     5%          92.7 Gbit/s, 240418         99.8 Gbit/s, 0
    20%          72.3 Gbit/s, 1152801       101.6 Gbit/s, 0
    50%           0.1 Gbit/s, 4077173        99.5 Gbit/s, 0
   100%          iperf3 fails, 10573852      98.9 Gbit/s, 0

With this patch the order-3 pages that the first attempt doesn't get come
from the retry with direct reclaim (3546571 of them at 100%). Not tested:
the retry failing as well, i.e. real memory pressure, and other callers
of alloc_skb_with_frags() than tun.

 net/core/skbuff.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a7d555b3d2f28..da9025e6f0c3c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6906,15 +6906,41 @@ struct sk_buff *alloc_skb_with_frags(unsigned long header_len,
 	while (data_len) {
 		if (nr_frags == MAX_SKB_FRAGS)
 			goto failure;
-		while (order && PAGE_ALIGN(data_len) < (PAGE_SIZE << order))
-			order--;
+		if (order && PAGE_ALIGN(data_len) < (PAGE_SIZE << order)) {
+			unsigned long pages = DIV_ROUND_UP(data_len, PAGE_SIZE);
+
+			/* If splitting the rest into smaller pages would need
+			 * more frags than we have left, use one larger page.
+			 */
+			if (hweight_long(pages) > MAX_SKB_FRAGS - nr_frags)
+				order = order_base_2(pages);
+			else
+				order = ilog2(pages);
+		}
 
 		if (order) {
+			unsigned long smaller = PAGE_SIZE << (order - 1);
+			bool can_shrink;
+
+			/* Only fall back to smaller pages if the frags left
+			 * can still hold the rest. If not, try this order
+			 * once more with the caller's direct reclaim.
+			 */
+			can_shrink = DIV_ROUND_UP(data_len, smaller) <=
+				     MAX_SKB_FRAGS - nr_frags;
+
 			page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) |
 					   __GFP_COMP |
 					   __GFP_NOWARN,
 					   order);
+			if (!page && !can_shrink &&
+			    gfpflags_allow_blocking(gfp_mask))
+				page = alloc_pages(gfp_mask | __GFP_COMP |
+						   __GFP_NOWARN | __GFP_NORETRY,
+						   order);
 			if (!page) {
+				if (!can_shrink)
+					goto failure;
 				order--;
 				continue;
 			}
-- 
2.43.0

Re: [PATCH net v2 1/2] net: do not run out of frags in alloc_skb_with_frags()

[Eric Dumazet]

Le mar. 10 nov. 2026 à 09:31, Haishuang Yan
<yanhaishuang@cmss.chinamobile.com> a écrit :
>
> alloc_skb_with_frags() rejects data_len above
> MAX_SKB_FRAGS * (PAGE_SIZE << order) up front, but two things can then
> make it need more than MAX_SKB_FRAGS frags, so it fails with -ENOBUFS
> for a data_len that passed the check:
>
> - It rounds the order down for the tail, so a tail of N pages is split
>   into one frag per bit set in N. With order 3 and 4K pages, a 516536
>   byte packet is 15 * 32K plus a 25016 byte tail, which becomes
>   16K + 8K + 4K, so 18 frags are needed although every page allocation
>   succeeded. Sizes between about 480K and 540K can be affected.
>
> - When a high-order allocation fails, it lowers the order for this and
>   every later chunk without checking that the frags left can still hold
>   the rest. With order 3, a failure on the first chunk leaves room for
>   at most 17 * 16K, so anything above 278528 bytes fails. These
>   allocations are made without direct reclaim, so they fail whenever no
>   order-3 page is free at that moment.
>
> Both came with the commit below. Before it, the limit was MAX_SKB_FRAGS
> pages, which fit at any order.
>
> tun hits this when vhost-net hands it a GSO packet of such a size from a
> guest. Nothing on the host limits the size of a guest's GSO packet to
> 64K: virtio_net_hdr has no total length, and tun_get_user() sizes the
> skb from the descriptor chain. Today's virtio-net drivers stay at 64K,
> but a guest can send bigger packets by raising its own TSO limit, and
> the host must not depend on the guest for that. Worse, vhost-net treats
> -ENOBUFS from sendmsg() as transient: handle_tx_copy() puts the
> descriptor back and retries it, so the vhost worker spins at 100% on
> the same packet and the device stops passing traffic in both
> directions.

This part is really a vhost-net/tun issue: handle_tx_copy() will spin
the same way on -ENOBUFS/-ENOMEM from real memory pressure, including
when the new direct reclaim attempt fails. Please trim this from the
changelog (or keep one sentence), this patch is about frag accounting
in alloc_skb_with_frags(), the retry loop deserves its own fix.


Also, this changelog is far too long. Ask your LLM to reduce it.

 On an unmodified host, tun_get_user() returned -ENOBUFS
> about 620,000 times per second, the tap's rx_dropped counter climbed at
> the same rate, and this went on with no traffic until the VM was
> destroyed.
>
> This patch only fixes the frag accounting for packets that pass the
> data_len check. A larger frame from a guest can still make
> tun_alloc_skb() fail in alloc_skb() and vhost-net retry it; that is a
> separate problem in tun.
>
> Keep rounding the tail down when the remaining frags allow it, and use
> one page rounded up for the whole tail otherwise. This is never larger
> than the full-size chunks already used. When a high-order allocation
> fails, only lower the order if the frags left can still hold the rest.
> If they can't, try the same order once more with the caller's direct
> reclaim (with __GFP_NORETRY, so no OOM killer), and fail only if that
> fails too. An allocation that succeeds today takes the same path; the
> extra attempt only happens where it would have failed.
>
> The tail case was triggered while prototyping >64K TSO for virtio-net,
> with a one-line guest virtio_net change raising its TSO limit; the host
> was unmodified. The fallback case was pointed out in review of v1. The
> fix was tested with that reproducer, with page allocation fault
> injection on the vhost worker, and with the KUnit test in the next
> patch.
>
> Fixes: 09c2c90705bb ("net: allow alloc_skb_with_frags() to allocate bigger packets")
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> ---
> Base: net a45dbf182137 ("ipvlan: make outbound skb headers writable").
> The tests below were run on net-next 303d91a15925, where
> alloc_skb_with_frags() is identical. KUnit on net 30c71476d842: net_core
> 20/20 (kunit.py, UML x86_64); with only patch 2 applied, the new test fails
> with err -105. allmodconfig W=1 build of net/core/ (arm64) on net
> a45dbf182137 with no warnings.
>
> Reproducer: x86_64 KVM host (i7-14700) running net-next 303d91a15925 with no
> local changes; vhost-net, a tap NIC with mrg_rxbuf=on and rx_queue_size=1024.
> The guest (net-next too) has a local one-line virtio_net change setting
> netif_set_tso_max_size(dev, GSO_MAX_SIZE), plus
> `ip link set dev ethX gso_max_size 524280 gso_ipv4_max_size 524280`; then
> a 10 s iperf3 guest -> host.
>
> Tail case, without this patch: iperf3 fails, one vhost worker stays at
> 99.9% CPU, tun_get_user() returns -105 (31014 times in 50 ms, kretprobe),
> the tap's rx_dropped goes up by 34M during the run and keeps climbing at
> ~690k/s with no traffic, and the guest can no longer ping the host until
> the VM is destroyed. With this patch: ~100 Gbit/s, no drops, and the vhost
> worker is idle afterwards; tun_get_user() handled 246783 packets, 237937
> of them between 480K and 540K, and none failed.
>
> Fallback case: CONFIG_FAIL_PAGE_ALLOC with min-order=3, ignore-gfp-wait=Y
> and task-filter set on the vhost workers, so only the order-3 attempts
> without direct reclaim fail. Failures of tun_get_user() during the run:
>
>   probability   tail fix only               this patch
>      0%          99.0 Gbit/s, 0              96.0 Gbit/s, 0
>      5%          92.7 Gbit/s, 240418         99.8 Gbit/s, 0
>     20%          72.3 Gbit/s, 1152801       101.6 Gbit/s, 0
>     50%           0.1 Gbit/s, 4077173        99.5 Gbit/s, 0
>    100%          iperf3 fails, 10573852      98.9 Gbit/s, 0
>
> With this patch the order-3 pages that the first attempt doesn't get come
> from the retry with direct reclaim (3546571 of them at 100%). Not tested:
> the retry failing as well, i.e. real memory pressure, and other callers
> of alloc_skb_with_frags() than tun.
>
>  net/core/skbuff.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index a7d555b3d2f28..da9025e6f0c3c 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6906,15 +6906,41 @@ struct sk_buff *alloc_skb_with_frags(unsigned long header_len,
>         while (data_len) {
>                 if (nr_frags == MAX_SKB_FRAGS)
>                         goto failure;
> -               while (order && PAGE_ALIGN(data_len) < (PAGE_SIZE << order))
> -                       order--;
> +               if (order && PAGE_ALIGN(data_len) < (PAGE_SIZE << order)) {
> +                       unsigned long pages = DIV_ROUND_UP(data_len, PAGE_SIZE);
> +
> +                       /* If splitting the rest into smaller pages would need
> +                        * more frags than we have left, use one larger page.
> +                        */
> +                       if (hweight_long(pages) > MAX_SKB_FRAGS - nr_frags)
> +                               order = order_base_2(pages);
> +                       else
> +                               order = ilog2(pages);
> +               }
>
>                 if (order) {
> +                       unsigned long smaller = PAGE_SIZE << (order - 1);
> +                       bool can_shrink;
> +
> +                       /* Only fall back to smaller pages if the frags left
> +                        * can still hold the rest. If not, try this order
> +                        * once more with the caller's direct reclaim.
> +                        */
> +                       can_shrink = DIV_ROUND_UP(data_len, smaller) <=
> +                                    MAX_SKB_FRAGS - nr_frags;
> +
>                         page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) |
>                                            __GFP_COMP |
>                                            __GFP_NOWARN,
>                                            order);
> +                       if (!page && !can_shrink &&
> +                           gfpflags_allow_blocking(gfp_mask))
> +                               page = alloc_pages(gfp_mask | __GFP_COMP |
> +                                                  __GFP_NOWARN | __GFP_NORETRY,
> +                                                  order);
>                         if (!page) {
> +                               if (!can_shrink)
> +                                       goto failure;
>                                 order--;
>                                 continue;
>                         }

can_shrink is only needed when the first alloc_pages() failed, but you
compute it for every chunk, with a 64bit divide since the divisor is
not a constant.

Please move all this in the slow path, and use a shift. Something like
(untested):


                if (order) {
                        page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) |
                                           __GFP_COMP |
                                           __GFP_NOWARN,
                                           order);
                        if (unlikely(!page)) {
                                unsigned int shift = PAGE_SHIFT + order - 1;

                                /* Can smaller pages hold the rest ? */
                                if (((data_len - 1) >> shift) + 1 <=
                                    MAX_SKB_FRAGS - nr_frags) {
                                        order--;
                                        continue;
                                }
                                if (!gfpflags_allow_blocking(gfp_mask))
                                        goto failure;
                                page = alloc_pages(gfp_mask | __GFP_COMP |
                                                   __GFP_NOWARN | __GFP_NORETRY,
                                                   order);
                                if (!page)
                                        goto failure;
                        }
                } else {

I find this also easier to follow than the
(!page && !can_shrink && ...) sequence.

The tail part looks good to me.

Re: [PATCH net v2 1/2] net: do not run out of frags in alloc_skb_with_frags()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] alloc_skb_with_frags() (net/core/skbuff.c) can still fail with
  -ENOBUFS for a data_len that passed its up-front check, and the vhost-…

--

net: do not run out of frags in alloc_skb_with_frags()

This patch changes alloc_skb_with_frags() so that it no longer runs out
of frags when it rounds down the tail or falls back to a lower order.
When lowering the order would need more frags than are left, it retries
the same order once with the caller's direct reclaim and __GFP_NORETRY,
and fails if that retry also fails.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index a7d555b3d2f28..da9025e6f0c3c 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6906,15 +6906,41 @@ struct sk_buff *alloc_skb_with_frags(unsigned long header_len,

[ ... ]

>  		if (order) {
> +			unsigned long smaller = PAGE_SIZE << (order - 1);
> +			bool can_shrink;
> +
> +			/* Only fall back to smaller pages if the frags left
> +			 * can still hold the rest. If not, try this order
> +			 * once more with the caller's direct reclaim.
> +			 */
> +			can_shrink = DIV_ROUND_UP(data_len, smaller) <=
> +				     MAX_SKB_FRAGS - nr_frags;
> +
>  			page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) |
>  					   __GFP_COMP |
>  					   __GFP_NOWARN,
>  					   order);
> +			if (!page && !can_shrink &&
> +			    gfpflags_allow_blocking(gfp_mask))
> +				page = alloc_pages(gfp_mask | __GFP_COMP |
> +						   __GFP_NOWARN | __GFP_NORETRY,
> +						   order);
>  			if (!page) {
> +				if (!can_shrink)
> +					goto failure;

[Severity: Medium]
Can a guest still livelock the vhost worker through this path?

can_shrink is false for any data_len above
(MAX_SKB_FRAGS - nr_frags) * (PAGE_SIZE << (order - 1)). For the first
chunk with order 3 and 4K pages, that is anything above 278528 bytes.
If the attempt without direct reclaim and the __GFP_NORETRY retry both
fail, alloc_skb_with_frags() still returns -ENOBUFS. The data_len passed
the up-front check, and order-0 pages may be plentiful.

A guest can reach this size. tun_alloc_skb() clamps the paged part to
exactly the limit:

drivers/net/tun.c:tun_alloc_skb() {
    ...
	if (len - linear > MAX_SKB_FRAGS * (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
		linear = len - MAX_SKB_FRAGS * (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER);
    ...
}

So the guest can choose a paged length of up to 557056 bytes through the
descriptor chain. Tap and af_packet clamp to the same value.

vhost-net still treats -ENOBUFS as transient:

drivers/vhost/net.c:handle_tx_copy() {
    ...
			if (err == -EAGAIN || err == -ENOMEM || err == -ENOBUFS) {
				vhost_discard_vq_desc(vq, 1, ndesc);
				vhost_net_enable_vq(net, vq);
				break;
			}
    ...
}

handle_tx_zerocopy() also retries on -ENOBUFS.

On a host with fragmented memory, wouldn't this give the same 100% CPU
vhost spin with no traffic that the commit message describes?

Previously the order-3 attempt failed fast. Now every pass also does the
order-3 GFP_KERNEL | __GFP_NORETRY allocation. For this request,
__alloc_pages_slowpath() runs async compaction, one direct reclaim pass
and direct compaction before it gives up. Each pass can evict host page
cache and slab, and the guest controls how often this happens.

This seems to partly undo fb05e7a89f50 ("net: don't wait for order-3
page allocation").

The commit message only says "fail only if that fails too". It does not
say that this failure still livelocks vhost-net, or that the vhost retry
loop now runs direct reclaim. The note that the retry-failure case was
not tested appears only below the --- line, so it will not be in the
commit.

The KUnit test in the next patch does not change net/core/skbuff.c, so
this case is still there at the end of the series.

Could the commit message describe this remaining failure case?

>  				order--;
>  				continue;
>  			}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110162418.28322-1-yanhaishuang%40cmss.chinamobile.com

[PATCH net v2 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split

[Haishuang Yan]

Allocate MAX_SKB_FRAGS - 2 full chunks of PAGE_ALLOC_COSTLY_ORDER plus
a 7 page tail. Rounding the tail down to 4 + 2 + 1 pages would need
one frag more than MAX_SKB_FRAGS, which made the allocation fail with
-ENOBUFS before "net: do not run out of frags in
alloc_skb_with_frags()".

If the allocation fails and no order-3 page can be had either, skip the
test instead of failing it, so that a host with fragmented memory does
not report this as a regression.

Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
 net/core/net_test.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/net/core/net_test.c b/net/core/net_test.c
index 2b5080c226636..de4d246f2f7ab 100644
--- a/net/core/net_test.c
+++ b/net/core/net_test.c
@@ -1721,7 +1721,42 @@ static void netmem_ref_test_tcp_devmem_recv(struct kunit *test)
 #endif
 #endif
 
+static void alloc_skb_with_frags_test_tail(struct kunit *test)
+{
+	/* MAX_SKB_FRAGS - 2 full chunks plus a 7 page tail: rounding the
+	 * tail down to 4 + 2 + 1 pages would need one frag too many.
+	 */
+	unsigned long len = (MAX_SKB_FRAGS - 2) *
+			    (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER) +
+			    7 * PAGE_SIZE;
+	unsigned long frags_len = 0;
+	struct sk_buff *skb;
+	int err, i;
+
+	skb = alloc_skb_with_frags(0, len, PAGE_ALLOC_COSTLY_ORDER, &err,
+				   GFP_KERNEL);
+	if (!skb) {
+		struct page *page;
+
+		/* Don't report a lack of high-order pages as a failure. */
+		page = alloc_pages(GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY,
+				   PAGE_ALLOC_COSTLY_ORDER);
+		if (!page)
+			kunit_skip(test, "no order-%d pages",
+				   PAGE_ALLOC_COSTLY_ORDER);
+		__free_pages(page, PAGE_ALLOC_COSTLY_ORDER);
+	}
+	KUNIT_ASSERT_NOT_NULL_MSG(test, skb, "err %d", err);
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
+		frags_len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
+	KUNIT_EXPECT_EQ(test, frags_len, len);
+
+	kfree_skb(skb);
+}
+
 static struct kunit_case net_test_cases[] = {
+	KUNIT_CASE(alloc_skb_with_frags_test_tail),
 	KUNIT_CASE_PARAM(gso_test_func, gso_test_gen_params),
 	KUNIT_CASE_PARAM(ip_tunnel_flags_test_run,
 			 ip_tunnel_flags_test_gen_params),
-- 
2.43.0

Re: [PATCH net v2 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The skip probe in alloc_skb_with_frags_test_tail()
  (net/core/net_test.c) cannot do what its comment says ("Don't report a…

--

net: add a KUnit test for the alloc_skb_with_frags() tail split

This adds a KUnit case that asks alloc_skb_with_frags() for
MAX_SKB_FRAGS - 2 order-3 chunks plus a 7 page tail. Before the fix it
references, splitting that tail ran out of frags. If the allocation fails
and a single order-3 page can't be allocated either, the test is meant to
skip rather than fail.

> diff --git a/net/core/net_test.c b/net/core/net_test.c
> index 2b5080c226636..de4d246f2f7ab 100644
> --- a/net/core/net_test.c
> +++ b/net/core/net_test.c
> @@ -1721,7 +1721,42 @@ static void netmem_ref_test_tcp_devmem_recv(struct kunit *test)

[ ... ]

> +	skb = alloc_skb_with_frags(0, len, PAGE_ALLOC_COSTLY_ORDER, &err,
> +				   GFP_KERNEL);
> +	if (!skb) {
> +		struct page *page;
> +
> +		/* Don't report a lack of high-order pages as a failure. */
> +		page = alloc_pages(GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY,
> +				   PAGE_ALLOC_COSTLY_ORDER);
> +		if (!page)
> +			kunit_skip(test, "no order-%d pages",
> +				   PAGE_ALLOC_COSTLY_ORDER);
> +		__free_pages(page, PAGE_ALLOC_COSTLY_ORDER);
> +	}
> +	KUNIT_ASSERT_NOT_NULL_MSG(test, skb, "err %d", err);

[Severity: Medium]
Does probing for a single order-3 page catch the case the comment
describes?

For this len, alloc_skb_with_frags() never lets a leading chunk drop
below order 3. At nr_frags = k, the check is:

net/core/skbuff.c:alloc_skb_with_frags() {
    ...
			can_shrink = DIV_ROUND_UP(data_len, smaller) <=
				     MAX_SKB_FRAGS - nr_frags;
    ...
}

This works out to 2*MAX_SKB_FRAGS - 2 - 2k <= MAX_SKB_FRAGS - k. That
holds only once k >= MAX_SKB_FRAGS - 2. So each of the first
MAX_SKB_FRAGS - 2 chunks has to get an order-3 page, or the function
does goto failure. In total the call needs roughly MAX_SKB_FRAGS - 1
order-3 pages, not one.

When the call fails partway through, the failure path does:

failure:
	kfree_skb(skb);
	return NULL;

This frees every order-3 page already placed in the frags. Since
pcp_allowed_order(3) is true, those pages land on this CPU's per-CPU
free lists right before the probe runs. The single
alloc_pages(..., PAGE_ALLOC_COSTLY_ORDER) probe will then almost always
succeed, the skip is not taken, and KUNIT_ASSERT_NOT_NULL_MSG() reports
a failure.

So the test seems to skip only when the very first order-3 allocation
fails. A host with some order-3 pages, but fewer than MAX_SKB_FRAGS - 1,
would still report a failure. That doesn't match "so that a host with
fragmented memory does not report this as a regression" in the commit
message.

Could the probe check the real requirement instead? For example, it
could try to hold MAX_SKB_FRAGS - 1 order-3 pages at once, or treat
-ENOBUFS as a skip unless a stronger check shows enough pages were
available.

> +
> +	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
> +		frags_len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
> +	KUNIT_EXPECT_EQ(test, frags_len, len);
> +
> +	kfree_skb(skb);
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110162418.28322-1-yanhaishuang%40cmss.chinamobile.com