[PATCH net] ipv6: change route cache aging logic

[PATCH net] ipv6: change route cache aging logic

[Wei Wang]

From: Wei Wang <weiwan@google.com>

In current route cache aging logic, if a route has both RTF_EXPIRE and
RTF_GATEWAY set, the route will only be removed if the neighbor cache
has no RTN_ROUTE flag. Otherwise, even if the route has expired, it
won't get deleted.
Fix this logic to always check if the route has expired first and then
do the gateway neighbor cache check if previous check decide to not
remove the exception entry.

Fixes: 1859bac04fb6 ("ipv6: remove from fib tree aged out RTF_CACHE dst")
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/route.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0458b761f3c5..a560fb1d0230 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1586,12 +1586,19 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
 	 * EXPIRES exceptions - e.g. pmtu-generated ones are pruned when
 	 * expired, independently from their aging, as per RFC 8201 section 4
 	 */
-	if (!(rt->rt6i_flags & RTF_EXPIRES) &&
-	    time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) {
-		RT6_TRACE("aging clone %p\n", rt);
+	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
+		if (time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) {
+			RT6_TRACE("aging clone %p\n", rt);
+			rt6_remove_exception(bucket, rt6_ex);
+			return;
+		}
+	} else if (time_after(jiffies, rt->dst.expires)) {
+		RT6_TRACE("purging expired route %p\n", rt);
 		rt6_remove_exception(bucket, rt6_ex);
 		return;
-	} else if (rt->rt6i_flags & RTF_GATEWAY) {
+	}
+
+	if (rt->rt6i_flags & RTF_GATEWAY) {
 		struct neighbour *neigh;
 		__u8 neigh_flags = 0;
 
@@ -1606,11 +1613,8 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
 			rt6_remove_exception(bucket, rt6_ex);
 			return;
 		}
-	} else if (__rt6_check_expired(rt)) {
-		RT6_TRACE("purging expired route %p\n", rt);
-		rt6_remove_exception(bucket, rt6_ex);
-		return;
 	}
+
 	gc_args->more++;
 }
 
-- 
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH net] ipv6: change route cache aging logic

[Martin KaFai Lau]

On Fri, Jan 26, 2018 at 11:40:17AM -0800, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
> 
> In current route cache aging logic, if a route has both RTF_EXPIRE and
> RTF_GATEWAY set, the route will only be removed if the neighbor cache
> has no RTN_ROUTE flag. Otherwise, even if the route has expired, it
You meant NTF_ROUTER instead of RTN_ROUTE?

> won't get deleted.
> Fix this logic to always check if the route has expired first and then
> do the gateway neighbor cache check if previous check decide to not
> remove the exception entry.
> 
> Fixes: 1859bac04fb6 ("ipv6: remove from fib tree aged out RTF_CACHE dst")
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Nice catch!

Acked-by: Martin KaFai Lau <kafai@fb.com>

> ---
>  net/ipv6/route.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 0458b761f3c5..a560fb1d0230 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1586,12 +1586,19 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
>  	 * EXPIRES exceptions - e.g. pmtu-generated ones are pruned when
>  	 * expired, independently from their aging, as per RFC 8201 section 4
>  	 */
> -	if (!(rt->rt6i_flags & RTF_EXPIRES) &&
> -	    time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) {
> -		RT6_TRACE("aging clone %p\n", rt);
> +	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
> +		if (time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) {
> +			RT6_TRACE("aging clone %p\n", rt);
> +			rt6_remove_exception(bucket, rt6_ex);
> +			return;
> +		}
> +	} else if (time_after(jiffies, rt->dst.expires)) {
> +		RT6_TRACE("purging expired route %p\n", rt);
>  		rt6_remove_exception(bucket, rt6_ex);
>  		return;
> -	} else if (rt->rt6i_flags & RTF_GATEWAY) {
> +	}
> +
> +	if (rt->rt6i_flags & RTF_GATEWAY) {
>  		struct neighbour *neigh;
>  		__u8 neigh_flags = 0;
>  
> @@ -1606,11 +1613,8 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
>  			rt6_remove_exception(bucket, rt6_ex);
>  			return;
>  		}
> -	} else if (__rt6_check_expired(rt)) {
> -		RT6_TRACE("purging expired route %p\n", rt);
> -		rt6_remove_exception(bucket, rt6_ex);
> -		return;
>  	}
> +
>  	gc_args->more++;
>  }
>  
> -- 
> 2.16.0.rc1.238.g530d649a79-goog
> 

Re: [PATCH net] ipv6: change route cache aging logic

[Wei Wang]

On Fri, Jan 26, 2018 at 12:05 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Fri, Jan 26, 2018 at 11:40:17AM -0800, Wei Wang wrote:
>> From: Wei Wang <weiwan@google.com>
>>
>> In current route cache aging logic, if a route has both RTF_EXPIRE and
>> RTF_GATEWAY set, the route will only be removed if the neighbor cache
>> has no RTN_ROUTE flag. Otherwise, even if the route has expired, it
> You meant NTF_ROUTER instead of RTN_ROUTE?
>
Yes. NTF_ROUTER flag. Sorry...

>> won't get deleted.
>> Fix this logic to always check if the route has expired first and then
>> do the gateway neighbor cache check if previous check decide to not
>> remove the exception entry.
>>
>> Fixes: 1859bac04fb6 ("ipv6: remove from fib tree aged out RTF_CACHE dst")
>> Signed-off-by: Wei Wang <weiwan@google.com>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Nice catch!
>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
>
>> ---
>>  net/ipv6/route.c | 20 ++++++++++++--------
>>  1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 0458b761f3c5..a560fb1d0230 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -1586,12 +1586,19 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
>>        * EXPIRES exceptions - e.g. pmtu-generated ones are pruned when
>>        * expired, independently from their aging, as per RFC 8201 section 4
>>        */
>> -     if (!(rt->rt6i_flags & RTF_EXPIRES) &&
>> -         time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) {
>> -             RT6_TRACE("aging clone %p\n", rt);
>> +     if (!(rt->rt6i_flags & RTF_EXPIRES)) {
>> +             if (time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) {
>> +                     RT6_TRACE("aging clone %p\n", rt);
>> +                     rt6_remove_exception(bucket, rt6_ex);
>> +                     return;
>> +             }
>> +     } else if (time_after(jiffies, rt->dst.expires)) {
>> +             RT6_TRACE("purging expired route %p\n", rt);
>>               rt6_remove_exception(bucket, rt6_ex);
>>               return;
>> -     } else if (rt->rt6i_flags & RTF_GATEWAY) {
>> +     }
>> +
>> +     if (rt->rt6i_flags & RTF_GATEWAY) {
>>               struct neighbour *neigh;
>>               __u8 neigh_flags = 0;
>>
>> @@ -1606,11 +1613,8 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
>>                       rt6_remove_exception(bucket, rt6_ex);
>>                       return;
>>               }
>> -     } else if (__rt6_check_expired(rt)) {
>> -             RT6_TRACE("purging expired route %p\n", rt);
>> -             rt6_remove_exception(bucket, rt6_ex);
>> -             return;
>>       }
>> +
>>       gc_args->more++;
>>  }
>>
>> --
>> 2.16.0.rc1.238.g530d649a79-goog
>>

Re: [PATCH net] ipv6: change route cache aging logic

[Paolo Abeni]

On Fri, 2018-01-26 at 11:40 -0800, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
> 
> In current route cache aging logic, if a route has both RTF_EXPIRE and
> RTF_GATEWAY set, the route will only be removed if the neighbor cache
> has no RTN_ROUTE flag. Otherwise, even if the route has expired, it
> won't get deleted.
> Fix this logic to always check if the route has expired first and then
> do the gateway neighbor cache check if previous check decide to not
> remove the exception entry.
> 
> Fixes: 1859bac04fb6 ("ipv6: remove from fib tree aged out RTF_CACHE dst")
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Thank you for the fix!

LGTM

Acked-by: Paolo Abeni <pabeni@redhat.com>

Cheers,

/P

Re: [PATCH net] ipv6: change route cache aging logic

[David Miller]

From: Wei Wang <weiwan@google.com>
Date: Fri, 26 Jan 2018 11:40:17 -0800

> From: Wei Wang <weiwan@google.com>
> 
> In current route cache aging logic, if a route has both RTF_EXPIRE and
> RTF_GATEWAY set, the route will only be removed if the neighbor cache
> has no RTN_ROUTE flag. Otherwise, even if the route has expired, it
> won't get deleted.
> Fix this logic to always check if the route has expired first and then
> do the gateway neighbor cache check if previous check decide to not
> remove the exception entry.
> 
> Fixes: 1859bac04fb6 ("ipv6: remove from fib tree aged out RTF_CACHE dst")
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queued up for -stable, thanks.