Re: [PATCH] netfilter: nfnetlink_queue: optimize verdict lookup with hash table

[Scott Mitchell]

On Wed, Nov 12, 2025 at 3:10 PM Florian Westphal <fw@xxxxxxxxx> wrote:
>
> Scott Mitchell <scott.k.mitch1@xxxxxxxxx> wrote:
> >  static inline u_int8_t instance_hashfn(u_int16_t queue_num)
> >  {
> >       return ((queue_num >> 8) ^ queue_num) % INSTANCE_BUCKETS;
> > @@ -114,13 +153,63 @@ instance_lookup(struct nfnl_queue_net *q, u_int16_t queue_num)
> >       return NULL;
> >  }
> >
> > +static int
> > +nfqnl_hash_resize(struct nfqnl_instance *inst, u32 hash_size)
> > +{
> > +     struct hlist_head *new_hash, *old_hash;
> > +     struct nf_queue_entry *entry;
> > +     unsigned int h, hash_mask;
> > +
> > +     /* lock scope includes kcalloc/kfree to bound memory if concurrent resizes.
> > +      * lock scope could be reduced to exclude the  kcalloc/kfree at the cost
> > +      * of increased code complexity (re-check of hash_size) and relaxed memory
> > +      * bounds (concurrent resize may each do allocations). since resize is
> > +      * expected to be rare, the broader lock scope is simpler and preferred.
> > +      */
>
> I'm all for simplicity. but I don't see how concurrent resizes are
> possible.  NFQNL_MSG_CONFIG runs under nfnetlink subsystem mutex.
>
> Or did I miss something?

This makes sense, and I will fix this.

>
> > +     new_hash = kcalloc(hash_size, sizeof(*new_hash), GFP_ATOMIC);
>
> Since the hash table could be large I would prefer if this could
> be GFP_KERNEL_ACCOUNT + kvcalloc to permit vmalloc fallback.

Good feedback, done.

>
> > +     if (nfqa[NFQA_CFG_HASH_SIZE]) {
> > +             hash_size = ntohl(nla_get_be32(nfqa[NFQA_CFG_HASH_SIZE]));
> > +     }
>
> Nit, no { } here.

Fixed.

Thanks for the review! I'll send v2 shortly.