Re: [PATCH net-next 1/3] ipv4: properly refresh rtable entries on pmtu/redirect events

[Timo Teras <timo.teras@iki.fi>]

On Tue, 28 May 2013 11:25:55 +0300 (EEST)
Julian Anastasov <ja@ssi.bg> wrote:

> 
> 	Hello,
> 
> On Tue, 28 May 2013, Timo Teräs wrote:
> 
> > This reverts commit 05ab86c5 (xfrm4: Invalidate all ipv4 routes on
> > IPsec pmtu events). Flushing all cached entries is not needed.
> > 
> > Instead, invalidate only the related next hop dsts to recheck for
> > the added next hop exception where needed. This also fixes a subtle
> > race due to bumping generation id's before updating the pmtu.
> > 
> > Cc: Steffen Klassert <steffen.klassert@secunet.com>
> > Signed-off-by: Timo Teräs <timo.teras@iki.fi>
> > ---
> 
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 550781a..561a378 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -594,11 +594,25 @@ static inline u32 fnhe_hashfun(__be32 daddr)
> >  	return hval & (FNHE_HASH_SIZE - 1);
> >  }
> >  
> > +static void fill_route_from_fnhe(struct rtable *rt, struct
> > fib_nh_exception *fnhe) +{
> > +	rt->rt_pmtu = fnhe->fnhe_pmtu;
> > +	rt->dst.expires = fnhe->fnhe_expires;
> 
> 	The 'if (time_before' ... dst_set_expires() logic from
> rt_bind_exception() is removed, may be it should be moved here,
> i.e. fnhe_pmtu should be ignored if expired.

That code would not help much. The route's rt_pmtu is never reset to
zero after the fnhe expires, so this would not make much difference.
The old rt_pmtu and dst.expires would be left there anyway. All rt
accesses check for expires too.

This was actually intentional on the old code, as non-zero rt_pmtu
implied that we had "next hop exception route" instead of "next hop
route" and affected how the rt was invalidated in pmtu update.

If we want to clear out these fields, then it would make sense to have
rt_bind_exception() to reset these on expiry to the struct
fib_nh_exception directly and keep the direct assignments in
fill_route_from_fnhe().

- Timo