ipv6: fixes for RTF_CACHE entries
Subsystems (3)
Paolo Abeni
maintainerv2 cover ipv6: fixes for RTF_CACHE entries
This series addresses 3 different but related issues with RTF_CACHE introduced by the recent refactory.
patch 1 restore the gc timer for such routes patch 2 removes the aged out dst from the fib tree
v1 -> v2:
- dropped the for ip route show cache
- avoid touching dst.obsolete when the dst is aged out
Paolo Abeni (2):
ipv6: start fib6 gc on RTF_CACHE dst creation
ipv6: remove from fib tree aged out RTF_CACHE dst
net/ipv6/route.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
Signature · 1 line
2.13.6
Paolo Abeni
maintainerPATCH v2 1/2 ipv6: start fib6 gc on RTF_CACHE dst creation
After the commit Fixes: 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache"), the fib6 gc is not started after the creation of a RTF_CACHE via a redirect or pmtu update, since fib6_add() isn't invoked anymore for such dsts.
We need the fib6 gc to run periodically to clean the RTF_CACHE, or the dst will stay there forever.
Fix it by explicitly calling fib6_force_start_gc() on successful exception creation. gc_args->more accounting will ensure that the gc timer will run for whatever time needed to properly clean the table.
--- net/ipv6/route.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
net/ipv6/route.c +3 -1
@@ -1340,8 +1340,10 @@ static int rt6_insert_exception(struct rt6_info *nrt,spin_unlock_bh (&rt6_exception_lock);/* Update fn->fn_sernum to invalidate all cached dst */ -if (!err) +if (!err) {fib6_update_sernum (ort); +fib6_force_start_gc (net); + }return err; }
Signature · 1 line
2.13.6
Re: [PATCH net-next v2 1/2] ipv6: start fib6 gc on RTF_CACHE dst creation
Hello!
Quoted 1 line · Paolo Abeni
After the commit Fixes:
2b760fcf5cfb("ipv6: hook up exception
Quoted 12 lines
table to store dst cache"), the fib6 gc is not started after the creation of a
RTF_CACHEvia a redirect or pmtu update, sincefib6_add()isn't invoked anymore for such dsts.We need the fib6 gc to run periodically to clean the
RTF_CACHE, or the dst will stay there forever.Fix it by explicitly calling
fib6_force_start_gc()on successful exception creation.gc_args->moreaccounting will ensure that the gc timer will run for whatever time needed to properly clean the table.
Quoted 4 lines
Fixes 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")Signed-off-by Paolo Abeni <[email protected]>Acked-by Wei Wang <[email protected]>Acked-by Martin KaFai Lau <[email protected]>
[...]
MBR, Sergei
Paolo Abeni
maintainerPATCH v2 2/2 ipv6: remove from fib tree aged out RTF_CACHE dst
In reply to Paolo AbeniThe commit 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache") partially reverted 1e2ea8ad37be ("ipv6: set dst.obsolete when a cached route has expired").
As a result, RTF_CACHE dst referenced outside the fib tree will not be removed until the next sernum change; dst_check() does not fail on aged-out dst, and dst->__refcnt can't decrease: the aged out dst will stay valid for a potentially unlimited time after the timeout expiration.
This change explicitly removes RTF_CACHE dst from the fib tree when aged out. The rt6_remove_exception() logic will then obsolete the dst and other entities will drop the related reference on next dst_check().
v1 -> v2:
- do not touch dst.obsolete in rt6_remove_exception(), not needed
--- net/ipv6/route.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
net/ipv6/route.c +5 -2
@@ -1575,8 +1575,11 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket, {struct rt6_info *rt = rt6_ex->rt6i; -if (atomic_read (&rt->dst.__refcnt) ==1 && -time_after_eq (now, rt->dst.lastuse + gc_args->timeout)) { +/* we are pruning and obsoleting the exception route even if others +* have still reference to it, so that on next dst_check() such +* reference can be dropped +*/ +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 ;
Signature · 1 line
2.13.6
Re: [PATCH net-next v2 2/2] ipv6: remove from fib tree aged out RTF_CACHE dst
Quoted 29 lines · Paolo Abeni
The commit
2b760fcf5cfb("ipv6: hook up exception table to store dst cache") partially reverted1e2ea8ad37be("ipv6: set dst.obsolete when a cached route has expired").As a result,
RTF_CACHEdst referenced outside the fib tree will not be removed until the next sernum change;dst_check()does not fail on aged-out dst, anddst->__refcntcan't decrease: the aged out dst will stay valid for a potentially unlimited time after the timeout expiration.This change explicitly removes
RTF_CACHEdst from the fib tree when aged out. Thert6_remove_exception()logic will then obsolete the dst and other entities will drop the related reference on nextdst_check().v1 -> v2:
- do not touch dst.obsolete inrt6_remove_exception(), not neededFixes 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")Signed-off-by Paolo Abeni <[email protected]>--- net/ipv6/route.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)net/ipv6/route.c +0
-0
net/ipv6/route.c static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket, · line 1578
Quoted 4 lines
net/ipv6/route.c +0
-1@@ -1575,8 +1575,11 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket, {struct rt6_info *rt = rt6_ex->rt6i; - if (atomic_read(&rt->dst.__refcnt) == 1 &&
Wei and I have discussed a few cases:
1) For PMTU rt, the dst.expires should have been set (to ip6_rt_mtu_expires) and the next dst_check() should have returned NULL. The ip6_pol_route() should not have returned this expired pmtu rt also.
The ip6_rt_mtu_expires should be used to decide if the rt is no longer useful instead of gc_args->timeout.
We should not ignore __refcnt in this case. Otherwise, we are punishing sk that is caching the rt since lastuse
is only updated during the route lookup.
2) The cases that we can ignore __refcnt are:
a) RTF_CACHE rt that has no expire set. I believe only redirect rt falls
into this bucket. In this case, we can use gc_args->timeout.
b) If there is an urgency to cleanup (like reaching ip6_rt_max_size) or user force a flush.
Quoted 12 lines
net/ipv6/route.c +5
-1- time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) { +/* we are pruning and obsoleting the exception route even if others +* have still reference to it, so that on next dst_check() such +* reference can be dropped +*/ +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 ;Signature · 1 line
2.13.6