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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Please drop the patch from the batch, it needs more work (see below).

On Thu, 18 Jan 2024, Eric Dumazet wrote:

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

Yeah, it calls cancel_delayed_work_sync() to handle the garbage collector 
and that can wait. The call can be moved into the main destroy function 
and let the rcu callback do just the minimal job, however it needs a 
restructuring. So please skip this patch now.

Best regards,
Jozsef
-- 
E-mail  : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxx
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
          H-1525 Budapest 114, POB. 49, Hungary

[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux