On 5/2/18 12:41 AM, Julian Anastasov wrote:
> Allow some non-cached routes to use non-expired fnhe:
> 
> 1. ip_del_fnhe: moved above and now called by find_exception.
> The 4.5+ commit deed49df7390 expires fnhe only when caching
> routes. Change that to:
> 
> 1.1. use fnhe for non-cached local output routes, with the help
> from (2)
> 
> 1.2. allow __mkroute_input to detect expired fnhe (outdated
> fnhe_gw, for example) when do_cache is false, eg. when itag!=0
> for unicast destinations.
> 
> 2. __mkroute_output: keep fi to allow local routes with orig_oif != 0
> to use fnhe info even when the new route will not be cached into fnhe.
> After commit 839da4d98960 ("net: ipv4: set orig_oif based on fib
> result for local traffic") it means all local routes will be affected
> because they are not cached. This change is used to solve a PMTU
> problem with IPVS (and probably Netfilter DNAT) setups that redirect
> local clients from target local IP (local route to Virtual IP)
> to new remote IP target, eg. IPVS TUN real server. Loopback has
> 64K MTU and we need to create fnhe on the local route that will
> keep the reduced PMTU for the Virtual IP. Without this change
> fnhe_pmtu is updated from ICMP but never exposed to non-cached
> local routes. This includes routes with flowi4_oif!=0 for 4.6+ and
> with flowi4_oif=any for 4.14+).

Can you add a test case to tools/testing/selftests/net/pmtu.sh to cover
this situation?


> @@ -1310,8 +1340,14 @@ static struct fib_nh_exception *find_exception(struct 
> fib_nh *nh, __be32 daddr)
>  
>       for (fnhe = rcu_dereference(hash[hval].chain); fnhe;
>            fnhe = rcu_dereference(fnhe->fnhe_next)) {
> -             if (fnhe->fnhe_daddr == daddr)
> +             if (fnhe->fnhe_daddr == daddr) {
> +                     if (fnhe->fnhe_expires &&
> +                         time_after(jiffies, fnhe->fnhe_expires)) {
> +                             ip_del_fnhe(nh, daddr);

I'm surprised this is done in the fast path vs gc time. (the existing
code does as well; your change is only moving the call to make the input
and output paths the same)


The change looks correct to me and all of my functional tests passed.

Acked-by: David Ahern <dsah...@gmail.com>

Reply via email to