Re: [PATCH v4] net: use exponential backoff in netdev_wait_allrefs

[David Miller]

From: frugg...@arista.com (Francesco Ruggeri)
Date: Fri, 18 Sep 2020 13:19:01 -0700

> The combination of aca_free_rcu, introduced in commit 2384d02520ff
> ("net/ipv6: Add anycast addresses to a global hashtable"), and
> fib6_info_destroy_rcu, introduced in commit 9b0a8da8c4c6 ("net/ipv6:
> respect rcu grace period before freeing fib6_info"), can result in
> an extra rcu grace period being needed when deleting an interface,
> with the result that netdev_wait_allrefs ends up hitting the msleep(250),
> which is considerably longer than the required grace period.
> This can result in long delays when deleting a large number of interfaces,
> and it can be observed with this script:
  ...
> Time with this patch on a 5.4 kernel:
> 
> real  0m7.704s
> user  0m0.385s
> sys   0m1.230s
> 
> Time without this patch:
> 
> real0m31.522s
> user0m0.438s
> sys 0m1.156s
> 
> v2: use exponential backoff instead of trying to wake up
> netdev_wait_allrefs.
> v3: preserve reverse christmas tree ordering of local variables
> v4: try an extra rcu_barrier before the backoff, plus some
> cosmetic changes.
> 
> Signed-off-by: Francesco Ruggeri 

Applied to net-next, thanks.

Re: [PATCH v4] net: use exponential backoff in netdev_wait_allrefs

[Eric Dumazet]

On Fri, Sep 18, 2020 at 10:19 PM Francesco Ruggeri  wrote:
>
> The combination of aca_free_rcu, introduced in commit 2384d02520ff
> ("net/ipv6: Add anycast addresses to a global hashtable"), and
> fib6_info_destroy_rcu, introduced in commit 9b0a8da8c4c6 ("net/ipv6:
> respect rcu grace period before freeing fib6_info"), can result in
> an extra rcu grace period being needed when deleting an interface,
> with the result that netdev_wait_allrefs ends up hitting the msleep(250),
> which is considerably longer than the required grace period.
> This can result in long delays when deleting a large number of interfaces,
> and it can be observed with this script:
>
>
>
> Signed-off-by: Francesco Ruggeri 
>

Reviewed-by: Eric Dumazet 

Thanks !

[PATCH v4] net: use exponential backoff in netdev_wait_allrefs

[Francesco Ruggeri]

The combination of aca_free_rcu, introduced in commit 2384d02520ff
("net/ipv6: Add anycast addresses to a global hashtable"), and
fib6_info_destroy_rcu, introduced in commit 9b0a8da8c4c6 ("net/ipv6:
respect rcu grace period before freeing fib6_info"), can result in
an extra rcu grace period being needed when deleting an interface,
with the result that netdev_wait_allrefs ends up hitting the msleep(250),
which is considerably longer than the required grace period.
This can result in long delays when deleting a large number of interfaces,
and it can be observed with this script:

ns=dummy-ns
NIFS=100

ip netns add $ns
ip netns exec $ns ip link set lo up
ip netns exec $ns sysctl net.ipv6.conf.default.disable_ipv6=0
ip netns exec $ns sysctl net.ipv6.conf.default.forwarding=1

for ((i=0; i<$NIFS; i++))
do
if=eth$i
ip netns exec $ns ip link add $if type dummy
ip netns exec $ns ip link set $if up
ip netns exec $ns ip -6 addr add 2021:$i::1/120 dev $if
done

for ((i=0; i<$NIFS; i++))
do
if=eth$i
ip netns exec $ns ip link del $if
done

ip netns del $ns

Instead of using a fixed msleep(250), this patch tries an extra
rcu_barrier() followed by an exponential backoff.

Time with this patch on a 5.4 kernel:

real0m7.704s
user0m0.385s
sys 0m1.230s

Time without this patch:

real0m31.522s
user0m0.438s
sys 0m1.156s

v2: use exponential backoff instead of trying to wake up
netdev_wait_allrefs.
v3: preserve reverse christmas tree ordering of local variables
v4: try an extra rcu_barrier before the backoff, plus some
cosmetic changes.

Signed-off-by: Francesco Ruggeri 
---
 net/core/dev.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 4086d335978c..cef4a5ea24d0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9986,10 +9986,12 @@ EXPORT_SYMBOL(netdev_refcnt_read);
  * We can get stuck here if buggy protocols don't correctly
  * call dev_put.
  */
+#define WAIT_REFS_MIN_MSECS 1
+#define WAIT_REFS_MAX_MSECS 250
 static void netdev_wait_allrefs(struct net_device *dev)
 {
unsigned long rebroadcast_time, warning_time;
-   int refcnt;
+   int wait = 0, refcnt;
 
linkwatch_forget_dev(dev);
 
@@ -10023,7 +10025,13 @@ static void netdev_wait_allrefs(struct net_device *dev)
rebroadcast_time = jiffies;
}
 
-   msleep(250);
+   if (!wait) {
+   rcu_barrier();
+   wait = WAIT_REFS_MIN_MSECS;
+   } else {
+   msleep(wait);
+   wait = min(wait << 1, WAIT_REFS_MAX_MSECS);
+   }
 
refcnt = netdev_refcnt_read(dev);
 
-- 
2.28.0