Re: [nf PATCH 2/5] netfilter: nf_tables: Add locking for NFT_MSG_GETRULE_RESET requests

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> wrote:
> > Can you split that into another patch?
> 
> You mean the whole creation of nf_tables_getrule_single()? Because the
> above change is only required due to the changed return type.

Yes, I was wondering if there is a way to convert the return type
in a different patch.

If its too costly, don't bother.

> > Hmm. Stupid question.  Why do we need a spinlock to serialize?
> > This is now a distinct function, so:
> 
> On Tue, Sep 05, 2023 at 11:11:07PM +0200, Phil Sutter wrote:
> [...]
> > I guess NFNL_CB_MUTEX is a no go because it locks down the whole
> > subsystem, right?
> But he didn't get a reply. :(

Sorry, missed that :-(

If thats really a concern. alernative would be to do same thing as
nft_netlink_dump_start_rcu(), i.e. use _RCU as-is and then switch
from rcu to module reference held, plus, in your case, the transaction
mutex.

Actually I like that better because we already use this pattern and
afaics all dumpers call rcu_read_lock for us; i.e.:

callback_that_might_reset()
{
	try_module_get ...
	rcu_read_unlock()
	mutex_lock(net->commit_mutex)
	  dumper();
	mutex_unlock(net->commit_mutex)
	rcu_read_lock();
	module_put()
}

should do the trick.

> What is the relation to this being a distinct function? Can't one have
> the same callback function once with type CB_RCU and once as CB_MUTEX?
> nfnetlink doesn't seem to care.

You can but you need conditional locking in that case.