Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Ralf Lici <ralf@mandelbit.com>]

On Fri, 18 Sep 2026 12:42:40 -0600, David Ahern <dsahern@kernel.org> wrote:
> On 9/18/26 12:51 AM, Ralf Lici wrote:
> > On Thu, 17 Sep 2026 13:09:10 -0600, David Ahern <dsahern@kernel.org> wrote:
> >> On 9/17/26 4:09 AM, Ralf Lici wrote:
> >>> diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
> >>> index 7f69e8890b5b..a28974cc36e8 100644
> >>> --- a/drivers/net/ovpn/udp.c
> >>> +++ b/drivers/net/ovpn/udp.c
> >>> @@ -250,7 +252,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
> >>>  		dst_cache_reset(cache);
> >>>  	}
> >>>  
> >>> -	dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl, NULL);
> >>> +	gen0 = rt_genid_ipv6(net);
> >>
> >> this detail about routes should not be buried here. rt_genid_ipv6 really
> >> should be local to net/ipv6/route.c
> >>
> >>
> > 
> > I agree. Keeping this detail inside the IPv6 routing code is exactly why
> > I sent the RFC linked from the cover letter, and I would appreciate your
> > thoughts on the proposed approach:
> > 
> > https://lore.kernel.org/netdev/20260901122501.482920-1-ralf@mandelbit.com/
> > 
> > The pattern of obtaining a dst and only later sampling its validation
> > cookie when publishing it into a persistent cache is widespread in both
> > drivers and the networking core. It is racy for IPv6 routes with
> > rt6_info::sernum == 0, because rt6_get_cookie then reads the current
> > fib6_node::fn_sernum, not necessarily the value corresponding to the
> > lookup which returned that dst.
> > 
>
> the patch is focused on detecting a genid change at the time the dst is
> cached. Why? It can just as easily change immediately after the double
> genid check, so you are not really solving the problem.
>
> Staleness is irrelevant until it is used again (ie, genid can change
> many times between dst_cache_set and next use). That is the proper time
> to recheck the genid and refresh the dst as needed.
>

The double generation check is not intended to guarantee that the dst
cannot become stale after the final read. Its purpose is to ensure that
a pre-change dst is not cached with a post-change cookie. And that is
enough because dst_cache_get_ip6, on the next run, misses the cache if
it's no longer valid.

For example, suppose the lookup returns D0 under cookie C0. If a
relevant FIB change to C1 happens after the final generation read (even
if it happens before dst_cache_set_ip6_cookie) the cache stores (D0, C0).
On the next use, dst_cache_get_ip6 invokes the ipv6 dst check, which
compares the cached C0 with the node's current C1 and rejects the entry.

The problematic sequence is instead:

lookup returns D0 under C0
FIB changes to C1
dst_cache_set_ip6 samples C1 from D0's fib6 node
cache stores (D0, C1)

The reuse-time check then compares the cached C1 with the current C1 and
accepts D0, potentially until another relevant FIB change. The patch
ensures that the saved cookie is either coherent with the lookup or
conservatively old, so the existing check at reuse remains meaningful.

-- 
Ralf Lici
Mandelbit Srl