[PATCH net-next 0/6] net: avoid slow rcu synchronizations in cleanup_net()

[PATCH net-next 0/6] net: avoid slow rcu synchronizations in cleanup_net()

[Eric Dumazet]

RTNL is a contended mutex, we prefer to expedite rcu synchronizations
in contexts we hold RTNL.

Similarly, cleanup_net() is a single threaded critical component and
should also use synchronize_rcu_expedited() even when not holding RTNL.

First patch removes a barrier with no clear purpose in ipv6_mc_down()

Eric Dumazet (6):
  ipv6: mcast: remove one synchronize_net() barrier in ipv6_mc_down()
  net: use synchronize_net() in dev_change_name()
  bridge: vlan: use synchronize_net() when holding RTNL
  ipv4/fib: use synchronize_net() when holding RTNL
  net: use synchronize_rcu_expedited in cleanup_net()
  netfilter: conntrack: expedite rcu in nf_conntrack_cleanup_net_list

 net/bridge/br_vlan.c              | 4 ++--
 net/core/dev.c                    | 2 +-
 net/core/net_namespace.c          | 2 +-
 net/ipv4/fib_trie.c               | 2 +-
 net/ipv6/mcast.c                  | 1 -
 net/netfilter/nf_conntrack_core.c | 2 +-
 6 files changed, 6 insertions(+), 7 deletions(-)

-- 
2.43.0.687.g38aa6559b0-goog

[PATCH net-next 1/6] ipv6: mcast: remove one synchronize_net() barrier in ipv6_mc_down()

[Eric Dumazet]

As discussed in the past (commit 2d3916f31891 ("ipv6: fix skb drops
in igmp6_event_query() and igmp6_event_report()")) I think the
synchronize_net() call in ipv6_mc_down() is not needed.

Under load, synchronize_net() can last between 200 usec and 5 ms.

KASAN seems to agree as well.

Fixes: f185de28d9ae ("mld: add new workqueues for process mld events")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Taehee Yoo <ap420073@gmail.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: David Ahern <dsahern@kernel.org>
---
 net/ipv6/mcast.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index bc6e0a0bad3c12d641a1dc60a8c790a6e72b1b5f..76ee1615ff2a07e1dd496aada377a7feb4703e8c 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -2719,7 +2719,6 @@ void ipv6_mc_down(struct inet6_dev *idev)
 	/* Should stop work after group drop. or we will
 	 * start work again in mld_ifc_event()
 	 */
-	synchronize_net();
 	mld_query_stop_work(idev);
 	mld_report_stop_work(idev);
 
-- 
2.43.0.687.g38aa6559b0-goog

[PATCH net-next 2/6] net: use synchronize_net() in dev_change_name()

[Eric Dumazet]

dev_change_name() holds RTNL, we better use synchronize_net()
instead of plain synchronize_rcu().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index e52e2888cccd37e0df794155004e77261c812ef9..cb0eaefa0aff79ddf009f0867b440200fa985e54 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1239,7 +1239,7 @@ int dev_change_name(struct net_device *dev, const char *newname)
 	netdev_name_node_del(dev->name_node);
 	write_unlock(&dev_base_lock);
 
-	synchronize_rcu();
+	synchronize_net();
 
 	write_lock(&dev_base_lock);
 	netdev_name_node_add(net, dev->name_node);
-- 
2.43.0.687.g38aa6559b0-goog

[PATCH net-next 3/6] bridge: vlan: use synchronize_net() when holding RTNL

[Eric Dumazet]

br_vlan_flush() and nbp_vlan_flush() should use synchronize_net()
instead of syncronize_rcu() to release RTNL sooner.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/bridge/br_vlan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 15f44d026e75a8818f958703c5ec054eaafc4d94..9c2fffb827ab195cf9a01281e4790361e0b14bfe 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -841,7 +841,7 @@ void br_vlan_flush(struct net_bridge *br)
 	vg = br_vlan_group(br);
 	__vlan_flush(br, NULL, vg);
 	RCU_INIT_POINTER(br->vlgrp, NULL);
-	synchronize_rcu();
+	synchronize_net();
 	__vlan_group_free(vg);
 }
 
@@ -1372,7 +1372,7 @@ void nbp_vlan_flush(struct net_bridge_port *port)
 	vg = nbp_vlan_group(port);
 	__vlan_flush(port->br, port, vg);
 	RCU_INIT_POINTER(port->vlgrp, NULL);
-	synchronize_rcu();
+	synchronize_net();
 	__vlan_group_free(vg);
 }
 
-- 
2.43.0.687.g38aa6559b0-goog

[PATCH net-next 4/6] ipv4/fib: use synchronize_net() when holding RTNL

[Eric Dumazet]

tnode_free() should use synchronize_net()
instead of syncronize_rcu() to release RTNL sooner.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/fib_trie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 3ff35f811765591638c5e03d9211138ca6df3603..0fc7ab5832d1ae00e33fdf6fad4ef379c7d0bd4d 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -501,7 +501,7 @@ static void tnode_free(struct key_vector *tn)
 
 	if (tnode_free_size >= READ_ONCE(sysctl_fib_sync_mem)) {
 		tnode_free_size = 0;
-		synchronize_rcu();
+		synchronize_net();
 	}
 }
 
-- 
2.43.0.687.g38aa6559b0-goog

[PATCH net-next 5/6] net: use synchronize_rcu_expedited in cleanup_net()

[Eric Dumazet]

cleanup_net() is calling synchronize_rcu() right before
acquiring RTNL.

synchronize_rcu() is much slower than synchronize_rcu_expedited(),
and cleanup_net() is currently single threaded. In many workloads
we want cleanup_net() to be fast, in order to free memory and various
sysfs and procfs entries as fast as possible.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/net_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 233ec0cdd0111d5ca21c6f8a66f4c1f3fbc4657b..f0540c5575157135b1dc5dece2220f81a408fb7e 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -622,7 +622,7 @@ static void cleanup_net(struct work_struct *work)
 	 * the rcu_barrier() below isn't sufficient alone.
 	 * Also the pre_exit() and exit() methods need this barrier.
 	 */
-	synchronize_rcu();
+	synchronize_rcu_expedited();
 
 	rtnl_lock();
 	list_for_each_entry_reverse(ops, &pernet_list, list) {
-- 
2.43.0.687.g38aa6559b0-goog

[PATCH net-next 6/6] netfilter: conntrack: expedite rcu in nf_conntrack_cleanup_net_list

[Eric Dumazet]

nf_conntrack_cleanup_net_list() is calling synchronize_net()
while RTNL is not held. This effectively calls synchronize_rcu().

synchronize_rcu() is much slower than synchronize_rcu_expedited(),
and cleanup_net() is currently single threaded. In many workloads
we want cleanup_net() to be faster, in order to free memory and various
sysfs and procfs entries as fast as possible.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_conntrack_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 2e5f3864d353a39cfde138b725e790d7290b82c9..90e6bd2c30002cbf45f417f014e8493c8d24984b 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2530,7 +2530,7 @@ void nf_conntrack_cleanup_net_list(struct list_head *net_exit_list)
 	 *  netfilter framework.  Roll on, two-stage module
 	 *  delete...
 	 */
-	synchronize_net();
+	synchronize_rcu_expedited();
 i_see_dead_people:
 	busy = 0;
 	list_for_each_entry(net, net_exit_list, exit_list) {
-- 
2.43.0.687.g38aa6559b0-goog

Re: [PATCH net-next 3/6] bridge: vlan: use synchronize_net() when holding RTNL

[Nikolay Aleksandrov]

On 2/9/24 17:30, Eric Dumazet wrote:
> br_vlan_flush() and nbp_vlan_flush() should use synchronize_net()
> instead of syncronize_rcu() to release RTNL sooner.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>   net/bridge/br_vlan.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 15f44d026e75a8818f958703c5ec054eaafc4d94..9c2fffb827ab195cf9a01281e4790361e0b14bfe 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -841,7 +841,7 @@ void br_vlan_flush(struct net_bridge *br)
>   	vg = br_vlan_group(br);
>   	__vlan_flush(br, NULL, vg);
>   	RCU_INIT_POINTER(br->vlgrp, NULL);
> -	synchronize_rcu();
> +	synchronize_net();
>   	__vlan_group_free(vg);
>   }
>   
> @@ -1372,7 +1372,7 @@ void nbp_vlan_flush(struct net_bridge_port *port)
>   	vg = nbp_vlan_group(port);
>   	__vlan_flush(port->br, port, vg);
>   	RCU_INIT_POINTER(port->vlgrp, NULL);
> -	synchronize_rcu();
> +	synchronize_net();
>   	__vlan_group_free(vg);
>   }
>   

Also CCed Roopa. Thanks,
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 4/6] ipv4/fib: use synchronize_net() when holding RTNL

[David Ahern]

On 2/9/24 8:30 AM, Eric Dumazet wrote:
> tnode_free() should use synchronize_net()
> instead of syncronize_rcu() to release RTNL sooner.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/ipv4/fib_trie.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next 0/6] net: avoid slow rcu synchronizations in cleanup_net()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri,  9 Feb 2024 15:30:55 +0000 you wrote:
> RTNL is a contended mutex, we prefer to expedite rcu synchronizations
> in contexts we hold RTNL.
> 
> Similarly, cleanup_net() is a single threaded critical component and
> should also use synchronize_rcu_expedited() even when not holding RTNL.
> 
> First patch removes a barrier with no clear purpose in ipv6_mc_down()
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] ipv6: mcast: remove one synchronize_net() barrier in ipv6_mc_down()
    https://git.kernel.org/netdev/net-next/c/17ef8efc00b3
  - [net-next,2/6] net: use synchronize_net() in dev_change_name()
    https://git.kernel.org/netdev/net-next/c/4cd582ffa5a9
  - [net-next,3/6] bridge: vlan: use synchronize_net() when holding RTNL
    https://git.kernel.org/netdev/net-next/c/48ebf6ebbc91
  - [net-next,4/6] ipv4/fib: use synchronize_net() when holding RTNL
    https://git.kernel.org/netdev/net-next/c/2cd0c51e3baf
  - [net-next,5/6] net: use synchronize_rcu_expedited in cleanup_net()
    https://git.kernel.org/netdev/net-next/c/78c3253f27e5
  - [net-next,6/6] netfilter: conntrack: expedite rcu in nf_conntrack_cleanup_net_list
    https://git.kernel.org/netdev/net-next/c/1ebb85f9c03d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html