* [PATCH net v3] netpoll: bound the deferred transmit queue
@ 2026-09-23 18:11 Zack Gomez
2026-09-25 15:32 ` Breno Leitao
2026-09-27 18:01 ` netdev-bot+sashiko
0 siblings, 2 replies; 4+ messages in thread
From: Zack Gomez @ 2026-09-23 18:11 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni
Cc: horms, leitao, stephen, netdev, linux-kernel, Zack Gomez
__netpoll_send_skb() parks an skb on npinfo->txq whenever the device
cannot take it at once, and once the queue is non-empty every later skb
goes straight there to keep ordering. queue_process() drains it from a
workqueue and, unlike the direct path, never polls the device for
completions: when the ring is stopped it backs off HZ/10. Nothing limits
the queue length.
A producer that outruns that drain therefore grows the queue until the
host is out of memory. Observed with netconsole forwarding a GPU driver
that logged one line at ~1e5/s after a firmware hang. The NIC was
moving ~17k packets/s: completions for each burst surfaced tens of ms
later, outside the one-tick window, so queue_process() slept HZ/10 per
ring while ~1e5 lines/s kept arriving. The queue grew at ~170 MB/s,
unreclaimable slab reached 51 GiB in five minutes and the OOM killer
ran from kswapd with 341 MiB of anonymous memory on the whole box. What
the queue held was the flood itself; the OOM report never left the
host.
Reproduced on the same host (netconsole over a 10G ConnectX-4 Lx) under
the same slow-completion condition: 200k lines to /dev/kmsg in 0.12 s
grew unreclaimable slab by 173 MiB, about 188k skbs, draining at
~8-10k packets/s. With prompt completions the same burst drains at line
rate; a stall on the link while lines keep arriving faster than the
drain reproduces the growth.
Until the 2006 netpoll rework [1] the deferred path drained through
dev_queue_xmit(), with the stack's own backpressure, and was capped at
16 skbs (MAX_QUEUE_DEPTH). That series moved it to a direct
hard_start_xmit() with the HZ/10 back-off and made the queue
per-device, dropping the cap on the way.
Cap it at 1024 skbs per device and drop new skbs beyond that. The drop
is counted in tx_dropped of the device whose queue is full and freed
with SKB_DROP_REASON_FULL_RING. A netconsole target bound directly to
that device also gets NET_XMIT_DROP and, with CONFIG_NETCONSOLE_DYNAMIC,
counts it in xmit_drop_count. When a stacked device (bond, bridge,
team, vlan, macvlan) passes the skb down and the lower device's queue
is the one that fills, the return value does not reach netconsole and
the lower device's tx_dropped is the record. The bound holds either
way. Nothing is logged on the drop path because that would recurse
into the console being drained.
[1] https://lore.kernel.org/netdev/20061026225645.482978803@osdl.org/
Fixes: b6cd27ed3388 ("netpoll per device txq")
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Zack Gomez <zack.gomez@gmail.com>
---
v3:
- no code change; the v2 mail was line-wrapped and whitespace-stripped
in transit and did not apply
https://lore.kernel.org/netdev/CAErB-w3Or6jkhsy9phjf5uo1O-T_hHQLx-XBFZeHX_8tqN38mw@mail.gmail.com/
v2:
- count the drop in tx_dropped of the device whose queue is full, so
it is visible when the sender ignores the return value or sits
above a stacked device (Breno, Sashiko)
- free with SKB_DROP_REASON_FULL_RING (Breno)
- skb_queue_len_lockless() for the unlocked length check; the cap is
a soft bound and the comment says so (Sashiko)
- commit message: xmit_drop_count only covers a directly bound
target; slab figures described as what they are; dropped the
sentence about the 2006 list discussion
- Cc Stephen Hemminger (Fixes: author)
v1: https://lore.kernel.org/netdev/20260914041221.1028092-1-zack.gomez@gmail.com/
Tested on 7.2.5 with this patch applied, same host and reproducer as
v1. Under the slow-completion condition the 200k-line burst that grew
unreclaimable slab by 173 MiB on the unpatched kernel grows it by
2 MiB, with 187229 drops counted in the target's transmit_errors and
the same number in the device's tx_dropped; with prompt completions
it drains at line rate with no drops and a sampled peak of ~1.1k
skbs. Slab figures are deltas sampled at 4 Hz. W=1 build of
netpoll.o and netconsole.o on this base is clean, checkpatch --strict
clean.
The first tx_dropped increment on a device allocates its core stats
with GFP_ATOMIC, the same class of allocation find_skb() already does
on this path; they could be allocated at netpoll setup instead if
preferred.
Still open from v1: tail drop keeps the oldest messages and loses the
newest, which for a console are usually the ones wanted, so dropping
from the head is a few more lines; and 1024 is arbitrary, about 1 MiB
of skbs.
net/core/netpoll.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index fe1e0cda5d6..aafbb19a288 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -38,6 +38,15 @@
#define USEC_PER_POLL 50
+/*
+ * Cap on skbs parked in npinfo->txq while the device is busy. The queue
+ * exists to ride out a transient stall; a producer that outruns the
+ * device for longer than that must lose packets, not grow it without
+ * bound. Checked without the queue lock, so the queue can overshoot
+ * slightly.
+ */
+#define NETPOLL_TXQ_MAX 1024
+
/*
* carrier_timeout is netconsole-specific and only kept here to preserve the
* netpoll.carrier_timeout module-parameter ABI. Its value is exposed to
@@ -314,6 +323,12 @@ static netdev_tx_t __netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
}
if (!dev_xmit_complete(status)) {
+ if (skb_queue_len_lockless(&npinfo->txq) >= NETPOLL_TXQ_MAX) {
+ dev_core_stats_tx_dropped_inc(dev);
+ dev_kfree_skb_irq_reason(skb,
+ SKB_DROP_REASON_FULL_RING);
+ goto out;
+ }
skb_queue_tail(&npinfo->txq, skb);
schedule_delayed_work(&npinfo->tx_work,0);
}
--
2.55.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v3] netpoll: bound the deferred transmit queue
2026-09-23 18:11 [PATCH net v3] netpoll: bound the deferred transmit queue Zack Gomez
@ 2026-09-25 15:32 ` Breno Leitao
2026-09-25 20:19 ` Zack Gomez
2026-09-27 18:01 ` netdev-bot+sashiko
1 sibling, 1 reply; 4+ messages in thread
From: Breno Leitao @ 2026-09-25 15:32 UTC (permalink / raw)
To: Zack Gomez
Cc: davem, edumazet, kuba, pabeni, horms, stephen, netdev, linux-kernel
On Wed, Sep 23, 2026 at 02:11:11PM -0400, Zack Gomez wrote:
> __netpoll_send_skb() parks an skb on npinfo->txq whenever the device
> cannot take it at once, and once the queue is non-empty every later skb
> goes straight there to keep ordering. queue_process() drains it from a
> workqueue and, unlike the direct path, never polls the device for
> completions: when the ring is stopped it backs off HZ/10. Nothing limits
> the queue length.
...
> Fixes: b6cd27ed3388 ("netpoll per device txq")
I don't think this should be a fix, honestly. I would prefer to see this
on net-next without the Fixes tag.
> Assisted-by: Claude:claude-fable-5-1
> Signed-off-by: Zack Gomez <zack.gomez@gmail.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v3] netpoll: bound the deferred transmit queue
2026-09-25 15:32 ` Breno Leitao
@ 2026-09-25 20:19 ` Zack Gomez
0 siblings, 0 replies; 4+ messages in thread
From: Zack Gomez @ 2026-09-25 20:19 UTC (permalink / raw)
To: leitao
Cc: davem, edumazet, kuba, pabeni, horms, stephen, netdev, linux-kernel
On Fri, Sep 25, 2026 at 08:32:56AM -0700, Breno Leitao wrote:
> I don't think this should be a fix, honestly. I would prefer to see this
> on net-next without the Fixes tag.
Agreed, I'll drop the tag and send v4 to net-next with your Reviewed-by.
Thanks for the reviews and I'll be watching your patchset as well.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v3] netpoll: bound the deferred transmit queue
2026-09-23 18:11 [PATCH net v3] netpoll: bound the deferred transmit queue Zack Gomez
2026-09-25 15:32 ` Breno Leitao
@ 2026-09-27 18:01 ` netdev-bot+sashiko
1 sibling, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 18:01 UTC (permalink / raw)
To: zack.gomez
Cc: davem, edumazet, kuba, pabeni, horms, leitao, stephen, netdev,
linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · 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).
- [High] netpoll: percpu allocation in __netpoll_send_skb triggers
PREEMPT_RT/NMI crashes
- [Medium] The FULL_RING drop reason that v2 added ("free with
SKB_DROP_REASON_FULL_RING") is thrown away on the netconsole path that…
--
netpoll: bound the deferred transmit queue
This patch caps npinfo->txq at NETPOLL_TXQ_MAX (1024) skbs per device.
When the queue is full, __netpoll_send_skb() now drops new skbs instead of
letting them pile up. Each drop is counted in the device's tx_dropped, and
the skb is freed with SKB_DROP_REASON_FULL_RING.
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index fe1e0cda5d6bf..aafbb19a28870 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
[ ... ]
> @@ -314,6 +323,12 @@ static netdev_tx_t __netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
> }
>
> if (!dev_xmit_complete(status)) {
> + if (skb_queue_len_lockless(&npinfo->txq) >= NETPOLL_TXQ_MAX) {
> + dev_core_stats_tx_dropped_inc(dev);
> + dev_kfree_skb_irq_reason(skb,
> + SKB_DROP_REASON_FULL_RING);
[Severity: High]
Can the first drop on a device end up in the percpu allocator with
hard IRQs disabled?
__netpoll_send_skb() asserts lockdep_assert_irqs_disabled(), and
netpoll_send_skb() gets here under local_irq_save(). When
dev->core_stats is still NULL, dev_core_stats_tx_dropped_inc() goes
through netdev_core_stats_inc() into netdev_core_stats_alloc(). That
calls alloc_percpu_gfp() with GFP_ATOMIC. For atomic requests,
pcpu_alloc_noprof() takes the global pcpu_lock with
spin_lock_irqsave().
On PREEMPT_RT, pcpu_lock is a sleeping lock. Taking it here with
interrupts hard-disabled is a sleeping-in-atomic bug.
There is also a problem on any config. If the console is flushed from
NMI context, for example a watchdog panic, and the interrupted code on
that CPU already holds pcpu_lock, this deadlocks.
The allocation happens only once per device, and only after 1024 skbs
are already parked. But that is exactly the flood or panic scenario
this patch is meant to survive.
The cover letter compares this with the GFP_ATOMIC skb allocation
find_skb() already does on this path. The two differ, though:
find_skb() can fall back to the preallocated skb pool, while the
percpu allocator always goes through pcpu_lock and has no fallback.
You offered to allocate the core stats at netpoll setup instead. Would
doing that in __netpoll_setup() be the better option? It runs in
process context for every device that owns an npinfo->txq. That
includes the lower devices that bond, team, bridge and vlan set up
through their ndo_netpoll_setup callbacks.
netdev_core_stats_alloc() is local to net/core/dev.c, so this probably
needs a small helper exposed from there. Or do you have a different way
to keep the tx_dropped accounting off the allocator in this context?
[Severity: Medium]
Will the FULL_RING reason reach drop monitoring tools on the netconsole
path?
dev_kfree_skb_irq_reason() only stores the reason in
get_kfree_skb_cb(skb)->reason and parks the skb on the per-CPU
softnet_data.completion_queue. The one consumer that reads that reason
and calls trace_kfree_skb() is net_tx_action().
netpoll_zap_completion_queue() also drains the same queue, and it frees
irq-freeable skbs with __kfree_skb():
net/core/netpoll.c:netpoll_zap_completion_queue() {
...
if (!skb_irq_freeable(skb)) {
refcount_set(&skb->users, 1);
dev_kfree_skb_any(skb); /* put this one back */
} else {
__kfree_skb(skb);
}
...
}
__kfree_skb() calls skb_release_all(skb, SKB_DROP_REASON_NOT_SPECIFIED)
and emits no kfree_skb tracepoint. The dev_kfree_skb_any() requeue path
overwrites the stored reason.
netconsole calls netpoll_zap_completion_queue() at the start of every skb
allocation, and netpoll_poll_dev() calls it too:
drivers/net/netconsole.c:find_skb() {
...
netpoll_zap_completion_queue();
repeat:
...
}
During a sustained flood, which is the case this patch targets, the next
message on the same CPU would normally drain the FULL_RING skbs before
NET_TX_SOFTIRQ runs. netconsole skbs have no destructor, nfct, dst,
secpath or frag_list, so skb_irq_freeable() is true and they take the
silent __kfree_skb() path.
As a result, could dropwatch and perf skb:kfree_skb see few or none of
these drops, even though tx_dropped counts all of them? The commit message
says the skbs are "freed with SKB_DROP_REASON_FULL_RING". Is that accurate
for this path?
> + goto out;
> + }
> skb_queue_tail(&npinfo->txq, skb);
> schedule_delayed_work(&npinfo->tx_work,0);
> }
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923181111.1182838-1-zack.gomez%40gmail.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-27 18:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 18:11 [PATCH net v3] netpoll: bound the deferred transmit queue Zack Gomez
2026-09-25 15:32 ` Breno Leitao
2026-09-25 20:19 ` Zack Gomez
2026-09-27 18:01 ` 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®