[PATCH net v3] net: iterate online nodes in skb_defer_free_flush()

[PATCH net v3] net: iterate online nodes in skb_defer_free_flush()

[Kris Pan]

skb_attempt_defer_free() only queues skbs on the current CPU's node
(numa_node_id() of a running CPU), which is always online, so the
flush loop never needs to visit nodes that are merely possible.

for_each_node() walks node_possible_map.  On machines where the
possible map is much larger than the online map -- e.g. a POWER10
LPAR with 32 possible but 1 online node -- the flush loop touches 31
cold, always-empty per-node lists on every softirq pass.

Use for_each_online_node() to iterate only node_online_map.

Since the flush now skips offline nodes, also drain the per-node
lists in dev_cpu_dead().  A skb is queued on the alloc CPU's list at
the freeing CPU's node index; if that node is offlined before the
alloc CPU flushes, the skb is stranded.  Likewise, skbs queued on a
CPU that later goes offline are never freed, since its softirq no
longer runs.

Loopback UDP throughput in a QEMU guest with 32 possible / 1 online
nodes (bench_udp, 8 senders, interleaved runs) improves by ~5%.

Fixes: 5628f3fe3b16 ("net: add NUMA awareness to skb_attempt_defer_free()")
Reported-by: kernel test robot <oliver.sang@intel.com>
Suggested-by: Adrian Tomasov <atomasov@redhat.com>
Suggested-by: Eric Dumazet <edumazet@google.com>
Closes: https://lore.kernel.org/oe-lkp/202512112119.5b9829a-lkp@intel.com
Signed-off-by: Kris Pan <kris.pan@intel.com>
---
 net/core/dev.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index ecfbd72d5d1a4..896498b349500 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6907,7 +6907,7 @@ static void skb_defer_free_flush(void)
 	struct skb_defer_node *sdn;
 	int node;
 
-	for_each_node(node) {
+	for_each_online_node(node) {
 		sdn = this_cpu_ptr(net_hotdata.skb_defer_nodes) + node;
 
 		if (llist_empty(&sdn->defer_list))
@@ -12895,9 +12895,12 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
 static int dev_cpu_dead(unsigned int oldcpu)
 {
 	struct sk_buff **list_skb;
-	struct sk_buff *skb;
+	struct llist_node *free_list;
+	struct sk_buff *skb, *next;
+	struct skb_defer_node *sdn;
 	unsigned int cpu;
 	struct softnet_data *sd, *oldsd, *remsd = NULL;
+	int node;
 
 	local_irq_disable();
 	cpu = smp_processor_id();
@@ -12957,6 +12960,26 @@ static int dev_cpu_dead(unsigned int oldcpu)
 		rps_input_queue_head_incr(oldsd);
 	}
 
+	/* Drain skbs deferred to the offlining CPU: its softirq no longer
+	 * runs, so skb_defer_free_flush() would never free them.  Also drain
+	 * skbs deferred to this CPU's node, which may leave node_online_map
+	 * and thus be skipped by the for_each_online_node() loop in
+	 * skb_defer_free_flush().
+	 */
+	for_each_node(node) {
+		sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, oldcpu) + node;
+		free_list = llist_del_all(&sdn->defer_list);
+		llist_for_each_entry_safe(skb, next, free_list, ll_node)
+			dev_consume_skb_any(skb);
+	}
+	node = cpu_to_node(oldcpu);
+	for_each_online_cpu(cpu) {
+		sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + node;
+		free_list = llist_del_all(&sdn->defer_list);
+		llist_for_each_entry_safe(skb, next, free_list, ll_node)
+			dev_consume_skb_any(skb);
+	}
+
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH net v3] net: iterate online nodes in skb_defer_free_flush()

[Eric Dumazet]

On Tue, Sep 15, 2026 at 5:37 PM Kris Pan <kris.pan@intel.com> wrote:
>
> skb_attempt_defer_free() only queues skbs on the current CPU's node
> (numa_node_id() of a running CPU), which is always online, so the
> flush loop never needs to visit nodes that are merely possible.
>
> for_each_node() walks node_possible_map.  On machines where the
> possible map is much larger than the online map -- e.g. a POWER10
> LPAR with 32 possible but 1 online node -- the flush loop touches 31
> cold, always-empty per-node lists on every softirq pass.
>
> Use for_each_online_node() to iterate only node_online_map.
>
> Since the flush now skips offline nodes, also drain the per-node
> lists in dev_cpu_dead().  A skb is queued on the alloc CPU's list at
> the freeing CPU's node index; if that node is offlined before the
> alloc CPU flushes, the skb is stranded.  Likewise, skbs queued on a
> CPU that later goes offline are never freed, since its softirq no
> longer runs.
>
> Loopback UDP throughput in a QEMU guest with 32 possible / 1 online
> nodes (bench_udp, 8 senders, interleaved runs) improves by ~5%.
>
> Fixes: 5628f3fe3b16 ("net: add NUMA awareness to skb_attempt_defer_free()")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Suggested-by: Adrian Tomasov <atomasov@redhat.com>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Closes: https://lore.kernel.org/oe-lkp/202512112119.5b9829a-lkp@intel.com
> Signed-off-by: Kris Pan <kris.pan@intel.com>
> ---

pw-bot: cr

Missing atomic_long_set(&sdn->defer_count, 0) permanently wedges defer_count.

I suggest we fix the pre-existing bug in the net tree, then later in
net-next you send your one-liner patch?
(s/for_each_node/for_each_online_node)

My LLM (Gemini) came up with this  (untested) patch.

commit 24094a55ca66a5507ea6eba395e469146ede7cc1
Author: Eric Dumazet <edumazet@google.com>
Date:   Wed Sep 16 00:12:57 2026 +0000

    net: flush skb_defer_nodes in dev_cpu_dead()

    When a CPU goes offline, dev_cpu_dead() drains its softnet queues
    (completion_queue, output_queue, poll_list, process_queue, and
    input_pkt_queue), but leaves net_hotdata.skb_defer_nodes untouched.

    If oldcpu goes offline while holding pending skbs in its
    skb_defer_nodes lists (e.g. below the sysctl_skb_defer_max >> 1 IPI
    threshold, or if the IPI races with CPU teardown), those skbs remain
    stranded until oldcpu is brought back online. If any of these skbs
    hold page_pool fragments, page_pool_destroy() will stall indefinitely
    waiting for inflight pages to be returned when a netdev or driver is
    torn down while oldcpu is offline.

    Additionally, if smp_call_function_single_async() fails in
    kick_defer_list_purge() because the target CPU went offline, reset
    defer_ipi_scheduled to 0 so future IPI kicks are not blocked when the
    CPU comes back online.

    Also, if oldcpu was the last online CPU on its NUMA node, drain that
    node's slot across all CPUs so no skbs deferred from that node remain
    stranded on idle remote CPUs (or if the node itself is subsequently
    offlined).

    Finally, in skb_attempt_defer_free(), re-check cpu_online(cpu) and
    whether the caller migrated CPUs after llist_add(), flushing the node
    list if so, to close the preemption TOCTOU race against CPU/node
    teardown.

    Fixes: 68822bdf76f1 ("net: generalize skb freeing deferral to
per-cpu lists")
    Fixes: 5628f3fe3b16 ("net: add NUMA awareness to skb_attempt_defer_free()")
    Assisted-by: LLM
    Signed-off-by: Eric Dumazet <edumazet@google.com>

diff --git a/net/core/dev.c b/net/core/dev.c
index ecfbd72d5d1a41d60e2276e9d78160c063fa347c..a3492d675138658f1f7bb4e6c48a306dbc3efb1b
100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5376,7 +5376,8 @@ void kick_defer_list_purge(unsigned int cpu)
                backlog_unlock_irq_restore(sd, flags);

        } else if (!cmpxchg(&sd->defer_ipi_scheduled, 0, 1)) {
-               smp_call_function_single_async(cpu, &sd->defer_csd);
+               if (smp_call_function_single_async(cpu, &sd->defer_csd))
+                       WRITE_ONCE(sd->defer_ipi_scheduled, 0);
        }
 }

@@ -6900,25 +6901,35 @@ bool napi_complete_done(struct napi_struct *n,
int work_done)
 }
 EXPORT_SYMBOL(napi_complete_done);

-static void skb_defer_free_flush(void)
+static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
 {
        struct llist_node *free_list;
        struct sk_buff *skb, *next;
+
+       if (llist_empty(&sdn->defer_list))
+               return;
+       atomic_long_set(&sdn->defer_count, 0);
+       free_list = llist_del_all(&sdn->defer_list);
+
+       llist_for_each_entry_safe(skb, next, free_list, ll_node) {
+               prefetch(next);
+               napi_consume_skb(skb, budget);
+       }
+}
+
+void skb_defer_node_flush(struct skb_defer_node *sdn)
+{
+       __skb_defer_free_flush(sdn, 0);
+}
+
+static void skb_defer_free_flush(void)
+{
        struct skb_defer_node *sdn;
        int node;

        for_each_node(node) {
                sdn = this_cpu_ptr(net_hotdata.skb_defer_nodes) + node;
-
-               if (llist_empty(&sdn->defer_list))
-                       continue;
-               atomic_long_set(&sdn->defer_count, 0);
-               free_list = llist_del_all(&sdn->defer_list);
-
-               llist_for_each_entry_safe(skb, next, free_list, ll_node) {
-                       prefetch(next);
-                       napi_consume_skb(skb, 1);
-               }
+               __skb_defer_free_flush(sdn, 1);
        }
 }

@@ -12897,6 +12908,7 @@ static int dev_cpu_dead(unsigned int oldcpu)
        struct sk_buff **list_skb;
        struct sk_buff *skb;
        unsigned int cpu;
+       int node;
        struct softnet_data *sd, *oldsd, *remsd = NULL;

        local_irq_disable();
@@ -12957,6 +12969,17 @@ static int dev_cpu_dead(unsigned int oldcpu)
                rps_input_queue_head_incr(oldsd);
        }

+       WRITE_ONCE(oldsd->defer_ipi_scheduled, 0);
+       for_each_node(node)
+               skb_defer_node_flush(per_cpu_ptr(net_hotdata.skb_defer_nodes,
+                                                oldcpu) + node);
+       node = cpu_to_node(oldcpu);
+       if (node_possible(node) && cpumask_empty(cpumask_of_node(node))) {
+               for_each_possible_cpu(cpu)
+
skb_defer_node_flush(per_cpu_ptr(net_hotdata.skb_defer_nodes,
+                                                        cpu) + node);
+       }
+
        return 0;
 }

diff --git a/net/core/dev.h b/net/core/dev.h
index b757faead4d1a3e445e54d2f468c38c9e09b6762..04fb0e9a571e03f41cac110a0d1dd864d110b339
100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -399,6 +399,8 @@ static inline void napi_assert_will_not_race(const
struct napi_struct *napi)
        WARN_ON(READ_ONCE(napi->list_owner) != -1);
 }

+struct skb_defer_node;
+void skb_defer_node_flush(struct skb_defer_node *sdn);
 void kick_defer_list_purge(unsigned int cpu);

 int dev_set_hwtstamp_phylib(struct net_device *dev,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a2d7b2fbc006d70e965c820b99d6eb4b3432e7c8..d8bd8ed2489e328d81aabde4114c0e878ebe0b63
100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -7332,8 +7332,8 @@ void skb_attempt_defer_free(struct sk_buff *skb)
        struct skb_defer_node *sdn;
        unsigned long defer_count;
        unsigned int defer_max;
+       int cpu, my_cpu;
        bool kick;
-       int cpu;

        if (static_branch_unlikely(&skb_defer_disable_key))
                goto nodefer;
@@ -7343,7 +7343,8 @@ void skb_attempt_defer_free(struct sk_buff *skb)
                goto nodefer;

        cpu = skb->alloc_cpu;
-       if (cpu == raw_smp_processor_id() ||
+       my_cpu = raw_smp_processor_id();
+       if (cpu == my_cpu ||
            WARN_ON_ONCE(cpu >= nr_cpu_ids) ||
            !cpu_online(cpu)) {
 nodefer:       kfree_skb_napi_cache(skb);
@@ -7354,7 +7355,7 @@ nodefer:  kfree_skb_napi_cache(skb);
        DEBUG_NET_WARN_ON_ONCE(skb->destructor);
        DEBUG_NET_WARN_ON_ONCE(skb_nfct(skb));

-       sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + numa_node_id();
+       sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) +
cpu_to_node(my_cpu);

        defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
        defer_count = atomic_long_inc_return(&sdn->defer_count);
@@ -7364,6 +7365,11 @@ nodefer: kfree_skb_napi_cache(skb);

        llist_add(&skb->ll_node, &sdn->defer_list);

+       if (unlikely(!cpu_online(cpu) || my_cpu != raw_smp_processor_id())) {
+               skb_defer_node_flush(sdn);
+               return;
+       }
+
        /* Send an IPI every time queue reaches half capacity. */
        kick = (defer_count - 1) == (defer_max >> 1);

Re: [PATCH net v3] net: iterate online nodes in skb_defer_free_flush()

[Kris Pan]

Thanks for the review and the patch, Eric.

Splitting it makes sense: the dev_cpu_dead() drain is a real pre-existing
bug fix for net, and the for_each_online_node() one-liner can go to
net-next afterwards.  I'll drop this v3 and wait for your net patch, then
send the one-liner.

Thanks,
Kris

Re: [PATCH net v3] net: iterate online nodes in skb_defer_free_flush()

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 2 · 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] dev_cpu_dead() drains the per-(cpu,node) deferred-free lists
  with llist_del_all() but never resets the paired sdn->defer_count.
- [Medium] The commit folds a pre-existing memory-leak fix into a hot-
  path NUMA-iteration optimisation and attributes both to Fixes:…
- [Medium] The new drain in dev_cpu_dead() is a one-shot sweep with no
  synchronization against skb_attempt_defer_free(), leaving the bug…

--

net: iterate online nodes in skb_defer_free_flush()

The patch switches skb_defer_free_flush() from for_each_node() to
for_each_online_node() so the softirq flush loop no longer walks per-node
lists for nodes that are only possible. It also adds two new drain loops to
dev_cpu_dead() so that skbs queued on the offlining CPU, or queued at the
offlining CPU's node index on other CPUs, are freed.

Closes: https://lore.kernel.org/oe-lkp/202512112119.5b9829a-lkp@intel.com

A couple of questions on the commit message first:

>     Since the flush now skips offline nodes, also drain the per-node
>     lists in dev_cpu_dead().  A skb is queued on the alloc CPU's list at
>     the freeing CPU's node index; if that node is offlined before the
>     alloc CPU flushes, the skb is stranded.  Likewise, skbs queued on a
>     CPU that later goes offline are never freed, since its softirq no
>     longer runs.
>
>     Fixes: 5628f3fe3b16 ("net: add NUMA awareness to skb_attempt_defer_free()")

[Severity: Medium]
Is the Fixes tag correct for the second half of this change?

Looking at 5628f3fe3b16, it only moved sd->defer_list / sd->defer_count into
the per-node net_hotdata.skb_defer_nodes array and adjusted
skb_defer_free_flush() and skb_attempt_defer_free().  It does not touch
dev_cpu_dead() at all.

So the "skbs queued on a CPU that later goes offline are never freed" leak
also existed before 5628f3fe3b16, when the single per-CPU sd->defer_list was
likewise never drained by dev_cpu_dead().  Only the part about lists indexed
by a node that may leave node_online_map is a consequence of the
for_each_online_node() switch here.

Following this Fixes tag, a stable backport would only reach trees that
already contain 5628f3fe3b16 and would leave the same leak in older trees.
Would it make sense to split the dev_cpu_dead() drain (stable candidate,
with its own Fixes tag) from the flush-loop optimisation?

Also, the message states that skb_attempt_defer_free() "only queues skbs on
the current CPU's node ... which is always online", but the first new loop in
dev_cpu_dead() iterates for_each_node(), i.e. all possible nodes.  Which of
the two assumptions is intended?

> diff --git a/net/core/dev.c b/net/core/dev.c
> index ecfbd72d5d1a4..896498b349500 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6907,7 +6907,7 @@ static void skb_defer_free_flush(void)
>  	struct skb_defer_node *sdn;
>  	int node;
>  
> -	for_each_node(node) {
> +	for_each_online_node(node) {
>  		sdn = this_cpu_ptr(net_hotdata.skb_defer_nodes) + node;
>  
>  		if (llist_empty(&sdn->defer_list))

[ ... ]

> @@ -12957,6 +12960,26 @@ static int dev_cpu_dead(unsigned int oldcpu)
>  		rps_input_queue_head_incr(oldsd);
>  	}
>  
> +	/* Drain skbs deferred to the offlining CPU: its softirq no longer
> +	 * runs, so skb_defer_free_flush() would never free them.  Also drain
> +	 * skbs deferred to this CPU's node, which may leave node_online_map
> +	 * and thus be skipped by the for_each_online_node() loop in
> +	 * skb_defer_free_flush().
> +	 */
> +	for_each_node(node) {
> +		sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, oldcpu) + node;
> +		free_list = llist_del_all(&sdn->defer_list);
> +		llist_for_each_entry_safe(skb, next, free_list, ll_node)
> +			dev_consume_skb_any(skb);
> +	}
> +	node = cpu_to_node(oldcpu);
> +	for_each_online_cpu(cpu) {
> +		sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + node;
> +		free_list = llist_del_all(&sdn->defer_list);
> +		llist_for_each_entry_safe(skb, next, free_list, ll_node)
> +			dev_consume_skb_any(skb);
> +	}
> +
>  	return 0;
>  }

[Severity: High]
Should both new loops in dev_cpu_dead() also reset sdn->defer_count when
they steal the list?

The existing consumer always pairs the two:

net/core/dev.c:skb_defer_free_flush() {
	...
		if (llist_empty(&sdn->defer_list))
			continue;
		atomic_long_set(&sdn->defer_count, 0);
		free_list = llist_del_all(&sdn->defer_list);
	...
}

The producer increments before the limit test and does not undo the
increment on the reject path:

net/core/skbuff.c:skb_attempt_defer_free() {
	...
	defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
	defer_count = atomic_long_inc_return(&sdn->defer_count);

	if (defer_count >= defer_max)
		goto nodefer;

	llist_add(&skb->ll_node, &sdn->defer_list);
	...
}

so defer_count can already be at or above sysctl_skb_defer_max while skbs
are queued.

If either new loop empties such a list without clearing the counter, can
that slot ever recover?  Every later skb_attempt_defer_free() for that
(cpu, node) increments the counter, sees defer_count >= defer_max, and goes
to nodefer before llist_add(), so the list stays empty and
skb_defer_free_flush() skips it on the llist_empty() fast path and never
runs atomic_long_set().

The second loop does this to lists owned by CPUs that are still online, so
would a still-running CPU permanently lose deferred freeing for the
offlined CPU's node index?

There is a milder variant as well: any residual non-zero count makes

	kick = (defer_count - 1) == (defer_max >> 1);

unable to fire for the values it skipped, so an idle target CPU may not get
the kick_defer_list_purge() IPI and queued skbs would wait for some
unrelated NET_RX_SOFTIRQ.

Nothing appears to repair this on the way back up either, since the startup
callback is NULL:

net/core/dev.c:net_dev_init() {
	...
	rc = cpuhp_setup_state_nocalls(CPUHP_NET_DEV_DEAD, "net/dev:dead",
				       NULL, dev_cpu_dead);
	...
}

[Severity: Medium]
Can an skb still be queued onto the dead CPU's list after these loops have
run?

skb_attempt_defer_free() checks cpu_online() and only later performs the
llist_add(), with no preempt_disable(), no RCU section and no re-check:

net/core/skbuff.c:skb_attempt_defer_free() {
	...
	cpu = skb->alloc_cpu;
	if (cpu == raw_smp_processor_id() ||
	    WARN_ON_ONCE(cpu >= nr_cpu_ids) ||
	    !cpu_online(cpu)) {
nodefer:	kfree_skb_napi_cache(skb);
		return;
	}
	...
	sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + numa_node_id();
	...
	llist_add(&skb->ll_node, &sdn->defer_list);
	...
}

Its process-context callers run preemptible, for example
skb_consume_udp() in net/ipv4/udp.c, tcp_eat_recv_skb() in net/ipv4/tcp.c
and mptcp_eat_recv_skb() in net/mptcp/protocol.c.

So the sequence would be:

CPU A (recvmsg)                        CPU B offlined
skb_attempt_defer_free()
  cpu_online(B) == true
  <preempted>
                                       dev_cpu_dead() runs both drain loops
  llist_add(... B's list ...)

After that, B's softirq never runs skb_defer_free_flush(), and the second
loop here only sweeps index cpu_to_node(oldcpu) on online CPUs, so that slot
is not visited again.  Is the skb then leaked for as long as B stays
offline, and would page_pool pages carried by it hold up
page_pool_destroy()?

Since the commit message presents "skbs queued on a CPU that later goes
offline are never freed" as fixed, would some ordering between the producer
and teardown (a re-check under a hotplug-safe section, or draining from
another CPU's flush path) be needed rather than a single sweep?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916003430.3612956-1-kris.pan%40intel.com