Re: [PATCH net] ipv6: add READ_ONCE() annotations to fib6_metrics reader paths

[Hangbin Liu <liuhangbin@gmail.com>]

On Tue, Apr 07, 2026 at 12:27:30PM +0200, Paolo Abeni wrote:
> On 4/3/26 6:16 AM, Hangbin Liu wrote:
> > diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> > index 9f8b6814a96a..2dfb04fab5da 100644
> > --- a/include/net/ip6_fib.h
> > +++ b/include/net/ip6_fib.h
> > @@ -594,7 +594,9 @@ void fib6_update_sernum_stub(struct net *net, struct fib6_info *f6i);
> >  void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val);
> >  static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
> >  {
> > -	return !!(f6i->fib6_metrics->metrics[RTAX_LOCK - 1] & (1 << metric));
> > +	struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
> > +
> > +	return !!(m->metrics[RTAX_LOCK - 1] & (1 << metric));
> 
> Sashiko notes that here you may want to add an additional READ_ONCE() on
> m->metrics[RTAX_LOCK - 1], which in turn looks like more a
> follow-up/separate change than a change specific to this patch

OK, I will drop this change in the patch.

> 
> >  }
> >  void fib6_info_hw_flags_set(struct net *net, struct fib6_info *f6i,
> >  			    bool offload, bool trap, bool offload_failed);
> > diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> > index dd26657b6a4a..3c96580c2a03 100644
> > --- a/net/ipv6/ip6_fib.c
> > +++ b/net/ipv6/ip6_fib.c
> > @@ -1144,9 +1144,11 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
> >  					iter->fib6_flags &= ~RTF_PREFIX_RT;
> >  				}
> >  
> > -				if (rt->fib6_pmtu)
> > +				u32 pmtu = READ_ONCE(rt->fib6_pmtu);
> 
> Here the READ_ONCE() on rt->metrics is still missing.

Which rt->metrics do you mean? I can't find it..


> 
> > @@ -1635,11 +1635,12 @@ __rt6_find_exception_rcu(struct rt6_exception_bucket **bucket,
> >  static unsigned int fib6_mtu(const struct fib6_result *res)
> >  {
> >  	const struct fib6_nh *nh = res->nh;
> > +	struct dst_metrics *m;
> >  	unsigned int mtu;
> >  
> > -	if (res->f6i->fib6_pmtu) {
> > -		mtu = res->f6i->fib6_pmtu;
> > -	} else {
> > +	m = READ_ONCE(res->f6i->fib6_metrics);
> > +	mtu = READ_ONCE(m->metrics[RTAX_MTU - 1]);

This is already a fib6_mtu() function.
> 
> After this patch there will be a single usage of the `fib6_pmtu` macro.
> I think it would be better to entirely drop it and replace with an helper:
> 
> static inline unsigned int fib6_mtu(const struct fib6_info *f6i)
> {
> 	const struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
> 	return READ_ONCE(m->metrics[RTAX_MTU - 1]);
> }
> 

Should we rename the helper? e.g. get_fib6_mtu ?

Hangbin