Re: [PATCH nf-next 3/9] netfilter: nft_set_pipapo: prepare destroy function for on-demand clone

[Florian Westphal <fw@strlen.de>]

Stefano Brivio <sbrivio@redhat.com> wrote:
> > The rcu_barrier() is removed, its not needed: old call_rcu instances
> > for pipapo_reclaim_match do not access struct nft_set.
> 
> True, pipapo_reclaim_match() won't, but nft_set_pipapo_match_destroy()
> will, right? That is:
> 
> > -	if (m) {
> > -		rcu_barrier();
> 
> ...before b0e256f3dd2b ("netfilter: nft_set_pipapo: release elements in
> clone only from destroy path"), this rcu_barrier() was needed because we'd
> call nft_set_pipapo_match_destroy() on 'm'.
> 
> That call is now gone, and we could have dropped it at that point, but:

I do not follow.  nft_pipapo_destroy() is not invoked asynchronously via
call_rcu, its invoked from either abort path or the gc work queue at at
time where there must be no references to nft_set anymore.

What do we wait for, i.e., which outstanding rcu callback could
reference a data structure that nft_pipapo_destroy() will free?

> > +	} else {
> > +		nft_set_pipapo_match_destroy(ctx, set, m);
> 
> now it's back, so we should actually move rcu_barrier() before this
> call? I think it's needed because nft_set_pipapo_match_destroy() does
> access nft_set data.

See above, what do we need to wait for?