netdev - Re: [PATCH v5] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aWg5yCcSrLZka854@chamomile>
Date: Thu, 15 Jan 2026 01:50:16 +0100
From: Pablo Neira Ayuso <pablo@...filter.org>
To: Scott Mitchell <scott.k.mitch1@...il.com>
Cc: kadlec@...filter.org, fw@...len.de, 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, syzbot@...kaller.appspotmail.com
Subject: Re: [PATCH v5] netfilter: nfnetlink_queue: optimize verdict lookup
 with hash table

Hi Scott,

On Tue, Jan 13, 2026 at 08:32:56PM -0500, Scott Mitchell wrote:
> > > +     NFQA_CFG_HASH_SIZE,             /* __u32 hash table size (rounded to power of 2) */
> >
> > This should use the rhashtable implementation, I don't find a good
> > reason why this is not used in first place for this enhancement.
> 
> Thank you for the review! I can make the changes. Before implementing,
> I have a few questions to ensure I understand the preferred approach:
> 
> 1. For the "perns" allocation comment - which approach did you have in mind:
>   a) Shared rhashtable in nfnl_queue_net (initialized in
> nfnl_queue_net_init) with key={queue_num, packet_id}
>   b) Per-instance rhashtable in nfqnl_instance, with lock refactoring
> so initialization happens outside rcu_read_lock

Yes, but...

Florian suggests a single rhashtable for all netns should be good
enough, you only have to include net_hash_mix(net) in the hash.

> 2. The lock refactoring (GFP_ATOMIC → GFP_KERNEL) is independent of
> the hash structure choice, correct? We could fix that separately?

No lock refactoring anymore since rhashtable would be initialized only
once for all netns, as Florian suggests.

> 3. Can you help me understand the trade-offs you considered for
> rhashtable vs hlist_head? Removing the API makes sense, and I want to
> better understand how to weigh that against runtime overhead (RCU,
> locks, atomic ops) for future design decisions.

Your approach consumes ~1Mbyte per queue instance, and we could end
up with 64k queues per-netns.

This is exposed to unprivileged containers, this allows userspace
to deplete the atomic reserves since GFP_ATOMIC is toggled, and...
there is no GFP_ATOMIC_ACCOUNT flag, then accounting does not apply in
this case.

While rhashtable a bit heavyweight, it should consume a lot less
memory and users does not have to do any hashtable bucket tunning.

> I'll use a custom hashfn to preserve the current mask-based hashing
> for the incrementing IDs.

OK.

Thanks.