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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFn2buBaQNSWtb8eU1Mwm-L2i2vaU4MVSjHi3OTwdd9ZyYx4RA@mail.gmail.com>
Date: Thu, 13 Nov 2025 01:14:52 -0800
From: Scott Mitchell <scott.k.mitch1@...il.com>
To: Florian Westphal <fw@...len.de>
Cc: pablo@...filter.org, kadlec@...filter.org, phil@....cc, 
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	horms@...nel.org, netfilter-devel@...r.kernel.org, coreteam@...filter.org, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Scott Mitchell <scott_mitchell@...le.com>
Subject: Re: [PATCH] netfilter: nfnetlink_queue: optimize verdict lookup with
 hash table

On Wed, Nov 12, 2025 at 3:10 PM Florian Westphal <fw@...len.de> wrote:
>
> Scott Mitchell <scott.k.mitch1@...il.com> 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.