Re: [PATCH] netfilter: nft_dynset: support for element deletion

[Florian Westphal <fw@strlen.de>]

Ander Juaristi <a@juaristi.eus> wrote:
> On 13/7/19 18:59, Florian Westphal wrote:
> > 
> >> +	if (he == NULL)
> >> +		return false;
> >> +
> >> +	rhashtable_remove_fast(&priv->ht, &he->node, nft_rhash_params);
> >> +	return true;
> > 
> > Perhaps add a small comment here that rhashtable_remove_fast retval
> > is ignored intentionally?
> > 
> > I.e., don't make this return false in case two cpus race to remove same
> > entry.
> 
> Hmm, this made me think. I don't know if this was all too intentional
> from me.
> 
> Maybe rather than ignoring it, it would be better to return true only if
> rhashtable_remove_fast returned 0, which will only happen if the element
> was actually deleted (locking is done internally so two cpus cannot race
> in there). Else, if return value is -ENOENT, we should return false.
> 
> And taking this reasoning further, maybe the initial call to
> rhashtable_lookup wouldn't be needed either?

You need it to obtain he->node, no?

Wrt. retval, I might be overthinking it indeed, so making this
a "return rhashtable_remove_fast() == 0;" seems fine too, saves the
comment :-)