Re: [PATCH nf-next] netfilter: nf_tables: use dedicated mutex for reset operations

[Florian Westphal <fw@strlen.de>]

Brian Witte <brianwitte@mailfence.com> wrote:
> Florian Westphal <fw@strlen.de> wrote:
> > Maybe its worth investigating if we should instead protect
> > only the reset action itself, i.e. add private reset spinlocks
> > in nft_quota_do_dump() et al?
> 
> Thanks for the suggestion. Implemented per-object spinlocks as proposed.
> Sending inline rather than v2 since I'm not certain about the approach.
> 
> Ran tests/shell/run-tests.sh with PROVE_LOCKING, PROVE_RCU, and
> PROVE_RCU_LIST enabled - no warnings.
> 
> Uses static lock class keys to avoid lockdep exhaustion with many objects.
> 
> Two questions:
> 
> 1. Should this be spin_lock_bh()? I think plain spin_lock() is fine
>    since the packet path doesn't take this lock.

Its fine if we're interrupted while holding this lock, no (soft)irq
grabs it.

> 2. The nf_tables_api.c changes also remove the try_module_get/module_put
>    and rcu_read_unlock/rcu_read_lock dance - that was only needed because
>    mutex_lock can sleep and we couldn't hold RCU across it. Since
>    spin_lock doesn't sleep, we stay under RCU the entire time. Please
>    confirm this is correct.

Yes, this dance isn't needed anymore.

> diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c
> index cc7325329496..ae3c339cbcee 100644
> --- a/net/netfilter/nft_counter.c
> +++ b/net/netfilter/nft_counter.c
> @@ -28,10 +28,13 @@ struct nft_counter_tot {
> 
>  struct nft_counter_percpu_priv {
>  	struct nft_counter __percpu *counter;
> +	spinlock_t	reset_lock;	/* protects concurrent reset */
>  };

I don't think we need per-object granularity;
a single spinlock in nft_pernet area is enough for this.