* [PATCH net 0/2] net: do not run out of frags in alloc_skb_with_frags() @ 2026-11-08 10:06 Haishuang Yan 2026-11-08 10:06 ` [PATCH net 1/2] " Haishuang Yan 2026-11-08 10:06 ` [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split Haishuang Yan 0 siblings, 2 replies; 8+ messages in thread From: Haishuang Yan @ 2026-11-08 10:06 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel, Haishuang Yan alloc_skb_with_frags() can fail with -ENOBUFS for a data_len that passed its own size check, because it splits the tail into one frag per bit set in its page count. With vhost-net + tun, a guest that sends a ~480-540K GSO packet makes the vhost worker retry that packet forever (100% CPU, device dead). Patch 1 fixes the split without changing any allocation that works today; patch 2 adds a KUnit test that fails before patch 1. 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 | 24 ++++++++++++++++++++++++ net/core/skbuff.c | 14 ++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) base-commit: 30c71476d842b094651bd7712f6c8ee8716147d0 -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 1/2] net: do not run out of frags in alloc_skb_with_frags() 2026-11-08 10:06 [PATCH net 0/2] net: do not run out of frags in alloc_skb_with_frags() Haishuang Yan @ 2026-11-08 10:06 ` Haishuang Yan 2026-11-08 8:20 ` Narcisa Vasile 2026-11-09 2:07 ` netdev-bot+sashiko 2026-11-08 10:06 ` [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split Haishuang Yan 1 sibling, 2 replies; 8+ messages in thread From: Haishuang Yan @ 2026-11-08 10:06 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel, Haishuang Yan alloc_skb_with_frags() rejects data_len above MAX_SKB_FRAGS * (PAGE_SIZE << order) up front, but then rounds the order down for the tail, so a tail of N pages is split into one frag per bit set in N. A data_len that passed the check can then need more than MAX_SKB_FRAGS frags, and the allocation fails with -ENOBUFS although every page allocation succeeded. 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. Sizes between about 480K and 540K can be affected. 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. Keep rounding the tail down when the remaining frags allow it, so no allocation that works today changes, and use one page rounded up for the whole tail otherwise. This is never larger than the full-size chunks already used. This 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 fix was tested with that reproducer 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 30c71476d842 ("ipv6: Check the Home Address option again before its address is used"). The reproducer below was run on net-next 303d91a15925, where alloc_skb_with_frags() is identical. On net: KUnit net_core 20/20 (kunit.py, UML x86_64), and allmodconfig W=1 build of net/core/ (arm64) 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 iperf3 guest -> host. 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 (same setup plus this patch on the host): 81 Gbit/s, no drops, and the vhost worker is idle afterwards. net/core/skbuff.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 8da53d5b76fc..93e7b04ddfe6 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -6902,8 +6902,18 @@ 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 = PAGE_ALIGN(data_len) >> PAGE_SHIFT; + + /* Rounding the tail down to smaller pages takes one + * frag per bit set in its page count. If not enough + * frags are left for that, use one bigger page. + */ + if (hweight_long(pages) > MAX_SKB_FRAGS - nr_frags) + order = order_base_2(pages); + else + order = ilog2(pages); + } if (order) { page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) | -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] net: do not run out of frags in alloc_skb_with_frags() 2026-11-08 10:06 ` [PATCH net 1/2] " Haishuang Yan @ 2026-11-08 8:20 ` Narcisa Vasile 2026-11-08 18:09 ` Haishuang Yan 2026-11-09 2:07 ` netdev-bot+sashiko 1 sibling, 1 reply; 8+ messages in thread From: Narcisa Vasile @ 2026-11-08 8:20 UTC (permalink / raw) To: Haishuang Yan Cc: netdev, davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel On Sun, Nov 08, 2026 at 07:06:16PM +0900, Haishuang Yan wrote: > alloc_skb_with_frags() rejects data_len above > MAX_SKB_FRAGS * (PAGE_SIZE << order) up front, but then rounds the order > down for the tail, so a tail of N pages is split into one frag per bit > set in N. A data_len that passed the check can then need more than > MAX_SKB_FRAGS frags, and the allocation fails with -ENOBUFS although > every page allocation succeeded. > > 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. Sizes > between about 480K and 540K can be affected. > > 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. > > Keep rounding the tail down when the remaining frags allow it, so no > allocation that works today changes, and use one page rounded up for > the whole tail otherwise. This is never larger than the full-size > chunks already used. > > This 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 fix was tested with that reproducer 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 30c71476d842 ("ipv6: Check the Home Address option again before its > address is used"). The reproducer below was run on net-next 303d91a15925, > where alloc_skb_with_frags() is identical. On net: KUnit net_core 20/20 > (kunit.py, UML x86_64), and allmodconfig W=1 build of net/core/ (arm64) > 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 > iperf3 guest -> host. > > 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 (same setup plus this patch on the host): 81 > Gbit/s, no drops, and the vhost worker is idle afterwards. > > net/core/skbuff.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > I think we should decouple the two parts you've pointed out: 1. For valid packets that pass the initial data_len size check, this patch avoids running out of frags by putting the tail into one larger block of pages instead of splitting it into several smaller blocks. 2. The vhost cpu spin: this patch fixes it for the sizes in (1), but a modified guest can still trigger it by sending a larger frame (above ~4.75 MB on x86). In that case, tun_alloc_skb() asks for a linear part above the kmalloc limit. The alloc_skb() allocation in this function can never succeed and returns -ENOBUFS, and vhost-net retries the same frame indefinitely. Should we also return -EMSGSIZE when header_len is above the kmalloc limit, like it already does for an oversized data_len? I'm not sure whether the errno change is acceptable?. > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 8da53d5b76fc..93e7b04ddfe6 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -6902,8 +6902,18 @@ 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 = PAGE_ALIGN(data_len) >> PAGE_SHIFT; > + > + /* Rounding the tail down to smaller pages takes one > + * frag per bit set in its page count. If not enough > + * frags are left for that, use one bigger page. > + */ nit, feel free to ignore, this might read more simply as: /* 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) { > page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] net: do not run out of frags in alloc_skb_with_frags() 2026-11-08 8:20 ` Narcisa Vasile @ 2026-11-08 18:09 ` Haishuang Yan 0 siblings, 0 replies; 8+ messages in thread From: Haishuang Yan @ 2026-11-08 18:09 UTC (permalink / raw) To: Narcisa Vasile Cc: Haishuang Yan, netdev, davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel On Sun, Nov 08, 2026 at 12:20:11AM -0800, Narcisa Vasile wrote: > I think we should decouple the two parts you've pointed out: [...] > Should we also return -EMSGSIZE when header_len > is above the kmalloc limit, like it already does for an oversized > data_len? I'm not sure whether the errno change is acceptable?. Thanks for the review. Agreed, these are two separate problems. I'll keep this patch to the frag accounting, and reword the changelog in v2 so it doesn't suggest that it fixes the vhost spin in general. Right, for larger frames tun_alloc_skb() moves everything above MAX_SKB_FRAGS << PAGE_ALLOC_COSTLY_ORDER into the linear part, so once that is above the kmalloc limit alloc_skb() can never succeed. I'll look at that one separately in tun. > nit, feel free to ignore, this might read more simply as: > > /* If splitting the rest into smaller pages would need > * more frags than we have left, use one larger page. > */ Will take it in v2, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] net: do not run out of frags in alloc_skb_with_frags() 2026-11-08 10:06 ` [PATCH net 1/2] " Haishuang Yan 2026-11-08 8:20 ` Narcisa Vasile @ 2026-11-09 2:07 ` netdev-bot+sashiko 1 sibling, 0 replies; 8+ messages in thread From: netdev-bot+sashiko @ 2026-11-09 2:07 UTC (permalink / raw) To: yanhaishuang Cc: netdev, davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 0 · 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). - [High] The patch fixes the run-out-of-frags case in alloc_skb_with_frags() (net/core/skbuff.c) only when every high-order… -- net: do not run out of frags in alloc_skb_with_frags() This patch changes how alloc_skb_with_frags() picks the order for the tail of a paged skb. When splitting the tail into smaller pages would need more frags than are left, it uses one rounded-up page for the whole tail instead. The aim is that a data_len that passes the up-front size check can no longer fail with -ENOBUFS, which vhost-net retries without end. > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 8da53d5b76fc2..93e7b04ddfe63 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -6902,8 +6902,18 @@ 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 = PAGE_ALIGN(data_len) >> PAGE_SHIFT; > + > + /* Rounding the tail down to smaller pages takes one > + * frag per bit set in its page count. If not enough > + * frags are left for that, use one bigger page. > + */ > + if (hweight_long(pages) > MAX_SKB_FRAGS - nr_frags) > + order = order_base_2(pages); > + else > + order = ilog2(pages); > + } > > if (order) { > page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) | [Severity: High] Can alloc_skb_with_frags() still run out of frags when a high-order allocation fails? The fallback just below this hunk has not changed: net/core/skbuff.c:alloc_skb_with_frags() { ... if (!page) { order--; continue; } ... } This lowers order for the current chunk and for every chunk after it. Nothing checks that the MAX_SKB_FRAGS - nr_frags frags left can still hold the rest of data_len at the lower order. The up-front check assumes the caller's order is used for the whole loop: if (unlikely(data_len > MAX_SKB_FRAGS * (PAGE_SIZE << order))) Take CONFIG_MAX_SKB_FRAGS=17, 4K pages and the order 3 that tun_alloc_skb() passes. data_len can then be up to 17 * 32K = 557056 bytes. If the first order-3 allocation fails, every later full chunk is 16K, and 17 * 16K is only 278528 bytes. Wouldn't any data_len above 278528 then reach nr_frags == MAX_SKB_FRAGS and return -ENOBUFS? The new tail logic doesn't cover this case. It only runs when PAGE_ALIGN(data_len) < (PAGE_SIZE << order), and that is false for the remaining full-size data. The tail logic also relies on its own bigger allocation succeeding. Take the 516536 byte example from the commit message: - 15 order-3 frags are filled. - The tail is 7 pages, hweight 3, with only 2 frags left. - The code picks order_base_2(7) = 3. If the order-3 attempt and then the order-2 attempt both fail, the tail is filled with 8K chunks and the loop runs out of frags again. These high-order attempts use gfp_mask & ~__GFP_DIRECT_RECLAIM with __GFP_NOWARN. They fail whenever no order-3 pages are free at that moment, which is common on fragmented hosts. handle_tx_copy() still treats the result 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; } ... } Each retry starts again at order 3 without direct reclaim. When host high-order pages are scarce, can a guest that raises its own TSO limit still cause the vhost worker spin and the device stall described in the commit message? The KUnit patch later in the series doesn't touch net/core/skbuff.c, drivers/net/tun.c or drivers/vhost/net.c. So this case seems to stay open at the end of the series. Would it work to treat the frag budget as a hard limit on every order reduction? For example, lower the order only when this still covers data_len: (MAX_SKB_FRAGS - nr_frags) * (PAGE_SIZE << (order - 1)) If it doesn't, the code could retry with direct reclaim, fail with an error vhost doesn't retry, or have tun_alloc_skb() put more data in the linear part. -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261108100617.23840-1-yanhaishuang%40cmss.chinamobile.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split 2026-11-08 10:06 [PATCH net 0/2] net: do not run out of frags in alloc_skb_with_frags() Haishuang Yan 2026-11-08 10:06 ` [PATCH net 1/2] " Haishuang Yan @ 2026-11-08 10:06 ` Haishuang Yan 2026-11-08 8:24 ` Narcisa Vasile 2026-11-09 2:07 ` netdev-bot+sashiko 1 sibling, 2 replies; 8+ messages in thread From: Haishuang Yan @ 2026-11-08 10:06 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel, 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 the previous patch. Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com> --- net/core/net_test.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/net/core/net_test.c b/net/core/net_test.c index 2b5080c22663..5a451aaf37c9 100644 --- a/net/core/net_test.c +++ b/net/core/net_test.c @@ -1721,7 +1721,31 @@ 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); + 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split 2026-11-08 10:06 ` [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split Haishuang Yan @ 2026-11-08 8:24 ` Narcisa Vasile 2026-11-09 2:07 ` netdev-bot+sashiko 1 sibling, 0 replies; 8+ messages in thread From: Narcisa Vasile @ 2026-11-08 8:24 UTC (permalink / raw) To: Haishuang Yan Cc: netdev, davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel On Sun, Nov 08, 2026 at 07:06:17PM +0900, Haishuang Yan wrote: > 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 the previous patch. > > Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com> > --- > net/core/net_test.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/net/core/net_test.c b/net/core/net_test.c Thanks, I tried it locally. Reviewed-by: Narcisa Vasile <narcisav.kernel@gmail.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split 2026-11-08 10:06 ` [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split Haishuang Yan 2026-11-08 8:24 ` Narcisa Vasile @ 2026-11-09 2:07 ` netdev-bot+sashiko 1 sibling, 0 replies; 8+ messages in thread From: netdev-bot+sashiko @ 2026-11-09 2:07 UTC (permalink / raw) To: yanhaishuang Cc: netdev, davem, edumazet, kuba, pabeni, horms, willemb, mst, jasowangio, eperezma, willemdebruijn.kernel, linux-kernel Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 1 · Low: 1 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 new test alloc_skb_with_frags_test_tail() in net/core/net_test.c only passes if every full-size high-order… - [Low] The commit message of "net: add a KUnit test for the alloc_skb_with_frags() tail split" says the allocation failed with… -- net: add a KUnit test for the alloc_skb_with_frags() tail split This commit adds a KUnit case, alloc_skb_with_frags_test_tail(), to net/core/net_test.c. The test asks alloc_skb_with_frags() for MAX_SKB_FRAGS - 2 full PAGE_ALLOC_COSTLY_ORDER chunks plus a 7 page tail. It then asserts that the allocation succeeds and that the frag sizes add up to the requested length. > 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 the previous patch. [Severity: Low] This isn't a bug, but could the commit message name the fix this test depends on, instead of saying "the previous patch"? Once the series is merged, "the previous patch" no longer points to anything in git history or in stable trees. The test fails on any tree that lacks "net: do not run out of frags in alloc_skb_with_frags()". Nothing in this commit tells a backporter or AUTOSEL about that dependency. Would it help to reference that commit by its subject (or sha once it is merged)? > diff --git a/net/core/net_test.c b/net/core/net_test.c > index 2b5080c226636..5a451aaf37c9b 100644 > --- a/net/core/net_test.c > +++ b/net/core/net_test.c > @@ -1721,7 +1721,31 @@ 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); > + KUNIT_ASSERT_NOT_NULL_MSG(test, skb, "err %d", err); [Severity: Medium] Can this assertion fail on a system with fragmented memory, even with the fix applied? For order > 0, alloc_skb_with_frags() in net/core/skbuff.c clears __GFP_DIRECT_RECLAIM. A failed order-3 allocation then drops the order for good: net/core/skbuff.c:alloc_skb_with_frags() { ... if (nr_frags == MAX_SKB_FRAGS) goto failure; ... if (order) { page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) | __GFP_COMP | __GFP_NOWARN, order); if (!page) { order--; continue; } ... } The tail branch never raises the order again, because order_base_2(pages) and ilog2(pages) are never above the current order. This length needs MAX_SKB_FRAGS - 1 frags at order 3, which leaves one frag spare. Take MAX_SKB_FRAGS = 17 with 4K pages: len = 15 * 32K + 28K 14 order-3 chunks succeed, then the 15th fails -> order 2 60K left, three 16K frags -> nr_frags == 17, 12K still left goto failure -> NULL, err = -ENOBUFS So a failure of any of the full-size order-3 chunks leaves too few frags for the rest of the data. Only a failure of the final tail allocation can recover, through the hweight_long()/order_base_2() branch. When this happens, the assertion prints "err -105". That is the same symptom the commit message gives for the unfixed kernel, so a failure caused by fragmentation looks like the regression the test is meant to catch. NET_TEST is tristate, so the test can be loaded with modprobe on a long-running host. With 64K pages, each full chunk is 512K of contiguous memory. Would it be more reliable to call kunit_skip() when the order-3 allocations cannot be satisfied? Another option is to build the skb so the page sizes are fixed, rather than asserting that the allocation always succeeds. > + > + 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/20261108100617.23840-1-yanhaishuang%40cmss.chinamobile.com ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-11-09 2:07 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-08 10:06 [PATCH net 0/2] net: do not run out of frags in alloc_skb_with_frags() Haishuang Yan 2026-11-08 10:06 ` [PATCH net 1/2] " Haishuang Yan 2026-11-08 8:20 ` Narcisa Vasile 2026-11-08 18:09 ` Haishuang Yan 2026-11-09 2:07 ` netdev-bot+sashiko 2026-11-08 10:06 ` [PATCH net 2/2] net: add a KUnit test for the alloc_skb_with_frags() tail split Haishuang Yan 2026-11-08 8:24 ` Narcisa Vasile 2026-11-09 2:07 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®