Re: [PATCH net] ipv4: route: don't blackhole in-use routes when retiring fnhe routes

[Eric Dumazet]

Le mer. 4 nov. 2026 à 19:21, Chase Hiltz <me@xxxxxxxxxxx> a écrit :
>
> When a route cached in a nexthop exception (fnhe) is replaced or
> flushed, rt_bind_exception() and fnhe_flush_routes() call
> dst_dev_put() on it. That sets dst->dev to blackhole_netdev and
> dst->input/output to dst_discard*(), but other CPUs can still be
> using the route, e.g. as a noref dst taken from the fnhe under RCU.
> Packets that reach dst_input() or dst_output() after that are dropped.
>
> If dst_dev_put() runs after a forwarded packet has entered ip_output(),
> ip_skb_dst_mtu() in __ip_finish_output() can return the MTU of
> blackhole_netdev (68), and ip_fragment() can then drop a DF packet and
> send ICMP_FRAG_NEEDED with mtu 68 to its source.
>
> This was hit on a 6.12 router that sent ICMP_FRAG_NEEDED with mtu 68
> for packets it forwarded. A kprobe on __icmp_send() showed that the
> skb's dst had dev == blackhole_netdev and still had the PMTU from its
> fnhe.
>
> Commit 5018c59607a5 ("ipv4: fix race condition between route lookup and
> invalidation") fixed the same race in rt_cache_route() by putting the
> old route on the uncached list instead of calling dst_dev_put(). Do the
> same for routes retired from an fnhe. rt_flush_dev() still moves them
> to blackhole_netdev when their device is unregistered.
>
> fnhe routes are also retired on fnhe expiry (ip_del_fnhe()) and
> eviction (fnhe_remove_oldest()). Those paths don't bump rt_genid, so
> without the obsolete marking ipv4_dst_check() could still accept the
> route. Keep marking retired routes DST_OBSOLETE_DEAD, as dst_dev_put()
> did, so that users that cached them, such as sockets, look up again.
>
> Tested on v7.4-rc1 with network namespaces: a router forwards IPv4 TCP
> (iperf3 -P 8) over an ip6tnl device whose PMTU update creates an fnhe
> for the destination, while a loop adds and deletes an IPv4 route. The
> sender drops incoming ICMP_FRAG_NEEDED, so it keeps setting DF.
>
> Over 60s without this patch, the sender received 505,291
> ICMP_FRAG_NEEDED with mtu 68, and the router dropped 67,024 packets in
> dst_discard_out() and 599 in dst_discard(). With this patch there were
> none. Neither kernel sent any without the route churn, or without an
> fnhe (ip6tnl with encaplimit none).
>
> When the sender accepted the ICMPs, its route to the destination ended
> up with "mtu lock 552" without this patch. With it, the route had no
> PMTU.
>
> Deleting the tunnel, its underlay or the peer netns under the same load
> (12 runs) completed without unregister hangs or warnings, with and
> without this patch.
>
> The same tests on a kernel with PROVE_LOCKING, KASAN, DEBUG_LIST,
> DEBUG_ATOMIC_SLEEP and the netdev/netns refcount trackers produced no
> reports. Lockdep recorded the new fnhe_lock -> uncached list lock
> dependency, and in a separate 20s run 109,305 routes were retired
> from an fnhe through the new path.
>
> Fixes: 95c47f9cf5e0 ("ipv4: call dst_dev_put() properly")
> Signed-off-by: Chase Hiltz <me@xxxxxxxxxxx>
> Co-authored-by: Claude Opus 5.5 <noreply@xxxxxxxxxxxxx>
> ---

Co-authored-by: is not a tag we use (checkpatch complains about it),
please use Assisted-by: instead.

The changelog is also quite long, you could move some of the testing
details below the --- line.

Otherwise this looks good to me, thanks.

fnhe routes are never on the uncached list while they are cached,
and the fnhe slot is cleared under fnhe_lock before
fnhe_retire_route() is called, so there is no risk of a double
list_add(). rt_flush_dev() takes care of the device reference.

Note that IPv6 has a similar issue in rt6_remove_exception().

Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>