[PATCH v2 net-next 0/2] ip: Batch flushing uncached routes per batched device unregistration.

[PATCH v2 net-next 0/2] ip: Batch flushing uncached routes per batched device unregistration.

[Kuniyuki Iwashima]

Chris J Arges reported high RTNL contention during cleanup_net()
caused by rt_flush_dev() and rt6_uncached_list_flush_dev() iterating
over the global per-cpu uncached route lists for every netdev in
dying netns [0].

This series resolves the issue by batching the uncached route
cleanup after __rtnl_unlock() in netdev_run_todo().

[0]: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/


Changelog:
  v2:
    * Drop ->pre_exit_batch()
    * Do batching after setting NETDEV_UNREGISTERED in netdev_run_todo()

  v1: https://lore.kernel.org/netdev/20260927202429.2452589-1-kuniyu@google.com/


Kuniyuki Iwashima (2):
  ipv4: Batch rt_flush_dev() in netdev_run_todo().
  ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().

 include/net/ip6_route.h |  7 +++++++
 include/net/route.h     |  6 ++++++
 net/core/dev.c          |  5 +++++
 net/ipv4/route.c        | 11 +++++++++--
 net/ipv6/route.c        | 18 ++++++++++++++----
 5 files changed, 41 insertions(+), 6 deletions(-)

-- 
2.56.0.rc1.315.gc6ed9934b7-goog

[PATCH v2 net-next 1/2] ipv4: Batch rt_flush_dev() in netdev_run_todo().

[Kuniyuki Iwashima]

IPv4 uncached routes are linked to the global per-cpu lists,
rt_uncached_list.

When unregistering a netdev, rt_flush_dev() iterates over the
potentially long lists to find uncached routes tied to the device
and swap it with blackhole_netdev.

Since it is called for every device in dying netns under RTNL,
it adds O(N_dev x (N_cpu + N_route)) costs to any batched device
unregistration.

Let's call it once per batched device unregistration without RTNL.

Note that rt_flush_dev() must be called after setting dev->reg_state
to NETREG_UNREGISTERED.  Otherwise, because rt_flush_dev(NULL) runs
without RTNL, it could race with unregister_netdevice_many_notify()
and prematurely purge routes for NETREG_UNREGISTERING dev, for
which flush_all_backlogs() has not been called yet.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/route.h |  6 ++++++
 net/core/dev.c      |  3 +++
 net/ipv4/route.c    | 11 +++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/net/route.h b/include/net/route.h
index 6b55de2e4df8..3fccd31eb74d 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -129,7 +129,13 @@ struct in_device;
 
 int ip_rt_init(void);
 void rt_cache_flush(struct net *net);
+#ifdef CONFIG_INET
 void rt_flush_dev(struct net_device *dev);
+#else
+static inline void rt_flush_dev(struct net_device *dev)
+{
+}
+#endif
 
 static inline void inet_sk_init_flowi4(const struct inet_sock *inet,
 				       struct flowi4 *fl4)
diff --git a/net/core/dev.c b/net/core/dev.c
index a8eb382f40ca..9cedc17b5bdc 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11838,6 +11838,9 @@ void netdev_run_todo(void)
 		linkwatch_sync_dev(dev);
 	}
 
+	if (!list_empty(&list))
+		rt_flush_dev(NULL);
+
 	cnt = 0;
 	while (!list_empty(&list)) {
 		dev = netdev_wait_allrefs_any(&list);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d7da2f1acbb5..1b641901f10d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1587,6 +1587,9 @@ void rt_flush_dev(struct net_device *dev)
 	struct rtable *rt, *safe;
 	int cpu;
 
+	if (dev && dev->dismantle)
+		return;
+
 	for_each_possible_cpu(cpu) {
 		struct uncached_list *ul = &per_cpu(rt_uncached_list, cpu);
 
@@ -1595,10 +1598,14 @@ void rt_flush_dev(struct net_device *dev)
 
 		spin_lock_bh(&ul->lock);
 		list_for_each_entry_safe(rt, safe, &ul->head, dst.rt_uncached) {
-			if (rt->dst.dev != dev)
+			struct net_device *rt_dev = rt->dst.dev;
+
+			if (dev ? rt_dev != dev :
+			    READ_ONCE(rt_dev->reg_state) != NETREG_UNREGISTERED)
 				continue;
+
 			rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
-			netdev_ref_replace(dev, blackhole_netdev,
+			netdev_ref_replace(rt_dev, blackhole_netdev,
 					   &rt->dst.dev_tracker, GFP_ATOMIC);
 			list_del_init(&rt->dst.rt_uncached);
 		}
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

[PATCH v2 net-next 2/2] ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().

[Kuniyuki Iwashima]

IPv6 uncached routes are linked to the global per-cpu lists,
rt6_uncached_list.

When unregistering a netdev, rt6_uncached_list_flush_dev()
iterates over the potentially long lists to find uncached
routes tied to the device and swap it with blackhole_netdev.

Since it is called for every device in dying netns under RTNL,
it adds O(N_dev x (N_cpu + N_route)) costs to any batched device
unregistration.

Let's call it once per batched device unregistration without RTNL.

Note that !ul->head.next must be checked in case disable_ipv6_mod
is true or boot fails before inet6_init().

Reported-by: Chris J Arges <carges@cloudflare.com>
Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/ip6_route.h |  7 +++++++
 net/core/dev.c          |  4 +++-
 net/ipv6/route.c        | 18 ++++++++++++++----
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 0f9b7a260d25..b9de335b639a 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -231,6 +231,13 @@ void rt6_multipath_rebalance(struct fib6_info *f6i);
 
 void rt6_uncached_list_add(struct rt6_info *rt);
 void rt6_uncached_list_del(struct rt6_info *rt);
+#ifdef CONFIG_IPV6
+void rt6_uncached_list_flush_dev(struct net_device *dev);
+#else
+static inline void rt6_uncached_list_flush_dev(struct net_device *dev)
+{
+}
+#endif
 
 static inline const struct rt6_info *skb_rt6_info(const struct sk_buff *skb)
 {
diff --git a/net/core/dev.c b/net/core/dev.c
index 9cedc17b5bdc..0b3213ff56e7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11838,8 +11838,10 @@ void netdev_run_todo(void)
 		linkwatch_sync_dev(dev);
 	}
 
-	if (!list_empty(&list))
+	if (!list_empty(&list)) {
 		rt_flush_dev(NULL);
+		rt6_uncached_list_flush_dev(NULL);
+	}
 
 	cnt = 0;
 	while (!list_empty(&list)) {
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index a76869ff87cd..38261775e713 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -158,14 +158,20 @@ void rt6_uncached_list_del(struct rt6_info *rt)
 	}
 }
 
-static void rt6_uncached_list_flush_dev(struct net_device *dev)
+void rt6_uncached_list_flush_dev(struct net_device *dev)
 {
 	int cpu;
 
+	if (dev && dev->dismantle)
+		return;
+
 	for_each_possible_cpu(cpu) {
 		struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
 		struct rt6_info *rt, *safe;
 
+		if (unlikely(!ul->head.next))
+			return;
+
 		if (list_empty(&ul->head))
 			continue;
 
@@ -175,19 +181,23 @@ static void rt6_uncached_list_flush_dev(struct net_device *dev)
 			struct net_device *rt_dev = rt->dst.dev;
 			bool handled = false;
 
-			if (rt_idev && rt_idev->dev == dev) {
+			if (rt_idev &&
+			    (dev ? rt_idev->dev == dev :
+			     READ_ONCE(rt_idev->dev->reg_state) == NETREG_UNREGISTERED)) {
 				rt->rt6i_idev = in6_dev_get(blackhole_netdev);
 				in6_dev_put(rt_idev);
 				handled = true;
 			}
 
-			if (rt_dev == dev) {
-				rt->dst.dev = blackhole_netdev;
+			if (dev ? rt_dev == dev :
+			    READ_ONCE(rt_dev->reg_state) == NETREG_UNREGISTERED) {
+				rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
 				netdev_ref_replace(rt_dev, blackhole_netdev,
 						   &rt->dst.dev_tracker,
 						   GFP_ATOMIC);
 				handled = true;
 			}
+
 			if (handled)
 				list_del_init(&rt->dst.rt_uncached);
 		}
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH v2 net-next 0/2] ip: Batch flushing uncached routes per batched device unregistration.

[Chris Arges]

On 2026-09-29 20:15:06, Kuniyuki Iwashima wrote:
> Chris J Arges reported high RTNL contention during cleanup_net()
> caused by rt_flush_dev() and rt6_uncached_list_flush_dev() iterating
> over the global per-cpu uncached route lists for every netdev in
> dying netns [0].
> 
> This series resolves the issue by batching the uncached route
> cleanup after __rtnl_unlock() in netdev_run_todo().
> 
> [0]: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
> 
> 
> Changelog:
>   v2:
>     * Drop ->pre_exit_batch()
>     * Do batching after setting NETDEV_UNREGISTERED in netdev_run_todo()
> 
>   v1: https://lore.kernel.org/netdev/20260927202429.2452589-1-kuniyu@google.com/
> 
> 
> Kuniyuki Iwashima (2):
>   ipv4: Batch rt_flush_dev() in netdev_run_todo().
>   ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().
> 
>  include/net/ip6_route.h |  7 +++++++
>  include/net/route.h     |  6 ++++++
>  net/core/dev.c          |  5 +++++
>  net/ipv4/route.c        | 11 +++++++++--
>  net/ipv6/route.c        | 18 ++++++++++++++----
>  5 files changed, 41 insertions(+), 6 deletions(-)
> 
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
> 
I was able to test and confirm that this patchset reduces RTNL lock contention
with my test case. Please add:
Tested-by: Chris J Arges <carges@cloudflare.com>

Also for "ipv4: Batch rt_flush_dev() in netdev_run_todo()" please include:
Reported-by: Chris J Arges <carges@cloudflare.com>.

Thanks,
--chris

Re: [PATCH v2 net-next 1/2] ipv4: Batch rt_flush_dev() in netdev_run_todo().

[netdev-bot+sashiko]

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

Critical: 0 · High: 3 · 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] Retrying the NETDEV_UNREGISTER broadcast no longer flushes IPv4
  uncached routes.
- [High] Moving the IPv4 uncached-route device swap from the
  NETDEV_UNREGISTER notifier into `netdev_run_todo()` removes the RCU…
- [High] ipv4: NULL pointer dereference in rt_flush_dev() with
  uninitialized list

--

ipv4: Batch rt_flush_dev() in netdev_run_todo().

This commit takes the IPv4 uncached route flush out of the per-device
NETDEV_UNREGISTER path. It now calls rt_flush_dev(NULL) once per batched
unregistration in netdev_run_todo(), without holding RTNL. rt_flush_dev()
returns early for dismantling devices. When passed NULL, it moves every
route whose device is NETREG_UNREGISTERED over to blackhole_netdev.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index a8eb382f40caf..9cedc17b5bdc2 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -11838,6 +11838,9 @@ void netdev_run_todo(void)
>  		linkwatch_sync_dev(dev);
>  	}
>  
> +	if (!list_empty(&list))
> +		rt_flush_dev(NULL);
> +
>  	cnt = 0;
>  	while (!list_empty(&list)) {
>  		dev = netdev_wait_allrefs_any(&list);
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index d7da2f1acbb59..1b641901f10dc 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1587,6 +1587,9 @@ void rt_flush_dev(struct net_device *dev)
>  	struct rtable *rt, *safe;
>  	int cpu;
>  
> +	if (dev && dev->dismantle)
> +		return;
> +

[Severity: High]
Can this call reach rt_flush_dev() before ip_rt_init() has set up
rt_uncached_list?

The per-cpu rt_uncached_list entries are only initialised in
ip_rt_init(), which does INIT_LIST_HEAD() and spin_lock_init() for
each possible CPU. That runs from inet_init(), an fs_initcall. Until
then ul->head.next is still NULL.

Before this patch, rt_flush_dev() was only reached through
fib_netdev_event(). That notifier is registered from ip_fib_init(),
which ip_rt_init() calls after it initialises the lists, so the
ordering was implicitly safe.

netdev_run_todo() has no such dependency. It runs from rtnl_unlock()
whenever net_todo_list is non-empty. Any netdev unregistration before
fs_initcall time would now call rt_flush_dev(NULL) on zeroed lists.
One example is an earlier initcall that registers a device and then
unregisters it on a later failure.

In that case list_empty(&ul->head) returns false, because head->next
is NULL rather than &ul->head. The loop then takes the uninitialised
ul->lock and enters list_for_each_entry_safe(). That derives pos from
the NULL head->next and immediately loads pos->dst.rt_uncached.next,
which is address 0. The result is a NULL pointer dereference during
boot.

The IPv6 patch later in the series seems to handle the same case in
rt6_uncached_list_flush_dev() by skipping a list whose head.next is
still NULL. Should rt_flush_dev() get a similar guard? Or should
netdev_run_todo() skip the call until IPv4 routing is initialised?

[Severity: High]
With this early return, does the NETDEV_UNREGISTER that
netdev_wait_allrefs_any() resends still flush any IPv4 uncached routes?

netdev_wait_allrefs_any() resends NETDEV_UNREGISTER about once a second so
protocols can drop late references. That reaches
fib_netdev_event()->rt_flush_dev(dev). dev->dismantle is already set, so
the first notifier call and every retry now return straight away.

The only flush left for a dying device is the single rt_flush_dev(NULL) in
netdev_run_todo(). It runs once, before the wait loop, and no grace period
follows it.

Can an uncached route with the dying device be added after that pass? One
possible path goes through xfrm_bundle_create():

xfrm_bundle_create()
    rcu_read_lock();
    dev = dst_dev_rcu(dst);
    ...
    xfrm_fill_dst(xdst_prev, dev, fl)
        xfrm4_fill_dst()
            netdev_hold(dev, &xdst->u.dst.dev_tracker, GFP_ATOMIC);
            ...
            rt_add_uncached_list(&xdst->u.rt);

Say the sweep moves dst_orig to blackhole_netdev after dev is read but
before the new xdst is linked. The xdst then goes onto rt_uncached_list
after the sweep has already passed that CPU's list, and it still holds a
reference on the dying device.

Before this patch, synchronize_net() in unregister_netdevice_many_notify()
ran after the notifier flush, and each retry ran rt_flush_dev(dev) again.
Nothing now seems to come back to such an entry.

Could unregister_netdevice then loop on "waiting for %s to become free" for
as long as the xdst lives? For a bundle cached on an idle connected socket,
that could be forever.

>  	for_each_possible_cpu(cpu) {
>  		struct uncached_list *ul = &per_cpu(rt_uncached_list, cpu);
>  
> @@ -1595,10 +1598,14 @@ void rt_flush_dev(struct net_device *dev)
>  
>  		spin_lock_bh(&ul->lock);
>  		list_for_each_entry_safe(rt, safe, &ul->head, dst.rt_uncached) {
> -			if (rt->dst.dev != dev)
> +			struct net_device *rt_dev = rt->dst.dev;
> +
> +			if (dev ? rt_dev != dev :
> +			    READ_ONCE(rt_dev->reg_state) != NETREG_UNREGISTERED)
>  				continue;
> +
>  			rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
> -			netdev_ref_replace(dev, blackhole_netdev,
> +			netdev_ref_replace(rt_dev, blackhole_netdev,
>  					   &rt->dst.dev_tracker, GFP_ATOMIC);

[Severity: High]
Does moving the swap here remove the RCU grace period between publishing
blackhole_netdev and freeing the old net_device?

Before, rt_flush_dev(dev) ran from the NETDEV_UNREGISTER notifier. Then
unregister_netdevice_many_notify() called synchronize_net(), so any reader
that had loaded the old rt->dst.dev was done before teardown.

The swap now runs after every synchronize_net(), and also after the
rcu_barrier() in netdev_run_todo(). If netdev_ref_replace() drops the last
extra reference, netdev_wait_allrefs_any() returns right away:

	list_for_each_entry(dev, list, todo_list)
		if (netdev_refcnt_read(dev) == 1)
			return dev;

netdev_run_todo() then goes on to free_netdev() and kobject_put(), and
netdev_release() frees the device directly:

	/* no need to wait for rcu grace period:
	 * device is dead and about to be freed.
	 */
	kfree(rcu_access_pointer(dev->ifalias));
	kvfree(dev);

Could an RCU reader that loaded the old device still be using it at that
point? One example is rt_is_expired():

	rcu_read_lock();
	res = rth->rt_genid != rt_genid_ipv4(dev_net_rcu(rth->dst.dev));
	rcu_read_unlock();

Another is xfrm_bundle_create(). It reads dst_dev_rcu(dst) and then calls
xfrm4_fill_dst()->netdev_hold(dev), which would write to the refcount of a
freed device.

The ul->lock spinlock only serializes list writers, not these readers. The
IPv6 patch that follows in the series uses the same pattern
(rt6_uncached_list_flush_dev(NULL) in netdev_run_todo()), so the problem
remains at the end of the series.

Does this need a synchronize_net() after the batched flush?

>  			list_del_init(&rt->dst.rt_uncached);
>  		}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929201546.1533268-1-kuniyu%40google.com

Re: [PATCH v2 net-next 2/2] ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 2 · 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).

- [High] Moving the IPv6 uncached-route flush into netdev_run_todo()
  removes the RCU grace period that used to follow the device-pointer…
- [Medium] Build failure with CONFIG_IPV6=n.
- [Medium] The batched flush reads dev->reg_state twice per route with no
  consistent snapshot, and another unregister batch can change reg_state…
- [Low] The new initialization guard `if (unlikely(!ul->head.next))
  return;` in rt6_uncached_list_flush_dev() (net/ipv6/route.c:172) is a…

--

ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().

This patch moves the IPv6 uncached route flush out of the per-device
NETDEV_UNREGISTER path. It now runs once per batched unregistration from
netdev_run_todo(), without RTNL. With dev == NULL, a route's device is
matched by reg_state == NETREG_UNREGISTERED.

Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 9cedc17b5bdc2..0b3213ff56e70 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -11838,8 +11838,10 @@ void netdev_run_todo(void)
>  		linkwatch_sync_dev(dev);
>  	}
>  
> -	if (!list_empty(&list))
> +	if (!list_empty(&list)) {
>  		rt_flush_dev(NULL);
> +		rt6_uncached_list_flush_dev(NULL);

[Severity: Medium]
With CONFIG_IPV6=n, is the new stub for rt6_uncached_list_flush_dev()
visible here?

net/core/dev.c doesn't include <net/ip6_route.h> directly. The only path
seems to be net/udp_tunnel.h -> net/ip_tunnels.h, and that include is
compiled out when IPv6 is off:

include/net/ip_tunnels.h:
#if IS_ENABLED(CONFIG_IPV6)
#include <net/ipv6.h>
#include <net/ip6_fib.h>
#include <net/ip6_route.h>
#endif

So when IPv6 is disabled, dev.c would see neither the prototype nor the
static inline stub added to include/net/ip6_route.h. Would this call then
fail with -Werror=implicit-function-declaration? It would also fail to
link, since net/ipv6/route.c isn't built.

rt_flush_dev() doesn't hit this, because net/route.h always reaches
dev.c through net/ip.h.

Should dev.c include <net/ip6_route.h> directly, or should the
declaration and stub go in a header that dev.c always includes?

> +	}
>  
>  	cnt = 0;
>  	while (!list_empty(&list)) {
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index a76869ff87cd6..38261775e7137 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -158,14 +158,20 @@ void rt6_uncached_list_del(struct rt6_info *rt)
>  	}
>  }
>  
> -static void rt6_uncached_list_flush_dev(struct net_device *dev)
> +void rt6_uncached_list_flush_dev(struct net_device *dev)
>  {
>  	int cpu;
>  
> +	if (dev && dev->dismantle)
> +		return;
> +
>  	for_each_possible_cpu(cpu) {
>  		struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
>  		struct rt6_info *rt, *safe;
>  
> +		if (unlikely(!ul->head.next))
> +			return;

[Severity: Low]
This isn't a functional bug, but ul->head.next is read here without
ul->lock. Other CPUs can update it concurrently under the lock:
list_add_tail() in rt6_uncached_list_add() when the list is empty, and
list_del_init() in rt6_uncached_list_del().

The list_empty() check just below already uses READ_ONCE(). Should this
be READ_ONCE(ul->head.next) so that KCSAN doesn't report it?

> +
>  		if (list_empty(&ul->head))
>  			continue;
>  
> @@ -175,19 +181,23 @@ static void rt6_uncached_list_flush_dev(struct net_device *dev)
>  			struct net_device *rt_dev = rt->dst.dev;
>  			bool handled = false;
>  
> -			if (rt_idev && rt_idev->dev == dev) {
> +			if (rt_idev &&
> +			    (dev ? rt_idev->dev == dev :
> +			     READ_ONCE(rt_idev->dev->reg_state) == NETREG_UNREGISTERED)) {
>  				rt->rt6i_idev = in6_dev_get(blackhole_netdev);
>  				in6_dev_put(rt_idev);
>  				handled = true;
>  			}
>  
> -			if (rt_dev == dev) {
> -				rt->dst.dev = blackhole_netdev;
> +			if (dev ? rt_dev == dev :
> +			    READ_ONCE(rt_dev->reg_state) == NETREG_UNREGISTERED) {

[Severity: Medium]
If two netdev_run_todo() instances overlap, can these two reg_state reads
see different values for the same device?

netdev_run_todo() calls __rtnl_unlock() before it sets each device to
NETREG_UNREGISTERED, and that write is done under netdev_lock(), not
ul->lock. So batch A can be scanning while batch B updates a device D
that is on B's todo list. Take a route whose rt6i_idev->dev and dst.dev
are both D:

CPU1 (batch A, flush(NULL))          CPU2 (batch B, owns D)
READ_ONCE(rt_idev->dev->reg_state)
  sees NETREG_UNREGISTERING,
  rt6i_idev is not swapped
                                     WRITE_ONCE(D->reg_state,
                                                NETREG_UNREGISTERED)
READ_ONCE(rt_dev->reg_state)
  sees NETREG_UNREGISTERED,
  dst.dev swapped, handled = true
list_del_init(&rt->dst.rt_uncached)

By the time batch B's own flush runs, the route is off the list. Its
rt6i_idev still points at D's inet6_dev, which holds a reference on D.

The NETDEV_UNREGISTER rebroadcasts can't fix this either.
rt6_disable_ip()->rt6_uncached_list_flush_dev(dev) now returns early on
dev->dismantle, and the route is no longer listed.

Could batch B then sit in netdev_wait_allrefs_any() printing "waiting for
D to become free" for as long as something holds the uncached route? The
window is narrow, but the old code compared pointers under RTNL, so it
could not race this way.

> +				rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
>  				netdev_ref_replace(rt_dev, blackhole_netdev,
>  						   &rt->dst.dev_tracker,
>  						   GFP_ATOMIC);

[Severity: High]
Is there still an RCU grace period between this pointer swap and the
old device being freed?

Before this patch, the flush ran from the NETDEV_UNREGISTER notifier in
unregister_netdevice_many_notify(). That is followed by synchronize_net(),
and later by the rcu_barrier() at the top of netdev_run_todo().

Now the flush runs after that rcu_barrier():

netdev_run_todo()
    __rtnl_unlock();
    rcu_barrier();
    ...
    rt_flush_dev(NULL);
    rt6_uncached_list_flush_dev(NULL);
        rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
        netdev_ref_replace(rt_dev, ...);  /* drops ref on old dev */
    netdev_wait_allrefs_any(&list);       /* returns if refcnt == 1 */
    ...
    free_netdev() / kobject_put()
        -> netdev_release()
            kvfree(dev);

netdev_release() frees the device without waiting for readers:

	/* no need to wait for rcu grace period:
	 * device is dead and about to be freed.
	 */

ip6_output() is one reader of this pointer:

	rcu_read_lock();
	dev = dst_dev_rcu(dst);

It then passes dev on to NF_HOOK_COND(), ip6_finish_output() and
dev_queue_xmit(). Suppose a reader loads the old pointer after the
rcu_barrier() but before this swap. Can it still be using dev, and its
tx queues, after they are freed?

The in6_dev_put(rt_idev) branch above has the same problem.
in6_dev_finish_destroy() drops the netdev reference before its call_rcu().

The previous patch in this series, "ipv4: Batch rt_flush_dev() in
netdev_run_todo().", uses the same ordering for IPv4.

>  				handled = true;
>  			}
> +
>  			if (handled)
>  				list_del_init(&rt->dst.rt_uncached);
>  		}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929201546.1533268-1-kuniyu%40google.com

Re: [PATCH v2 net-next 1/2] ipv4: Batch rt_flush_dev() in netdev_run_todo().

[Kuniyuki Iwashima]

On Wed, Sep 30, 2026 at 7:15 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 3 · 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] Retrying the NETDEV_UNREGISTER broadcast no longer flushes IPv4
>   uncached routes.
> - [High] Moving the IPv4 uncached-route device swap from the
>   NETDEV_UNREGISTER notifier into `netdev_run_todo()` removes the RCU…
> - [High] ipv4: NULL pointer dereference in rt_flush_dev() with
>   uninitialized list
>
> --
>
> ipv4: Batch rt_flush_dev() in netdev_run_todo().
>
> This commit takes the IPv4 uncached route flush out of the per-device
> NETDEV_UNREGISTER path. It now calls rt_flush_dev(NULL) once per batched
> unregistration in netdev_run_todo(), without holding RTNL. rt_flush_dev()
> returns early for dismantling devices. When passed NULL, it moves every
> route whose device is NETREG_UNREGISTERED over to blackhole_netdev.
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index a8eb382f40caf..9cedc17b5bdc2 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -11838,6 +11838,9 @@ void netdev_run_todo(void)
> >               linkwatch_sync_dev(dev);
> >       }
> >
> > +     if (!list_empty(&list))
> > +             rt_flush_dev(NULL);
> > +
> >       cnt = 0;
> >       while (!list_empty(&list)) {
> >               dev = netdev_wait_allrefs_any(&list);
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index d7da2f1acbb59..1b641901f10dc 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -1587,6 +1587,9 @@ void rt_flush_dev(struct net_device *dev)
> >       struct rtable *rt, *safe;
> >       int cpu;
> >
> > +     if (dev && dev->dismantle)
> > +             return;
> > +
>
> [Severity: High]
> Can this call reach rt_flush_dev() before ip_rt_init() has set up
> rt_uncached_list?

This is false positive, and IPv6 checks !&ul->head.next
just because the IPv6 init level is wrong.

Sashiko pointed out a similar one for neighbour changes, and
considering IPv4 modulise effort, I guess I should add a new init
level for IPv6.


>
> The per-cpu rt_uncached_list entries are only initialised in
> ip_rt_init(), which does INIT_LIST_HEAD() and spin_lock_init() for
> each possible CPU. That runs from inet_init(), an fs_initcall. Until
> then ul->head.next is still NULL.
>
> Before this patch, rt_flush_dev() was only reached through
> fib_netdev_event(). That notifier is registered from ip_fib_init(),
> which ip_rt_init() calls after it initialises the lists, so the
> ordering was implicitly safe.
>
> netdev_run_todo() has no such dependency. It runs from rtnl_unlock()
> whenever net_todo_list is non-empty. Any netdev unregistration before
> fs_initcall time would now call rt_flush_dev(NULL) on zeroed lists.
> One example is an earlier initcall that registers a device and then
> unregisters it on a later failure.
>
> In that case list_empty(&ul->head) returns false, because head->next
> is NULL rather than &ul->head. The loop then takes the uninitialised
> ul->lock and enters list_for_each_entry_safe(). That derives pos from
> the NULL head->next and immediately loads pos->dst.rt_uncached.next,
> which is address 0. The result is a NULL pointer dereference during
> boot.
>
> The IPv6 patch later in the series seems to handle the same case in
> rt6_uncached_list_flush_dev() by skipping a list whose head.next is
> still NULL. Should rt_flush_dev() get a similar guard? Or should
> netdev_run_todo() skip the call until IPv4 routing is initialised?
>
> [Severity: High]
> With this early return, does the NETDEV_UNREGISTER that
> netdev_wait_allrefs_any() resends still flush any IPv4 uncached routes?
>
> netdev_wait_allrefs_any() resends NETDEV_UNREGISTER about once a second so
> protocols can drop late references. That reaches
> fib_netdev_event()->rt_flush_dev(dev). dev->dismantle is already set, so
> the first notifier call and every retry now return straight away.
>
> The only flush left for a dying device is the single rt_flush_dev(NULL) in
> netdev_run_todo(). It runs once, before the wait loop, and no grace period
> follows it.
>
> Can an uncached route with the dying device be added after that pass? One
> possible path goes through xfrm_bundle_create():
>
> xfrm_bundle_create()
>     rcu_read_lock();
>     dev = dst_dev_rcu(dst);
>     ...
>     xfrm_fill_dst(xdst_prev, dev, fl)
>         xfrm4_fill_dst()

It seems this happens only for IPv4 since xfrm6_fill_dst() uses
in6_dev_get(), which returns after the last in6_dev_put().

I'll add dev_isalive() check after dst_dev_rcu() in xfrm_bundle_create()
and xfrm_create_dummy_bundle(), then they must finish before
synchronize_rcu() or fail to add a new entry.



>             netdev_hold(dev, &xdst->u.dst.dev_tracker, GFP_ATOMIC);
>             ...
>             rt_add_uncached_list(&xdst->u.rt);
>
> Say the sweep moves dst_orig to blackhole_netdev after dev is read but
> before the new xdst is linked. The xdst then goes onto rt_uncached_list
> after the sweep has already passed that CPU's list, and it still holds a
> reference on the dying device.
>
> Before this patch, synchronize_net() in unregister_netdevice_many_notify()
> ran after the notifier flush, and each retry ran rt_flush_dev(dev) again.
> Nothing now seems to come back to such an entry.
>
> Could unregister_netdevice then loop on "waiting for %s to become free" for
> as long as the xdst lives? For a bundle cached on an idle connected socket,
> that could be forever.
>
> >       for_each_possible_cpu(cpu) {
> >               struct uncached_list *ul = &per_cpu(rt_uncached_list, cpu);
> >
> > @@ -1595,10 +1598,14 @@ void rt_flush_dev(struct net_device *dev)
> >
> >               spin_lock_bh(&ul->lock);
> >               list_for_each_entry_safe(rt, safe, &ul->head, dst.rt_uncached) {
> > -                     if (rt->dst.dev != dev)
> > +                     struct net_device *rt_dev = rt->dst.dev;
> > +
> > +                     if (dev ? rt_dev != dev :
> > +                         READ_ONCE(rt_dev->reg_state) != NETREG_UNREGISTERED)
> >                               continue;
> > +
> >                       rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
> > -                     netdev_ref_replace(dev, blackhole_netdev,
> > +                     netdev_ref_replace(rt_dev, blackhole_netdev,
> >                                          &rt->dst.dev_tracker, GFP_ATOMIC);
>
> [Severity: High]
> Does moving the swap here remove the RCU grace period between publishing
> blackhole_netdev and freeing the old net_device?

This is false positive, also free_nh_exceptions() already does the
same thing.

---
pw-bot: cr

Re: [PATCH v2 net-next 2/2] ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().

[Kuniyuki Iwashima]

On Wed, Sep 30, 2026 at 7:15 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 4 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 2 · 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).
>
> - [High] Moving the IPv6 uncached-route flush into netdev_run_todo()
>   removes the RCU grace period that used to follow the device-pointer…
> - [Medium] Build failure with CONFIG_IPV6=n.
> - [Medium] The batched flush reads dev->reg_state twice per route with no
>   consistent snapshot, and another unregister batch can change reg_state…
> - [Low] The new initialization guard `if (unlikely(!ul->head.next))
>   return;` in rt6_uncached_list_flush_dev() (net/ipv6/route.c:172) is a…
>
> --
>
> ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().
>
> This patch moves the IPv6 uncached route flush out of the per-device
> NETDEV_UNREGISTER path. It now runs once per batched unregistration from
> netdev_run_todo(), without RTNL. With dev == NULL, a route's device is
> matched by reg_state == NETREG_UNREGISTERED.
>
> Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 9cedc17b5bdc2..0b3213ff56e70 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -11838,8 +11838,10 @@ void netdev_run_todo(void)
> >               linkwatch_sync_dev(dev);
> >       }
> >
> > -     if (!list_empty(&list))
> > +     if (!list_empty(&list)) {
> >               rt_flush_dev(NULL);
> > +             rt6_uncached_list_flush_dev(NULL);
>
> [Severity: Medium]
> With CONFIG_IPV6=n, is the new stub for rt6_uncached_list_flush_dev()
> visible here?
>
> net/core/dev.c doesn't include <net/ip6_route.h> directly. The only path
> seems to be net/udp_tunnel.h -> net/ip_tunnels.h, and that include is
> compiled out when IPv6 is off:
>
> include/net/ip_tunnels.h:
> #if IS_ENABLED(CONFIG_IPV6)
> #include <net/ipv6.h>
> #include <net/ip6_fib.h>
> #include <net/ip6_route.h>
> #endif
>
> So when IPv6 is disabled, dev.c would see neither the prototype nor the
> static inline stub added to include/net/ip6_route.h. Would this call then
> fail with -Werror=implicit-function-declaration? It would also fail to
> link, since net/ipv6/route.c isn't built.
>
> rt_flush_dev() doesn't hit this, because net/route.h always reaches
> dev.c through net/ip.h.
>
> Should dev.c include <net/ip6_route.h> directly, or should the
> declaration and stub go in a header that dev.c always includes?
>
> > +     }
> >
> >       cnt = 0;
> >       while (!list_empty(&list)) {
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index a76869ff87cd6..38261775e7137 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -158,14 +158,20 @@ void rt6_uncached_list_del(struct rt6_info *rt)
> >       }
> >  }
> >
> > -static void rt6_uncached_list_flush_dev(struct net_device *dev)
> > +void rt6_uncached_list_flush_dev(struct net_device *dev)
> >  {
> >       int cpu;
> >
> > +     if (dev && dev->dismantle)
> > +             return;
> > +
> >       for_each_possible_cpu(cpu) {
> >               struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
> >               struct rt6_info *rt, *safe;
> >
> > +             if (unlikely(!ul->head.next))
> > +                     return;
>
> [Severity: Low]
> This isn't a functional bug, but ul->head.next is read here without
> ul->lock. Other CPUs can update it concurrently under the lock:
> list_add_tail() in rt6_uncached_list_add() when the list is empty, and
> list_del_init() in rt6_uncached_list_del().
>
> The list_empty() check just below already uses READ_ONCE(). Should this
> be READ_ONCE(ul->head.next) so that KCSAN doesn't report it?
>
> > +
> >               if (list_empty(&ul->head))
> >                       continue;
> >
> > @@ -175,19 +181,23 @@ static void rt6_uncached_list_flush_dev(struct net_device *dev)
> >                       struct net_device *rt_dev = rt->dst.dev;
> >                       bool handled = false;
> >
> > -                     if (rt_idev && rt_idev->dev == dev) {
> > +                     if (rt_idev &&
> > +                         (dev ? rt_idev->dev == dev :
> > +                          READ_ONCE(rt_idev->dev->reg_state) == NETREG_UNREGISTERED)) {
> >                               rt->rt6i_idev = in6_dev_get(blackhole_netdev);
> >                               in6_dev_put(rt_idev);
> >                               handled = true;
> >                       }
> >
> > -                     if (rt_dev == dev) {
> > -                             rt->dst.dev = blackhole_netdev;
> > +                     if (dev ? rt_dev == dev :
> > +                         READ_ONCE(rt_dev->reg_state) == NETREG_UNREGISTERED) {
>
> [Severity: Medium]
> If two netdev_run_todo() instances overlap, can these two reg_state reads
> see different values for the same device?
>
> netdev_run_todo() calls __rtnl_unlock() before it sets each device to
> NETREG_UNREGISTERED, and that write is done under netdev_lock(), not
> ul->lock. So batch A can be scanning while batch B updates a device D
> that is on B's todo list. Take a route whose rt6i_idev->dev and dst.dev
> are both D:
>
> CPU1 (batch A, flush(NULL))          CPU2 (batch B, owns D)
> READ_ONCE(rt_idev->dev->reg_state)
>   sees NETREG_UNREGISTERING,
>   rt6i_idev is not swapped
>                                      WRITE_ONCE(D->reg_state,
>                                                 NETREG_UNREGISTERED)
> READ_ONCE(rt_dev->reg_state)
>   sees NETREG_UNREGISTERED,
>   dst.dev swapped, handled = true
> list_del_init(&rt->dst.rt_uncached)

Good catch, I'll unlink it only when both devices point to
blackhole_netdev.

And I noticed a similar bug in the current code; if we move one
of dev/idev to another netns, the route is unlinked, and removing
the netns will get stuck until socket releases the uncached route.



>
> By the time batch B's own flush runs, the route is off the list. Its
> rt6i_idev still points at D's inet6_dev, which holds a reference on D.
>
> The NETDEV_UNREGISTER rebroadcasts can't fix this either.
> rt6_disable_ip()->rt6_uncached_list_flush_dev(dev) now returns early on
> dev->dismantle, and the route is no longer listed.
>
> Could batch B then sit in netdev_wait_allrefs_any() printing "waiting for
> D to become free" for as long as something holds the uncached route? The
> window is narrow, but the old code compared pointers under RTNL, so it
> could not race this way.
>
> > +                             rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
> >                               netdev_ref_replace(rt_dev, blackhole_netdev,
> >                                                  &rt->dst.dev_tracker,
> >                                                  GFP_ATOMIC);
>
> [Severity: High]
> Is there still an RCU grace period between this pointer swap and the
> old device being freed?
>
> Before this patch, the flush ran from the NETDEV_UNREGISTER notifier in
> unregister_netdevice_many_notify(). That is followed by synchronize_net(),
> and later by the rcu_barrier() at the top of netdev_run_todo().
>
> Now the flush runs after that rcu_barrier():
>
> netdev_run_todo()
>     __rtnl_unlock();
>     rcu_barrier();
>     ...
>     rt_flush_dev(NULL);
>     rt6_uncached_list_flush_dev(NULL);
>         rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
>         netdev_ref_replace(rt_dev, ...);  /* drops ref on old dev */
>     netdev_wait_allrefs_any(&list);       /* returns if refcnt == 1 */
>     ...
>     free_netdev() / kobject_put()
>         -> netdev_release()
>             kvfree(dev);
>
> netdev_release() frees the device without waiting for readers:
>
>         /* no need to wait for rcu grace period:
>          * device is dead and about to be freed.
>          */
>
> ip6_output() is one reader of this pointer:
>
>         rcu_read_lock();
>         dev = dst_dev_rcu(dst);
>
> It then passes dev on to NF_HOOK_COND(), ip6_finish_output() and
> dev_queue_xmit(). Suppose a reader loads the old pointer after the
> rcu_barrier() but before this swap. Can it still be using dev, and its
> tx queues, after they are freed?
>
> The in6_dev_put(rt_idev) branch above has the same problem.
> in6_dev_finish_destroy() drops the netdev reference before its call_rcu().
>
> The previous patch in this series, "ipv4: Batch rt_flush_dev() in
> netdev_run_todo().", uses the same ordering for IPv4.
>
> >                               handled = true;
> >                       }
> > +
> >                       if (handled)
> >                               list_del_init(&rt->dst.rt_uncached);
> >               }
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929201546.1533268-1-kuniyu%40google.com