Re: [PATCH net 14/14] netfilter: ipset: fix performance regression in swap operation

[Eric Dumazet <edumazet@google.com>]

On Wed, Jan 17, 2024 at 5:00 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> From: Jozsef Kadlecsik <kadlec@netfilter.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@automattic.com>
> Reported-by: David Wang <00107082@163.com
> Tested-by: Ale Crismani <ale.crismani@automattic.com>
> Tested-by: David Wang <00107082@163.com
> Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.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