Re: [PATCH nf-next 4/5] netfilter: nft_set_pipapo: merge pipapo_get/lookup

[Florian Westphal <fw@strlen.de>]

Stefano Brivio <sbrivio@redhat.com> wrote:
> > -	if (unlikely(!m || !*raw_cpu_ptr(m->scratch)))
> > +	/* XXX: fix this, prealloc and remove this check */
> > +	if (unlikely(!raw_cpu_ptr(m->scratch)))
> 
> The check should be cheap, but sure, why not. I'm just asking if you
> accidentally left the XXX: here in this version or if you meant it as a
> TODO: for the future.

I can remove the XXX.  I already have a follow patch that axes this
conditional (m->scratch will always be allocated).

> >  /**
> > @@ -605,6 +536,11 @@ static struct nft_pipapo_elem *pipapo_get(const struct nft_pipapo_match *m,
> >   * @set:	nftables API set representation
> >   * @elem:	nftables API element representation containing key data
> >   * @flags:	Unused
> > + *
> > + * This function is called from the control plane path under
> > + * RCU read lock.
> > + *
> > + * Return: set element private pointer; ERR_PTR if no match.
> 
> Conceptually, we rather return -ENOENT, I'd mention that instead.

Hmm, maybe?

 * Return: set element private pointer or ERR_PTR(-ENOENT).

(Compiler should warn in case someone compares -ENOENT to return value
without the ERR macros, so maybe i am overthinking this...).

Thanks for reviewing!