[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89iKtpVy1kSSuk_RSGN0R6L+roNJr81ED4+a2SZ2WKzGsng@mail.gmail.com>
Date: Thu, 18 Jan 2024 11:08:00 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Pablo Neira Ayuso <pablo@...filter.org>
Cc: netfilter-devel@...r.kernel.org, davem@...emloft.net,
netdev@...r.kernel.org, kuba@...nel.org, pabeni@...hat.com, fw@...len.de
Subject: Re: [PATCH net 14/14] netfilter: ipset: fix performance regression in
swap operation
On Wed, Jan 17, 2024 at 5:00 PM Pablo Neira Ayuso <pablo@...filter.org> wrote:
>
> From: Jozsef Kadlecsik <kadlec@...filter.org>
>
> The patch "netfilter: ipset: fix race condition between swap/destroy
> and kernel side add/del/test", commit 28628fa9 fixes a race condition.
> But the synchronize_rcu() added to the swap function unnecessarily slows
> it down: it can safely be moved to destroy and use call_rcu() instead.
> Thus we can get back the same performance and preventing the race condition
> at the same time.
>
> Fixes: 28628fa952fe ("netfilter: ipset: fix race condition between swap/destroy and kernel side add/del/test")
> Link: https://lore.kernel.org/lkml/C0829B10-EAA6-4809-874E-E1E9C05A8D84@automattic.com/
> Reported-by: Ale Crismani <ale.crismani@...omattic.com>
> Reported-by: David Wang <00107082@....com
> Tested-by: Ale Crismani <ale.crismani@...omattic.com>
> Tested-by: David Wang <00107082@....com
> Signed-off-by: Jozsef Kadlecsik <kadlec@...filter.org>
> Signed-off-by: Pablo Neira Ayuso <pablo@...filter.org>
> ---
> include/linux/netfilter/ipset/ip_set.h | 2 ++
> net/netfilter/ipset/ip_set_core.c | 31 +++++++++++++++++++-------
> 2 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
> index e8c350a3ade1..912f750d0bea 100644
> --- a/include/linux/netfilter/ipset/ip_set.h
> +++ b/include/linux/netfilter/ipset/ip_set.h
> @@ -242,6 +242,8 @@ extern void ip_set_type_unregister(struct ip_set_type *set_type);
>
> /* A generic IP set */
> struct ip_set {
> + /* For call_cru in destroy */
> + struct rcu_head rcu;
> /* The name of the set */
> char name[IPSET_MAXNAMELEN];
> /* Lock protecting the set data */
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index 4c133e06be1d..3bf9bb345809 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -1182,6 +1182,14 @@ ip_set_destroy_set(struct ip_set *set)
> kfree(set);
> }
>
> +static void
> +ip_set_destroy_set_rcu(struct rcu_head *head)
> +{
> + struct ip_set *set = container_of(head, struct ip_set, rcu);
> +
> + ip_set_destroy_set(set);
Calling ip_set_destroy_set() from BH (rcu callbacks) is not working.
I think you should test your patch with LOCKDEP enabled, and/or
CONFIG_DEBUG_ATOMIC_SLEEP=y
Powered by blists - more mailing lists