* [PATCH net] ipv6: route: purge exception on removal
@ 2019-02-20 14:08 Paolo Abeni
2019-02-20 16:00 ` Paolo Abeni
0 siblings, 1 reply; 2+ messages in thread
From: Paolo Abeni @ 2019-02-20 14:08 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
When a netdevice is unregistered, we flush the relevant exception
via rt6_sync_down_dev() -> fib6_ifdown() -> fib6_del() -> fib6_del_route().
Finally, we end-up calling rt6_remove_exception(), where we release
the relevant dst, while we keep the references to the related fib6_info and
dev. Such references should be released later when the dst will be
destroyed.
There are a number of caches that can keep the exception around for an
unlimited amount of time - namely dst_cache, possibly even socket cache.
As a result device registration may hang, as demonstrated by this script:
ip netns add cl
ip netns add rt
ip netns add srv
ip netns exec rt sysctl -w net.ipv6.conf.all.forwarding=1
ip link add name cl_veth type veth peer name cl_rt_veth
ip link set dev cl_veth netns cl
ip -n cl link set dev cl_veth up
ip -n cl addr add dev cl_veth 2001::2/64
ip -n cl route add default via 2001::1
ip -n cl link add tunv6 type ip6tnl mode ip6ip6 local 2001::2 remote 2002::1 hoplimit 64 dev cl_veth
ip -n cl link set tunv6 up
ip -n cl addr add 2013::2/64 dev tunv6
ip link set dev cl_rt_veth netns rt
ip -n rt link set dev cl_rt_veth up
ip -n rt addr add dev cl_rt_veth 2001::1/64
ip link add name rt_srv_veth type veth peer name srv_veth
ip link set dev srv_veth netns srv
ip -n srv link set dev srv_veth up
ip -n srv addr add dev srv_veth 2002::1/64
ip -n srv route add default via 2002::2
ip -n srv link add tunv6 type ip6tnl mode ip6ip6 local 2002::1 remote 2001::2 hoplimit 64 dev srv_veth
ip -n srv link set tunv6 up
ip -n srv addr add 2013::1/64 dev tunv6
ip link set dev rt_srv_veth netns rt
ip -n rt link set dev rt_srv_veth up
ip -n rt addr add dev rt_srv_veth 2002::2/64
ip netns exec srv netserver & sleep 0.1
ip netns exec cl ping6 -c 4 2013::1
ip netns exec cl netperf -H 2013::1 -t TCP_STREAM -l 3 & sleep 1
ip -n rt link set dev rt_srv_veth mtu 1400
wait %2
ip -n cl link del cl_veth
This commit addresses the issue purging all the references held by the
exception at time, as we currently do for e.g. ipv6 pcpu dst entries.
Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/route.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 964491cf3672..43a819ae46c2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1274,11 +1274,21 @@ static DEFINE_SPINLOCK(rt6_exception_lock);
static void rt6_remove_exception(struct rt6_exception_bucket *bucket,
struct rt6_exception *rt6_ex)
{
+ struct fib6_info *from;
struct net *net;
if (!bucket || !rt6_ex)
return;
+ /* purge completely the exception to allow releasing the held resources:
+ * some [sk] cache may keep the dst around for unlimited time
+ */
+ from = rcu_dereference_protected(rt6_ex->rt6i->from,
+ lockdep_is_held(&rt6_exception_lock));
+ rcu_assign_pointer(rt6_ex->rt6i->from, NULL);
+ fib6_info_release(from);
+ dst_dev_put(&rt6_ex->rt6i->dst);
+
net = dev_net(rt6_ex->rt6i->dst.dev);
hlist_del_rcu(&rt6_ex->hlist);
dst_release(&rt6_ex->rt6i->dst);
--
2.20.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH net] ipv6: route: purge exception on removal
2019-02-20 14:08 [PATCH net] ipv6: route: purge exception on removal Paolo Abeni
@ 2019-02-20 16:00 ` Paolo Abeni
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Abeni @ 2019-02-20 16:00 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
On Wed, 2019-02-20 at 15:08 +0100, Paolo Abeni wrote:
> When a netdevice is unregistered, we flush the relevant exception
> via rt6_sync_down_dev() -> fib6_ifdown() -> fib6_del() -> fib6_del_route().
>
> Finally, we end-up calling rt6_remove_exception(), where we release
> the relevant dst, while we keep the references to the related fib6_info and
> dev. Such references should be released later when the dst will be
> destroyed.
>
> There are a number of caches that can keep the exception around for an
> unlimited amount of time - namely dst_cache, possibly even socket cache.
> As a result device registration may hang, as demonstrated by this script:
>
> ip netns add cl
> ip netns add rt
> ip netns add srv
> ip netns exec rt sysctl -w net.ipv6.conf.all.forwarding=1
>
> ip link add name cl_veth type veth peer name cl_rt_veth
> ip link set dev cl_veth netns cl
> ip -n cl link set dev cl_veth up
> ip -n cl addr add dev cl_veth 2001::2/64
> ip -n cl route add default via 2001::1
>
> ip -n cl link add tunv6 type ip6tnl mode ip6ip6 local 2001::2 remote 2002::1 hoplimit 64 dev cl_veth
> ip -n cl link set tunv6 up
> ip -n cl addr add 2013::2/64 dev tunv6
>
> ip link set dev cl_rt_veth netns rt
> ip -n rt link set dev cl_rt_veth up
> ip -n rt addr add dev cl_rt_veth 2001::1/64
>
> ip link add name rt_srv_veth type veth peer name srv_veth
> ip link set dev srv_veth netns srv
> ip -n srv link set dev srv_veth up
> ip -n srv addr add dev srv_veth 2002::1/64
> ip -n srv route add default via 2002::2
>
> ip -n srv link add tunv6 type ip6tnl mode ip6ip6 local 2002::1 remote 2001::2 hoplimit 64 dev srv_veth
> ip -n srv link set tunv6 up
> ip -n srv addr add 2013::1/64 dev tunv6
>
> ip link set dev rt_srv_veth netns rt
> ip -n rt link set dev rt_srv_veth up
> ip -n rt addr add dev rt_srv_veth 2002::2/64
>
> ip netns exec srv netserver & sleep 0.1
> ip netns exec cl ping6 -c 4 2013::1
> ip netns exec cl netperf -H 2013::1 -t TCP_STREAM -l 3 & sleep 1
> ip -n rt link set dev rt_srv_veth mtu 1400
> wait %2
>
> ip -n cl link del cl_veth
>
> This commit addresses the issue purging all the references held by the
> exception at time, as we currently do for e.g. ipv6 pcpu dst entries.
>
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/ipv6/route.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 964491cf3672..43a819ae46c2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1274,11 +1274,21 @@ static DEFINE_SPINLOCK(rt6_exception_lock);
> static void rt6_remove_exception(struct rt6_exception_bucket *bucket,
> struct rt6_exception *rt6_ex)
> {
> + struct fib6_info *from;
> struct net *net;
>
> if (!bucket || !rt6_ex)
> return;
>
> + /* purge completely the exception to allow releasing the held resources:
> + * some [sk] cache may keep the dst around for unlimited time
> + */
> + from = rcu_dereference_protected(rt6_ex->rt6i->from,
> + lockdep_is_held(&rt6_exception_lock));
> + rcu_assign_pointer(rt6_ex->rt6i->from, NULL);
> + fib6_info_release(from);
> + dst_dev_put(&rt6_ex->rt6i->dst);
> +
> net = dev_net(rt6_ex->rt6i->dst.dev);
uhm... This is possibly racy, if dev goes away on dst_dev_put(). I'll
send a v2 reordering the operation so that we don't access dev (and net
) after dst_dev_put().
Sorry for the noise,
Paolo
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-02-20 16:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-20 14:08 [PATCH net] ipv6: route: purge exception on removal Paolo Abeni
2019-02-20 16:00 ` Paolo Abeni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox