[PATCH net-next v3 0/4] net: make moving a device to another netns cheaper

[PATCH net-next v3 0/4] net: make moving a device to another netns cheaper

[Qiliang Yuan]

Moving a device to another netns waits in rcu_barrier() with RTNL held.
Container and sandbox managers move one end of a veth pair for every
new network namespace, so this barrier limits how fast such namespaces
can be created on the whole system, to under 80 per second in a 4
vCPU guest however many threads create them.

The barrier was added for NETDEV_UNREGISTER_FINAL, which is long gone,
but it still keeps cached IPv4 routes through the moved device from
being accepted in the old netns, and it lets the RCU callbacks of the
nexthops removed with the device flush their IPv6 route exceptions
before the move. Routes through nexthop objects are not fully covered by
it: their cached dst entries are released by an RCU callback queued from
another RCU callback, so a socket in the old netns can still send
through a device that now belongs to another netns whenever the route
generation numbers of the two netns happen to match.

Patches 1 and 2 draw the IPv4 rt_genid and the IPv6 fib6_sernum bumps
from sequences shared by all netns, so a cached route no longer matches
another netns. Patch 3 flushes the cached routes of a nexthop group when
one of its members is removed, instead of leaving the IPv6 exceptions
through that member valid until it is freed. With these, nothing relies
on the barrier, and patch 4 removes it. Patches 1 to 3 fix bugs that
exist today and can go to net on their own if preferred.

Without patch 3, removing the barrier lets a socket keep sending through
a moved device that carries no IPv6 address and is a member of a nexthop
group, until its exceptions are flushed. Datagrams a socket in netns A
sent through such a device after it moved to netns B, 10 trials:

                   patches 1, 2, 4     patches 1-4
  datagrams               561-6525               0

Creating 400 netns, each with a veth pair whose host end is moved back
to the initial netns, a tap device, addresses and routes, in a 4 vCPU
guest, median of 5 runs:

                   before      after
  1 thread        46 /s       56 /s
  4 threads       58 /s       84 /s
  16 threads      75 /s      146 /s

Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
---
V2 -> V3:
- Add patch 3: flush the cached routes of a group when one of its
  nexthops is removed. Without it, patch 4 lets a socket keep sending
  through a moved device that has no IPv6 address (Sashiko review)
- 1/4, 2/4: say that cached routes carry values drawn from the sequence
  because inserting a route bumps it, not because NETDEV_UP does, and
  that this holds until the sequence wraps (Sashiko review)
- 1/4, 2/4: add Kuniyuki's Reviewed-by, the code changes only in comments
- 4/4: commit message only, keep Eric's and Kuniyuki's Reviewed-by
- Rebase onto net-next and remeasure in another guest, where the barrier
  costs about 4 ms per move instead of about 10 ms, so the gains are
  smaller than in v2
- Send from my linux.dev address

V1 -> V2:
- 2/3: keep the initial fib6_sernum of each netns and never hand it out
  from the shared sequence, instead of drawing a value at netns creation
  (Eric Dumazet); 1/3: do the same for rt_genid
- 2/3: note that fib6_sernum is no longer monotonic per netns (Eric
  Dumazet); 1/3: likewise for rt_genid
- Drop Kuniyuki's Reviewed-by from 1/3 and 2/3, which changed
- Rebase onto net-next and remeasure

v2: https://lore.kernel.org/r/20261019-bug-net-netns-move-rcu-barrier-v2-0-b6eb7780c492@gmail.com
v1: https://lore.kernel.org/r/20261016-bug-net-netns-move-rcu-barrier-v1-0-4157d798658f@gmail.com

---
Qiliang Yuan (4):
      ipv4: draw rt_genid from a sequence shared by all netns
      ipv6: take fib6_sernum from a sequence shared by all netns
      nexthop: flush cached routes when a nexthop leaves a group
      net: remove rcu_barrier() from __dev_change_net_namespace()

 include/net/net_namespace.h |  5 +----
 net/core/dev.c              |  1 -
 net/ipv4/nexthop.c          | 10 ++++++++++
 net/ipv4/route.c            | 21 +++++++++++++++++++++
 net/ipv6/ip6_fib.c          | 16 +++++++++++++---
 5 files changed, 45 insertions(+), 8 deletions(-)
---
base-commit: fa5196392691051963726787944db872420dfa75
change-id: 20261016-bug-net-netns-move-rcu-barrier-d998c6395830

Best regards,
-- 
Qiliang Yuan <qiliang.yuan@linux.dev>

[PATCH net-next v3 1/4] ipv4: draw rt_genid from a sequence shared by all netns

[Qiliang Yuan]

An IPv4 route records the rt_genid of the netns it was created in, and
rt_is_expired() compares it with the rt_genid of the netns its device
belongs to when the route is checked. Every netns starts its rt_genid
at 0 and increments it on each cache flush, so different netns often
hold the same value.

When a device moves to another netns, routes cached through it remain
usable until dst_dev_put() points them to blackhole_netdev. For a route
using a nexthop object this happens in nexthop_free_rcu(), which
free_fib_info_rcu() queues from inside an RCU callback, so the
rcu_barrier() in __dev_change_net_namespace() does not wait for it. If
the rt_genid of the new netns matches the cached one in that window,
ipv4_dst_check() accepts the route and a socket in the old netns keeps
sending through a device that now belongs to another netns.

Take every rt_genid bump from one sequence shared by all netns, which
never hands out 0, the value every netns starts with. Inserting a route
bumps rt_genid, so the routes a netns caches carry values drawn from the
sequence, which no other netns holds until it wraps after 2^32 bumps.
Concurrent bumps of one netns can leave the older of two values, so
rt_genid is no longer monotonic per netns. It is only compared for
equality, so this costs at most an extra route lookup. A connected UDP
socket in netns A caches a route through a nexthop object on a veth. The
veth is moved to netns B and brought up with one RTM_NEWLINK, then the
socket sends 200000 datagrams. B's rt_genid is bumped 0 to 20 times
before the move, 63 trials:

                                before     after
  trials sending after move          3         0
  datagrams sent after move       3132         0

Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info")
Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/net_namespace.h |  5 +----
 net/ipv4/route.c            | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index c29a22ca178fa..140858a2fe6d4 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -557,10 +557,7 @@ static inline int rt_genid_ipv6(const struct net *net)
 }
 #endif
 
-static inline void rt_genid_bump_ipv4(struct net *net)
-{
-	atomic_inc(&net->ipv4.rt_genid);
-}
+void rt_genid_bump_ipv4(struct net *net);
 
 extern void (*__fib6_flush_trees)(struct net *net);
 static inline void rt_genid_bump_ipv6(struct net *net)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d0a5f6d77094c..16d470ab50142 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -393,6 +393,27 @@ static inline int ip_rt_proc_init(void)
 }
 #endif /* CONFIG_PROC_FS */
 
+/*
+ * Draw rt_genid bumps from one sequence shared by all netns. A route keeps
+ * the genid of the netns it was created in, and is checked against the netns
+ * its device is in now, so distinct values make that check fail once the
+ * device has moved. Every netns starts at 0, which the sequence never hands
+ * out, and inserting a route bumps the genid, so cached routes carry values
+ * drawn from the sequence.
+ */
+static atomic_t rt_genid_seq;
+
+void rt_genid_bump_ipv4(struct net *net)
+{
+	int genid;
+
+	do {
+		genid = atomic_inc_return(&rt_genid_seq);
+	} while (!genid);
+	atomic_set(&net->ipv4.rt_genid, genid);
+}
+EXPORT_SYMBOL_GPL(rt_genid_bump_ipv4);
+
 static inline bool rt_is_expired(const struct rtable *rth)
 {
 	bool res;

-- 
2.43.0

[PATCH net-next v3 2/4] ipv6: take fib6_sernum from a sequence shared by all netns

[Qiliang Yuan]

A per-cpu IPv6 route of a nexthop object records fib6_sernum of the
netns it was created in, and rt6_is_valid() compares it with fib6_sernum
of the netns its device belongs to when the route is checked. Every netns
starts its fib6_sernum at 1 and increments it on each tree change, so
different netns often hold the same value.

When a device moves to another netns, such a route remains usable until
nexthop_free_rcu() releases it, and fib6_info_destroy_rcu() queues that
callback from inside an RCU callback, after the device has moved. If the
fib6_sernum of the new netns matches the cached one in that window,
ip6_dst_check() accepts the route and a socket in the old netns keeps
sending through a device that now belongs to another netns.

Take every fib6_sernum bump from one sequence shared by all netns, which
only hands out values from 2 up: every netns still starts at 1, and 0
means there is no sernum to check. Inserting a route bumps fib6_sernum,
so the routes a netns caches carry values drawn from the sequence, which
no other netns holds until it wraps after 2^31 bumps. fib6_new_sernum()
callers only hold their own table's tb6_lock, so two tables of one netns
can store their values in either order, and fib6_sernum is no longer
monotonic per netns. It is only compared for equality, so this costs at
most an extra invalidation. A connected UDP socket in netns A caches a
route through an IPv6 nexthop object on a veth. The veth is moved to
netns B and brought up with one RTM_NEWLINK, then the socket sends
200000 datagrams. A route is added to and removed from B 0 to 20 times
before the move, 63 trials:

                                before     after
  trials sending after move          1         0
  datagrams sent after move       3429         0

Fixes: 8f34e53b60b3 ("ipv6: Use global sernum for dst validation with nexthop objects")
Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv6/ip6_fib.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index e1d49897d6ab0..827051cbf7482 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -123,13 +123,23 @@ static void fib6_walkers_skip_rt(struct net *net, struct fib6_info *rt)
 	fib6_walkers_replace_leaf(net, rt, next);
 }
 
+/*
+ * Draw fib6_sernum bumps from one sequence shared by all netns, so that a
+ * route cached against one netns does not match the sernum of another one,
+ * e.g. after its device moved, until the sequence wraps. Every netns starts
+ * at 1 and the sequence only hands out values from 2 up: 0 means no sernum
+ * to check.
+ */
+static atomic_t fib6_sernum_seq = ATOMIC_INIT(1);
+
 static int fib6_new_sernum(struct net *net)
 {
-	int new, old = atomic_read(&net->ipv6.fib6_sernum);
+	int new, old = atomic_read(&fib6_sernum_seq);
 
 	do {
-		new = old < INT_MAX ? old + 1 : 1;
-	} while (!atomic_try_cmpxchg(&net->ipv6.fib6_sernum, &old, new));
+		new = old < INT_MAX ? old + 1 : 2;
+	} while (!atomic_try_cmpxchg(&fib6_sernum_seq, &old, new));
+	atomic_set(&net->ipv6.fib6_sernum, new);
 
 	return new;
 }

-- 
2.43.0

[PATCH net-next v3 3/4] nexthop: flush cached routes when a nexthop leaves a group

[Qiliang Yuan]

When a nexthop is removed while other members keep its group alive, only
the group array changes: the routes through the group stay in the FIB,
and the dsts they cached through the removed nexthop are released when
that nexthop is freed, from an RCU callback.

Until then those dsts stay valid. IPv6 route exceptions, such as the
ones created on Packet Too Big, are checked against the cookie of their
FIB node, which does not change, so a socket keeps sending through the
removed nexthop. When the nexthop goes away with its device, removing
the device's IPv6 addresses usually flushes the routing tree first, but
not when the device carries no IPv6 address.

Flush the cached routes of each affected group once all readers see its
new array, as replacing a group already does. A connected UDP socket in
netns A sends to a route through group {nh 1, nh 2}, on an IPv6
exception of nh 1 created by Packet Too Big, while nh 1 is deleted.
Datagrams still sent through nh 1 after the new group is published and
before nh 1 is freed, 10 trials, the one or two left after the change
being sent between synchronize_net() returning and the flush:

                          before      after
  datagrams per trial   395-4423        1-2

Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
---
 net/ipv4/nexthop.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 42e55b5a755ee..e0089a6450e75 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -2109,6 +2109,9 @@ remove_nh_grp_entry(struct net *net, struct nh_grp_entry *nhge,
 	return false;
 }
 
+static void nh_rt_cache_flush(struct net *net, struct nexthop *nh,
+			      struct nexthop *replaced_nh);
+
 static bool __must_check
 remove_nexthop_from_groups(struct net *net, struct nexthop *nh,
 			   struct nl_info *nlinfo)
@@ -2130,6 +2133,13 @@ remove_nexthop_from_groups(struct net *net, struct nexthop *nh,
 	/* make sure all see the newly published array before releasing rtnl */
 	synchronize_net();
 
+	/* Routes through the groups can still hold dsts cached through @nh,
+	 * which are only released when @nh is freed. Invalidate them now that
+	 * no reader can pick @nh from the old groups any more.
+	 */
+	list_for_each_entry(nhge, &deferred_free, nh_list)
+		nh_rt_cache_flush(net, nhge->nh_parent, nh);
+
 	/* Now safe to free percpu stats — all RCU readers have finished */
 	list_for_each_entry_safe(nhge, tmp, &deferred_free, nh_list) {
 		list_del(&nhge->nh_list);

-- 
2.43.0

[PATCH net-next v3 4/4] net: remove rcu_barrier() from __dev_change_net_namespace()

[Qiliang Yuan]

Commit 6549dd43c043 ("net: dev: fix the incorrect hold of net
namespace's lo device") added rcu_barrier() before
NETDEV_UNREGISTER_FINAL in dev_change_net_namespace(), so that dst
entries released by RCU callbacks were on the dst garbage list before
dst_ifdown() moved them to the loopback device of the old netns.
Commit 070f2d7e264a ("net: Drop NETDEV_UNREGISTER_FINAL") removed the
event but kept the barrier, and dead dst entries now point to
blackhole_netdev instead of a loopback device.

The barrier waits for the RCU callbacks pending on every CPU while RTNL
is held, which takes a full grace period on each netns move. Container
and sandbox managers move one end of a veth pair for every new network
namespace, so this limits how fast such namespaces can be created on the
whole system, regardless of how many threads create them.

Remove the barrier. The RCU callbacks queued by NETDEV_UNREGISTER
handlers free memory and drop device references, which the move does not
wait for. Routes cached through the moved device are rejected once its
netns changes: IPv4 and per-cpu IPv6 routes no longer match the rt_genid
or fib6_sernum of another netns, and the IPv6 exceptions of a nexthop
that leaves a group were already flushed when it left. Creating 400
netns, each with a veth pair whose host end is moved back to the initial
netns, a tap device, addresses and routes, in a 4 vCPU guest, median of
5 runs:

                   before      after
  1 thread        46 /s       56 /s
  4 threads       58 /s       84 /s
  16 threads      75 /s      146 /s

Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/dev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 3b60b15e2175e..de1fd77440543 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -13044,7 +13044,6 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
 	 * the device is just moving and can keep their slaves up.
 	 */
 	call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
-	rcu_barrier();
 
 	new_nsid = peernet2id_alloc(dev_net(dev), net, GFP_KERNEL);
 

-- 
2.43.0

Re: [PATCH net-next v3 2/4] ipv6: take fib6_sernum from a sequence shared by all netns

[netdev-bot+sashiko]

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 fix is incomplete.

--

ipv6: take fib6_sernum from a sequence shared by all netns

This patch makes fib6_new_sernum() take its values from one global atomic
sequence shared by all netns, which only hands out values from 2 up. The
goal is that a per-cpu nexthop route cached in one netns cannot match the
fib6_sernum of another netns after its device moves. Every netns still
starts its fib6_sernum at 1.

> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index e1d49897d6ab0..827051cbf7482 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -123,13 +123,23 @@ static void fib6_walkers_skip_rt(struct net *net, struct fib6_info *rt)
>  	fib6_walkers_replace_leaf(net, rt, next);
>  }
>  
> +/*
> + * Draw fib6_sernum bumps from one sequence shared by all netns, so that a
> + * route cached against one netns does not match the sernum of another one,
> + * e.g. after its device moved, until the sequence wraps. Every netns starts
> + * at 1 and the sequence only hands out values from 2 up: 0 means no sernum
> + * to check.
> + */
> +static atomic_t fib6_sernum_seq = ATOMIC_INIT(1);
> +
>  static int fib6_new_sernum(struct net *net)
>  {
> -	int new, old = atomic_read(&net->ipv6.fib6_sernum);
> +	int new, old = atomic_read(&fib6_sernum_seq);
>  
>  	do {
> -		new = old < INT_MAX ? old + 1 : 1;
> -	} while (!atomic_try_cmpxchg(&net->ipv6.fib6_sernum, &old, new));
> +		new = old < INT_MAX ? old + 1 : 2;
> +	} while (!atomic_try_cmpxchg(&fib6_sernum_seq, &old, new));
> +	atomic_set(&net->ipv6.fib6_sernum, new);
>  
>  	return new;
>  }

[Severity: High]
Is the value 1 still shared by every netns after this change?

inet6_net_init() in net/ipv6/af_inet6.c still does:

    atomic_set(&net->ipv6.fib6_sernum, 1);

The shared sequence never hands out 1. So a netns that has never changed
its fib6 tree stays at 1, the same value every other fresh netns starts
with.

The commit message says "Inserting a route bumps fib6_sernum, so the
routes a netns caches carry values drawn from the sequence". Does that
hold for routes cached while the insertion is still in progress?
fib6_add() publishes the route before it bumps the sernum:

fib6_add()
  fib6_add_rt2node_nh() / fib6_add_rt2node()
    rcu_assign_pointer(*ins, rt);
    inet6_rt_notify(RTM_NEWROUTE, rt, info, nlflags);
  fib6_purge_rt() for each entry on purge_list
  __fib6_update_sernum_upto_root(rt, fib6_new_sernum(info->nl_net));

In that window, a concurrent RCU-only lookup in netns A can find the new
nexthop route and go through
ip6_pol_route()->rt6_make_pcpu_route()->ip6_rt_pcpu_alloc(), which does:

    if (f6i->nh)
        pcpu_rt->sernum = rt_genid_ipv6(dev_net(dev));

If this is the first fib6 change in A, the cached value is 1.
rt6_get_cookie() then gives that value to a socket that connects at that
moment.

Once the bump happens the dst is invalid in A. But suppose the socket
does not send until the device has moved to a netns B that has never
changed its fib6 tree. rt6_is_valid() then evaluates this as 1 == 1:

    return rt6->sernum == rt_genid_ipv6(dev_net(rt6->dst.dev));

and ip6_dst_check() accepts the stale dst. Could the socket in A then
keep sending through B's device until nexthop_free_rcu() releases the
per-cpu routes? None of this needs the sequence to wrap.

Both preconditions look reachable:

In A, the nexthop device only needs IFF_UP and IPv6 enabled. addrconf
adds no routes while the link is not ready (for example a veth with
carrier off), or for IFF_NO_ADDRCONF devices.

In B, disable_ipv6 makes addrconf_notify() stop before it adds any
route. An IPv6-disabled container netns therefore stays at sernum 1
for good.

The later patch in this series, "net: remove rcu_barrier() from
__dev_change_net_namespace()", does not close this window and may make
it longer. Nothing later in the series changes the initial value or the
order of publishing and bumping.

Would it make sense to also seed each netns's initial fib6_sernum from
fib6_sernum_seq in inet6_net_init()? Alternatively, could the sernum be
bumped before the route becomes visible to readers?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109-bug-net-netns-move-rcu-barrier-v3-0-6084b02610b3%40linux.dev

Re: [PATCH net-next v3 3/4] nexthop: flush cached routes when a nexthop leaves a group

[netdev-bot+sashiko]

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] In remove_nexthop_from_groups() (net/ipv4/nexthop.c), the new
  loop calls nh_rt_cache_flush(net, nhge->nh_parent, nh) once for every…

--

nexthop: flush cached routes when a nexthop leaves a group

When a nexthop is removed and other members keep its group alive, dsts
cached through the removed nexthop could stay in use until the nexthop is
freed from an RCU callback. This patch calls nh_rt_cache_flush() for each
affected group in remove_nexthop_from_groups(), after synchronize_net().

> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index 42e55b5a755ee..e0089a6450e75 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c

[ ... ]

> @@ -2130,6 +2133,13 @@ remove_nexthop_from_groups(struct net *net, struct nexthop *nh,
>  	/* make sure all see the newly published array before releasing rtnl */
>  	synchronize_net();
>  
> +	/* Routes through the groups can still hold dsts cached through @nh,
> +	 * which are only released when @nh is freed. Invalidate them now that
> +	 * no reader can pick @nh from the old groups any more.
> +	 */
> +	list_for_each_entry(nhge, &deferred_free, nh_list)
> +		nh_rt_cache_flush(net, nhge->nh_parent, nh);

[Severity: High]
This loop runs a netns-wide flush once for every surviving group. Is
that needed, or would one flush after the loop be enough?

remove_nexthop_from_groups() is only reached from the !nh->is_group
branch of __remove_nexthop(), so @nh is never a group here. That means
each nh_rt_cache_flush() call only does the two global invalidations:

net/ipv4/nexthop.c:nh_rt_cache_flush() {
	if (!list_empty(&nh->fi_list))
		rt_cache_flush(net);

	spin_lock_bh(&nh->lock);
	have_f6i = !list_empty(&nh->f6i_list);
	spin_unlock_bh(&nh->lock);
	if (have_f6i)
		rt_genid_bump_ipv6(net);

	if (!replaced_nh->is_group)
		return;
	...
}

rt_cache_flush() is just a genid bump. rt_genid_bump_ipv6(), however,
calls fib6_flush_trees(), and __fib6_clean_all() then walks every IPv6
table in the netns while holding tb6_lock with BH disabled:

net/ipv6/ip6_fib.c:__fib6_clean_all() {
	...
			spin_lock_bh(&table->tb6_lock);
			fib6_clean_tree(net, &table->tb6_root,
					func, sernum, arg, skip_notify);
			spin_unlock_bh(&table->tb6_lock);
	...
}

The comment above __fib6_flush_trees in addrconf_core.c describes this
as "fairly expensive, be careful".

Every route through a group puts its fib6_info on the group's f6i_list
(fib6_add() does list_add(&rt->nh_list, &rt->nh->f6i_list)). So any
group that carries IPv6 routes triggers a full walk. After the first
walk, the walks for the remaining groups have no additional effect.

Before this patch, removing a nexthop from G groups did no fib6 walk.
Now it does G full walks back to back under RTNL, with no
cond_resched().

On link down or unregister, nexthop_flush_dev() calls remove_nexthop()
for every nexthop on the device, which multiplies the cost again. With
FRR using kernel nexthop groups, one peer nexthop is often a member of
many ECMP groups, and a full IPv6 table can hold around 200k routes.

CAP_NET_ADMIN in a user namespace is enough to set this up. Could that
lead to long RTNL hold times, or soft-lockup or RCU stall warnings on
!PREEMPT kernels?

The replace_nexthop() precedent cited in the commit message calls
nh_rt_cache_flush() once per operation. For groups, it uses the targeted
fib6_nh_release_dsts() instead of repeated global walks. The IPv4 side
of this file also batches through need_flush and a single fib_flush().

Would it work to collect the fi_list and f6i_list checks into two
booleans in this loop, and then call rt_cache_flush() and
rt_genid_bump_ipv6() at most once afterwards? Another option is to
release only the removed nexthop's own dsts with fib6_nh_release_dsts(),
which does not need a tree walk.

> +
>  	/* Now safe to free percpu stats — all RCU readers have finished */
>  	list_for_each_entry_safe(nhge, tmp, &deferred_free, nh_list) {
>  		list_del(&nhge->nh_list);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109-bug-net-netns-move-rcu-barrier-v3-0-6084b02610b3%40linux.dev

Re: [PATCH net-next v3 4/4] net: remove rcu_barrier() from __dev_change_net_namespace()

[netdev-bot+sashiko]

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] net: UAF in rt6_remove_exception() after
  __dev_change_net_namespace()

--

> diff --git a/net/core/dev.c b/net/core/dev.c
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -13044,7 +13044,6 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
>  	 * the device is just moving and can keep their slaves up.
>  	 */
>  	call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
> -	rcu_barrier();
>  
>  	new_nsid = peernet2id_alloc(dev_net(dev), net, GFP_KERNEL);
>  
>

[Severity: High]
The commit message says the IPv6 exceptions of a nexthop that leaves a
group were already flushed when it left, but I don't think that holds.
When remove_nexthop_from_groups() drops a single IPv6 nexthop from a
multi-member group, nh_rt_cache_flush() is called with replaced_nh ==
nh. It bumps fib6_sernum through rt_genid_bump_ipv6() and then returns
early on !replaced_nh->is_group, so fib6_nh_release_dsts() is never
called. Any rt6_ex created through a route using the group stays in
nh->nh_info->fib6_nh.rt6i_exception_bucket until nexthop_free_rcu()
runs.

That nexthop_free_rcu() is not always queued directly from the
NETDEV_UNREGISTER handler. If nh is also used directly by another IPv6
route, __remove_nexthop_fib() deletes that fib6_info. Its
fib6_purge_rt() only flushes exceptions created from itself, and the
reference on nh is only dropped by nexthop_put() in
fib6_info_destroy_rcu(). nexthop_free_rcu() is therefore queued from
inside an RCU callback. Before this patch, the rcu_barrier() here plus
the one in netdev_run_todo() drained that two-stage chain before the
old netns could go on with its pernet exit handlers.

Now consider dev being moved out of a netns that is about to exit, or
default_device_exit_net() moving it back to init_net from
default_device_exit_batch(). The moved device is not on the old
netns's todo list, so netdev_run_todo() issues a single rcu_barrier(),
which only runs fib6_info_destroy_rcu() and queues nexthop_free_rcu().
netdev_wait_allrefs_any() then sees the loopback refcount already at 1
and returns without another barrier. ops_undo_list() then runs
fib6_net_exit(), freeing net->ipv6.rt6_stats, and ip6_route_net_exit(),
calling dst_entries_destroy() on net->ipv6.ip6_dst_ops, both before
the final rcu_barrier() in cleanup_net().

When nexthop_free_rcu() finally runs, fib6_nh_release() ->
fib6_nh_release_dsts() -> fib6_nh_flush_exceptions() ->
rt6_remove_exception() decrements net->ipv6.rt6_stats->fib_rt_cache in
freed memory, and dst_release() reaches dst_entries_add() on the
destroyed pcpuc_entries counter. Nothing beyond CAP_NET_ADMIN in a
user-owned netns is needed to set this up with RTM_NEWNEXTHOP,
RTM_NEWROUTE and RTM_NEWLINK.

Chains with more stages, such as a one-member group or two members of
the same group on the moved device, look like they can outlast even
the two barriers the old code had. Removing this barrier makes the
simple case reachable as well.

Should the leaving nexthop's exceptions be flushed explicitly in
remove_nexthop_from_groups() or nh_rt_cache_flush(), so that the claim
in the commit message actually holds? Or would you rather keep the
barrier until the netns exit ordering is sorted out?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109-bug-net-netns-move-rcu-barrier-v3-0-6084b02610b3%40linux.dev