* [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
@ 2026-11-06 0:37 Alireza Asgari via B4 Relay
2026-11-06 0:37 ` [PATCH RFC net 1/3] netfilter: nfnetlink_queue: reject duplicate pending packet IDs Alireza Asgari via B4 Relay
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Alireza Asgari via B4 Relay @ 2026-11-06 0:37 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal
Cc: Phil Sutter, netfilter-devel, netdev, kunit-dev, Alireza Asgari
This series addresses packet-ID reuse and batch-verdict ordering in
NFQUEUE. I am posting it as an RFC to discuss the handling of ambiguous
batch boundaries and the additional sequence state used to resolve them.
An old packet can remain queued while newer packets receive individual
verdicts. Queue length therefore does not bound the distance between
pending packet IDs. After a complete 32-bit ID cycle, two pending packets
can have the same ID, making verdict lookup ambiguous. Even before reuse,
the signed 32-bit comparison in the batch handler can select the wrong
FIFO prefix when IDs are separated by half an ID cycle or more.
For example, with pending IDs [1, 0x80000002], a batch verdict through 1
currently selects both packets instead of only the first. If the queue
contains [1, 2, 0x80000003], removing 2 individually and then batching
through 2 also selects both remaining packets instead of only 1. These
cases are covered by synthetic KUnit tests that seed the sequence values
directly.
Patch 1 rejects an incoming duplicate ID before list insertion or userspace
notification, preserving the existing pending packet. The incoming packet
follows the existing enqueue-failure policy: drop by default, or accept
with FAIL_OPEN.
Patch 2 uses an exact queued boundary to select the FIFO prefix. For a
boundary no longer queued, it uses a 64-bit queue sequence and a per-entry
32-bit epoch to recover issuance order. Ambiguous missing boundaries
return -ERANGE without changing the queue. The wire format is unchanged,
but the rejection behavior is userspace-visible. This does not distinguish
stale verdicts after an ID has been released and reused.
Patch 3 adds 45 KUnit cases covering ordering, wraparound, missing
boundaries, collisions and the actual enqueue path. Each case uses a
private network namespace.
I would appreciate feedback on:
- Whether -ERANGE with the queue unchanged is the appropriate response
for an ambiguous missing batch boundary.
- Whether the per-entry epoch and widened queue counter are appropriate
for handling boundaries that have already been removed.
- Whether the existing drop/FAIL_OPEN policy is the preferred way to
handle an incoming packet whose ID is still pending.
Validation:
- Focused UML KUnit run: 45 cases passed with GCC 13.3 and W=1.
- Kconfig dependency checks with NFQUEUE built in: tests enabled with
KUNIT=y and NET_NS=y, and disabled with KUNIT=m or NET_NS=n.
- Full x86-64 allmodconfig with GCC 13.3 and allyesconfig with Clang
18.1.3: both passed with W=1 and CONFIG_WERROR=n.
- Separate GCC builds of NFQUEUE and all eight other direct users of
nf_queue.h passed in both configurations with W=1 and
CONFIG_WERROR=y, without compiler diagnostics.
The full builds disabled WERROR after unrelated failures were reproduced
on the unpatched base. GCC allyesconfig also encountered an unrelated
MCTP KUnit static assertion failure on that base; the full allyesconfig
build therefore used Clang. Runtime coverage is the focused KUnit suite;
no throughput or latency measurements are included.
Codex assisted with developing and reviewing this series, fixing the
KUnit dependencies and namespace isolation, running validation, and
preparing the submission text.
---
Alireza Asgari (3):
netfilter: nfnetlink_queue: reject duplicate pending packet IDs
netfilter: nfnetlink_queue: fix batch verdict ordering across large ID gaps
netfilter: nfnetlink_queue: test packet ID ordering and collisions
include/net/netfilter/nf_queue.h | 1 +
net/netfilter/Kconfig | 12 +
net/netfilter/nfnetlink_queue.c | 78 ++-
net/netfilter/nfnetlink_queue_gap_test.c | 906 +++++++++++++++++++++++++++++++
4 files changed, 988 insertions(+), 9 deletions(-)
---
base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
change-id: 20261106-nfqueue-review-1840e52eafdb
Best regards,
--
Alireza Asgari <alireza@asgari.net>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH RFC net 1/3] netfilter: nfnetlink_queue: reject duplicate pending packet IDs
2026-11-06 0:37 [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps Alireza Asgari via B4 Relay
@ 2026-11-06 0:37 ` Alireza Asgari via B4 Relay
2026-11-06 0:37 ` [PATCH RFC net 2/3] netfilter: nfnetlink_queue: fix batch verdict ordering across large ID gaps Alireza Asgari via B4 Relay
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Alireza Asgari via B4 Relay @ 2026-11-06 0:37 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal
Cc: Phil Sutter, netfilter-devel, netdev, kunit-dev, Alireza Asgari
From: Alireza Asgari <alireza@asgari.net>
A queued packet can outlive a complete 32-bit ID cycle while newer
packets receive verdicts. rhashtable_insert_fast() permits equal keys,
so the reused ID can identify two pending packets and verdict lookup
can select the wrong entry.
Use rhashtable_lookup_insert_fast() under the queue lock to reject an
incoming collision before list insertion or userspace notification.
Preserve the pending packet and use the existing enqueue-failure path:
drop the incoming packet by default, or accept it with FAIL_OPEN.
The attempted ID is consumed and the next enqueue advances normally.
This enforces uniqueness among pending entries. The unchanged 32-bit
interface cannot distinguish stale verdicts after an ID is released
and reused.
Codex assisted with development, review and validation of this change.
Fixes: 7af4cc3fa158 ("[NETFILTER]: Add "nfnetlink_queue" netfilter queue handler over nfnetlink")
Signed-off-by: Alireza Asgari <alireza@asgari.net>
---
net/netfilter/nfnetlink_queue.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 90d887546f7db..c71f4fe890eee 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -227,8 +227,10 @@ __enqueue_entry(struct nfqnl_instance *queue, struct nf_queue_entry *entry)
{
int err;
- err = rhashtable_insert_fast(&queue->nfqnl_packet_map, &entry->hash_node,
- nfqnl_rhashtable_params);
+ /* A wrapped ID must not alias a packet that is still queued. */
+ err = rhashtable_lookup_insert_fast(&queue->nfqnl_packet_map,
+ &entry->hash_node,
+ nfqnl_rhashtable_params);
if (unlikely(err))
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC net 2/3] netfilter: nfnetlink_queue: fix batch verdict ordering across large ID gaps
2026-11-06 0:37 [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps Alireza Asgari via B4 Relay
2026-11-06 0:37 ` [PATCH RFC net 1/3] netfilter: nfnetlink_queue: reject duplicate pending packet IDs Alireza Asgari via B4 Relay
@ 2026-11-06 0:37 ` Alireza Asgari via B4 Relay
2026-11-06 4:45 ` Florian Westphal
2026-11-06 0:37 ` [PATCH RFC net 3/3] netfilter: nfnetlink_queue: test packet ID ordering and collisions Alireza Asgari via B4 Relay
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Alireza Asgari via B4 Relay @ 2026-11-06 0:37 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal
Cc: Phil Sutter, netfilter-devel, netdev, kunit-dev, Alireza Asgari
From: Alireza Asgari <alireza@asgari.net>
Signed 32-bit ID differences do not order packets separated by half an
ID cycle or more. Queue length does not bound this distance: an old
packet can remain queued while newer packets receive individual verdicts.
For IDs [1, 0x80000002], batching through 1 incorrectly selects both.
For [1, 2, 0x80000003], removing 2 before batching through it also selects
both remaining packets instead of only 1.
Look up a queued boundary by its unique ID and dequeue the FIFO prefix
through that entry. For an absent boundary, use the issuance interval
from the oldest pending packet through the latest assigned ID. Widen the
queue counter to u64 and store its upper 32 bits in a tail id_epoch field;
combine that field with the existing packet ID to recover issuance order.
Wire IDs, hash keys and the proc sequence column remain 32-bit.
An absent ID has a unique offset while the interval spans fewer than
2^32 increments. Outside that interval, retain before/after behavior
only within a strict half-range window. Return ERANGE without changing
the queue for a full-cycle interval or an ambiguous boundary. Empty
prefixes still return ENOENT. Exact queued boundaries work independently
of the interval length.
Assignment and selection stay under the queue lock; reinjection remains
outside it. Failed insertion or notification still consumes an ID.
The internal epoch cannot identify stale userspace verdicts after ID
reuse, and outstanding issuance distances must remain below 2^64.
Codex assisted with development, review and validation of this change.
Fixes: 97d32cf9440d ("netfilter: nfnetlink_queue: batch verdict support")
Signed-off-by: Alireza Asgari <alireza@asgari.net>
---
include/net/netfilter/nf_queue.h | 1 +
net/netfilter/nfnetlink_queue.c | 68 +++++++++++++++++++++++++++++++++++-----
2 files changed, 62 insertions(+), 7 deletions(-)
diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
index 86021c55ba3ce..41aeca590cc3a 100644
--- a/include/net/netfilter/nf_queue.h
+++ b/include/net/netfilter/nf_queue.h
@@ -25,6 +25,7 @@ struct nf_queue_entry {
struct nf_hook_state state;
bool nf_ct_is_unconfirmed;
u16 size; /* sizeof(entry) + saved route keys */
+ u32 id_epoch; /* upper 32 bits of NFQUEUE issuance order */
/* extra space to store route keys */
};
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index c71f4fe890eee..23f9896930bb2 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -89,7 +89,7 @@ struct nfqnl_instance {
*/
spinlock_t lock ____cacheline_aligned_in_smp;
unsigned int queue_total;
- unsigned int id_sequence; /* 'sequence' of pkt ids */
+ u64 id_sequence; /* full sequence of assigned ids */
struct list_head queue_list; /* packets in queue */
};
@@ -994,6 +994,7 @@ __nfqnl_enqueue_packet(struct net *net, struct nfqnl_instance *queue,
int err = -ENOBUFS;
__be32 *packet_id_ptr;
int failopen = 0;
+ u64 sequence;
nskb = nfqnl_build_packet_message(net, queue, entry, &packet_id_ptr);
if (nskb == NULL) {
@@ -1005,7 +1006,9 @@ __nfqnl_enqueue_packet(struct net *net, struct nfqnl_instance *queue,
if (queue->queue_total >= queue->queue_maxlen)
goto err_out_queue_drop;
- entry->id = ++queue->id_sequence;
+ sequence = ++queue->id_sequence;
+ entry->id = (u32)sequence;
+ entry->id_epoch = (u32)(sequence >> 32);
*packet_id_ptr = htonl(entry->id);
/* Insert into hash BEFORE unicast. If failure don't send to userspace. */
@@ -1714,9 +1717,45 @@ verdicthdr_get(const struct nlattr * const nfqa[])
return vhdr;
}
-static int nfq_id_after(unsigned int id, unsigned int max)
+/* Called under queue->lock, for a boundary absent from the packet map. */
+static int nfqnl_batch_range(struct nfqnl_instance *queue, u32 maxid,
+ u32 *firstid, u32 *max_offset)
{
- return (int)(id - max) > 0;
+ struct nf_queue_entry *first;
+ u64 first_sequence, span;
+
+ if (list_empty(&queue->queue_list))
+ return -ENOENT;
+
+ first = list_first_entry(&queue->queue_list, struct nf_queue_entry,
+ list);
+ first_sequence = ((u64)first->id_epoch << 32) | first->id;
+ span = queue->id_sequence - first_sequence;
+ if (span > U32_MAX)
+ return -ERANGE;
+
+ /*
+ * Interpret an absent ID in the current live issuance interval. Its
+ * unsigned offset is unique while that interval is shorter than a
+ * complete ID cycle. Failed notifications also advance the interval.
+ * This cannot distinguish stale requests aliasing a later ID cycle.
+ */
+ *firstid = first->id;
+ *max_offset = maxid - *firstid;
+ if (*max_offset <= span)
+ return 0;
+
+ /*
+ * Outside that interval, preserve ordinary before/after requests only
+ * when the boundary and both endpoints fit a strict half-range window.
+ * Otherwise leave every packet queued rather than guess the prefix.
+ */
+ if (*max_offset < BIT(31))
+ return 0;
+ if ((u32)(queue->id_sequence - maxid) < BIT(31))
+ return -ENOENT;
+
+ return -ERANGE;
}
static int nfqnl_recv_verdict_batch(struct sk_buff *skb,
@@ -1725,11 +1764,13 @@ static int nfqnl_recv_verdict_batch(struct sk_buff *skb,
{
struct nfnl_queue_net *q = nfnl_queue_pernet(info->net);
u16 queue_num = ntohs(info->nfmsg->res_id);
- struct nf_queue_entry *entry, *tmp;
+ struct nf_queue_entry *entry, *tmp, *last;
struct nfqnl_msg_verdict_hdr *vhdr;
struct nfqnl_instance *queue;
unsigned int verdict, maxid;
+ u32 firstid = 0, max_offset = 0;
LIST_HEAD(batch_list);
+ int err = 0;
queue = verdict_instance_lookup(q, queue_num,
NETLINK_CB(skb).portid);
@@ -1745,14 +1786,27 @@ static int nfqnl_recv_verdict_batch(struct sk_buff *skb,
spin_lock_bh(&queue->lock);
+ /* A unique queued boundary identifies the prefix across large gaps. */
+ last = rhashtable_lookup_fast(&queue->nfqnl_packet_map, &maxid,
+ nfqnl_rhashtable_params);
+ if (!last) {
+ err = nfqnl_batch_range(queue, maxid, &firstid, &max_offset);
+ if (err)
+ goto unlock;
+ }
list_for_each_entry_safe(entry, tmp, &queue->queue_list, list) {
- if (nfq_id_after(entry->id, maxid))
+ if (!last && (u32)(entry->id - firstid) > max_offset)
break;
__dequeue_entry(queue, entry);
list_add_tail(&entry->list, &batch_list);
+ if (entry == last)
+ break;
}
+unlock:
spin_unlock_bh(&queue->lock);
+ if (err)
+ return err;
if (list_empty(&batch_list))
return -ENOENT;
@@ -2150,7 +2204,7 @@ static int seq_show(struct seq_file *s, void *v)
inst->peer_portid, inst->queue_total,
inst->copy_mode, inst->copy_range,
inst->queue_dropped, inst->queue_user_dropped,
- inst->id_sequence, 1);
+ (u32)inst->id_sequence, 1);
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC net 3/3] netfilter: nfnetlink_queue: test packet ID ordering and collisions
2026-11-06 0:37 [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps Alireza Asgari via B4 Relay
2026-11-06 0:37 ` [PATCH RFC net 1/3] netfilter: nfnetlink_queue: reject duplicate pending packet IDs Alireza Asgari via B4 Relay
2026-11-06 0:37 ` [PATCH RFC net 2/3] netfilter: nfnetlink_queue: fix batch verdict ordering across large ID gaps Alireza Asgari via B4 Relay
@ 2026-11-06 0:37 ` Alireza Asgari via B4 Relay
2026-11-06 0:41 ` [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps netdev-bot+sinfo
2026-11-06 4:54 ` Florian Westphal
4 siblings, 0 replies; 10+ messages in thread
From: Alireza Asgari via B4 Relay @ 2026-11-06 0:37 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal
Cc: Phil Sutter, netfilter-devel, netdev, kunit-dev, Alireza Asgari
From: Alireza Asgari <alireza@asgari.net>
Add 45 bounded KUnit cases using synthetic queues and the actual batch
handler, enqueue path and hash table. Cover ordinary prefixes, zero IDs,
wraparound, half-range gaps, removed boundaries, ambiguous intervals,
consumed IDs beyond the pending tail and recovery after head removal.
Verify collision rejection, retained identity and later ID reuse for
individual and batch verdicts. A local netlink receiver checks notification
suppression and subsequent delivery; a recording continuation checks
default and FAIL_OPEN disposal. Poison the entry ID fields before actual
enqueue to verify production assignment across high epochs and rollover.
Check release order, hash/list/count consistency, metadata, drop counters
and unchanged queues on rejection. Seed sequence distances directly;
no physical-network traffic or packet-ID control interface is required.
Run each case in a private network namespace so hook teardown cannot
flush unrelated queues. The suite requires built-in NFQUEUE and KUnit.
Codex assisted with development, review and validation of this change.
Signed-off-by: Alireza Asgari <alireza@asgari.net>
---
net/netfilter/Kconfig | 12 +
net/netfilter/nfnetlink_queue.c | 4 +
net/netfilter/nfnetlink_queue_gap_test.c | 906 +++++++++++++++++++++++++++++++
3 files changed, 922 insertions(+)
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 09874c26fd135..59975856628a5 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -59,6 +59,18 @@ config NETFILTER_NETLINK_QUEUE
If this option is enabled, the kernel will include support
for queueing packets via NFNETLINK.
+config NFNETLINK_QUEUE_KUNIT_TEST
+ bool "KUnit tests for NFQUEUE packet IDs"
+ depends on KUNIT=y
+ depends on NET_NS
+ depends on NETFILTER_NETLINK_QUEUE=y
+ help
+ Exercise cumulative verdict selection and packet ID collisions,
+ including wraparound and gaps across the signed half-range.
+ Uses synthetic packets and local netlink messages in private
+ network namespaces.
+ If unsure, say N.
+
config NETFILTER_NETLINK_LOG
tristate "Netfilter LOG over NFNETLINK interface"
default m if NETFILTER_ADVANCED=n
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 23f9896930bb2..70d39f1370b9f 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -2310,3 +2310,7 @@ MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_QUEUE);
module_init(nfnetlink_queue_init);
module_exit(nfnetlink_queue_fini);
+
+#if IS_ENABLED(CONFIG_NFNETLINK_QUEUE_KUNIT_TEST)
+#include "nfnetlink_queue_gap_test.c"
+#endif
diff --git a/net/netfilter/nfnetlink_queue_gap_test.c b/net/netfilter/nfnetlink_queue_gap_test.c
new file mode 100644
index 0000000000000..044c5498fefbb
--- /dev/null
+++ b/net/netfilter/nfnetlink_queue_gap_test.c
@@ -0,0 +1,906 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Included by nfnetlink_queue.c for bounded packet-ID and batch tests. */
+#include <kunit/test.h>
+#include <linux/bits.h>
+#include <linux/err.h>
+#include <linux/net.h>
+#include <linux/sched.h>
+#include <linux/user_namespace.h>
+#include <net/net_namespace.h>
+
+#define NFQNL_TEST_QUEUE 65530
+#define NFQNL_TEST_PORTID 0x1234abcd
+#define NFQNL_TEST_MARK 0x12345678U
+#define NFQNL_TEST_PRIORITY 0x87654321
+#define NFQNL_TEST_MAX_ENTRIES 4
+#define NFQNL_TEST_DROPPED 7
+#define NFQNL_TEST_USER_DROPPED 11
+
+struct nfqnl_test_result {
+ u64 sequence;
+ u32 id;
+ u32 mark;
+ u32 priority;
+};
+
+struct nfqnl_test_ctx {
+ struct net *net;
+ struct nfqnl_instance *queue;
+ struct nfqnl_test_result freed[NFQNL_TEST_MAX_ENTRIES];
+ unsigned int freed_count;
+ struct socket *socket;
+ bool hook_registered;
+ unsigned int accepted_count;
+ u64 accepted_sequence;
+};
+
+struct nfqnl_test_skb_cb {
+ struct nfqnl_test_ctx *ctx;
+ u64 sequence;
+ u32 id;
+};
+
+struct nfqnl_batch_case {
+ const char *name;
+ u32 ids[NFQNL_TEST_MAX_ENTRIES];
+ u32 boundary;
+ unsigned int count;
+ unsigned int selected;
+ unsigned int remove_mask;
+ u64 sequence_offset;
+ u64 issued_advance;
+ int error;
+};
+
+static const struct nfqnl_batch_case nfqnl_batch_cases[] = {
+ {
+ .name = "empty",
+ .boundary = 42,
+ },
+ {
+ .name = "ordinary_prefix",
+ .ids = { 10, 11, 12 },
+ .boundary = 11,
+ .count = 3,
+ .selected = 2,
+ },
+ {
+ .name = "ordinary_wrap",
+ .ids = { 0xfffffffe, 0xffffffff, 0, 1 },
+ .boundary = 0,
+ .count = 4,
+ .selected = 3,
+ },
+ {
+ .name = "zero_boundary",
+ .ids = { 0, 1 },
+ .boundary = 0,
+ .count = 2,
+ .selected = 1,
+ },
+ {
+ .name = "small_gap",
+ .ids = { 10, 15, 18 },
+ .boundary = 15,
+ .count = 3,
+ .selected = 2,
+ },
+ {
+ .name = "removed_boundary",
+ .ids = { 10, 12, 14 },
+ .boundary = 13,
+ .count = 3,
+ .selected = 2,
+ },
+ {
+ .name = "removed_wrap_boundary",
+ .ids = { 0xfffffffe, 0, 2 },
+ .boundary = 1,
+ .count = 3,
+ .selected = 2,
+ },
+ {
+ .name = "before_all",
+ .ids = { 10, 12 },
+ .boundary = 9,
+ .count = 2,
+ },
+ {
+ .name = "after_all",
+ .ids = { 10, 12 },
+ .boundary = 13,
+ .count = 2,
+ .selected = 2,
+ },
+ {
+ .name = "old_head_below_half",
+ .ids = { 1, 0x80000000, 0x80000001 },
+ .boundary = 0x80000000,
+ .count = 3,
+ .selected = 2,
+ },
+ {
+ .name = "old_head_at_half",
+ .ids = { 1, 0x80000001, 0x80000002 },
+ .boundary = 0x80000001,
+ .count = 3,
+ .selected = 2,
+ },
+ {
+ .name = "old_head_over_half",
+ .ids = { 1, 0x80000002, 0x80000003 },
+ .boundary = 0x80000002,
+ .count = 3,
+ .selected = 2,
+ },
+ {
+ .name = "new_tail_below_half",
+ .ids = { 1, 0x80000000 },
+ .boundary = 1,
+ .count = 2,
+ .selected = 1,
+ },
+ {
+ .name = "new_tail_at_half",
+ .ids = { 1, 0x80000001 },
+ .boundary = 1,
+ .count = 2,
+ .selected = 1,
+ },
+ {
+ .name = "new_tail_over_half",
+ .ids = { 1, 0x80000002 },
+ .boundary = 1,
+ .count = 2,
+ .selected = 1,
+ },
+ {
+ .name = "removed_early_large_span",
+ .ids = { 1, 2, 0x80000003 },
+ .boundary = 2,
+ .count = 3,
+ .selected = 1,
+ .remove_mask = BIT(1),
+ },
+ {
+ .name = "removed_late_large_span",
+ .ids = { 1, 0x80000002, 0x80000003 },
+ .boundary = 0x80000002,
+ .count = 3,
+ .selected = 1,
+ .remove_mask = BIT(1),
+ },
+ {
+ .name = "missing_inside_large_span",
+ .ids = { 1, 0x80000003 },
+ .boundary = 0x80000001,
+ .count = 2,
+ .selected = 1,
+ },
+ {
+ .name = "missing_before_large_span",
+ .ids = { 1, 0x80000003 },
+ .boundary = 0,
+ .count = 2,
+ .error = -ERANGE,
+ },
+ {
+ .name = "missing_after_large_span",
+ .ids = { 1, 0x80000003 },
+ .boundary = 0x80000004,
+ .count = 2,
+ .error = -ERANGE,
+ },
+ {
+ .name = "missing_half_from_head",
+ .ids = { 1, 2 },
+ .boundary = 0x80000001,
+ .count = 2,
+ .error = -ERANGE,
+ },
+ {
+ .name = "missing_half_from_issued",
+ .ids = { 1, 2 },
+ .boundary = 0x80000002,
+ .count = 2,
+ .error = -ERANGE,
+ },
+ {
+ .name = "missing_future_below_half",
+ .ids = { 1, 2 },
+ .boundary = 0x80000000,
+ .count = 2,
+ .selected = 2,
+ },
+ {
+ .name = "missing_past_below_half",
+ .ids = { 1, 2 },
+ .boundary = 0x80000003,
+ .count = 2,
+ },
+ {
+ .name = "removed_wrap_large_span",
+ .ids = { 0xfffffffe, 1, 0x80000002 },
+ .boundary = 1,
+ .count = 3,
+ .selected = 1,
+ .remove_mask = BIT(1),
+ },
+ {
+ .name = "missing_full_cycle",
+ .ids = { 1, 0x80000002, 2 },
+ .boundary = 0,
+ .count = 3,
+ .error = -ERANGE,
+ },
+ {
+ .name = "present_full_cycle",
+ .ids = { 1, 0x80000002, 2 },
+ .boundary = 1,
+ .count = 3,
+ .selected = 1,
+ },
+ {
+ .name = "maximum_missing_window",
+ .ids = { 1, 3 },
+ .boundary = 0,
+ .count = 2,
+ .selected = 2,
+ .issued_advance = U32_MAX - 2,
+ },
+ {
+ .name = "assigned_ids_expire_window",
+ .ids = { 1, 2 },
+ .boundary = 3,
+ .count = 2,
+ .issued_advance = U32_MAX,
+ .error = -ERANGE,
+ },
+ {
+ .name = "assigned_ids_extend_window",
+ .ids = { 1, 2 },
+ .boundary = 0x80000001,
+ .count = 2,
+ .selected = 2,
+ .issued_advance = 0x80000000,
+ },
+ {
+ .name = "removal_restores_window",
+ .ids = { 1, 0x80000002, 2 },
+ .boundary = 0,
+ .count = 3,
+ .selected = 1,
+ .remove_mask = BIT(0),
+ },
+ {
+ .name = "counter_wrap_missing",
+ .ids = { 0xfffffffe, 0, 2 },
+ .boundary = 1,
+ .count = 3,
+ .selected = 2,
+ .sequence_offset = U64_MAX - U32_MAX,
+ },
+ {
+ .name = "removal_empties_aged_queue",
+ .ids = { 1, 2 },
+ .boundary = 3,
+ .count = 2,
+ .remove_mask = BIT(0) | BIT(1),
+ .issued_advance = U32_MAX,
+ },
+};
+
+static void nfqnl_batch_case_desc(const struct nfqnl_batch_case *param,
+ char *desc)
+{
+ strscpy(desc, param->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(nfqnl_batch, nfqnl_batch_cases, nfqnl_batch_case_desc);
+
+static void nfqnl_test_skb_destructor(struct sk_buff *skb)
+{
+ struct nfqnl_test_skb_cb *cb = (void *)skb->cb;
+ struct nfqnl_test_ctx *ctx = cb->ctx;
+ unsigned int i = ctx->freed_count++;
+
+ if (i >= ARRAY_SIZE(ctx->freed))
+ return;
+ ctx->freed[i].sequence = cb->sequence;
+ ctx->freed[i].id = cb->id;
+ ctx->freed[i].mark = skb->mark;
+ ctx->freed[i].priority = skb->priority;
+}
+
+static struct nf_queue_entry *nfqnl_test_alloc(struct nfqnl_test_ctx *ctx,
+ u64 sequence)
+{
+ struct nfqnl_test_skb_cb *cb;
+ struct nf_queue_entry *entry;
+
+ entry = kzalloc_obj(*entry);
+ if (!entry)
+ return NULL;
+ entry->skb = alloc_skb(0, GFP_KERNEL);
+ if (!entry->skb) {
+ kfree(entry);
+ return NULL;
+ }
+ entry->id = (u32)sequence;
+ entry->id_epoch = (u32)(sequence >> 32);
+ entry->size = sizeof(*entry);
+ entry->state.net = ctx->net;
+ entry->state.pf = AF_INET;
+ entry->state.hook = NF_INET_PRE_ROUTING;
+ entry->skb->mark = 10;
+ entry->skb->priority = 20;
+ cb = (void *)entry->skb->cb;
+ cb->ctx = ctx;
+ cb->sequence = sequence;
+ cb->id = entry->id;
+ entry->skb->destructor = nfqnl_test_skb_destructor;
+ return entry;
+}
+
+static int nfqnl_test_enqueue(struct nfqnl_test_ctx *ctx, u64 sequence)
+{
+ struct nf_queue_entry *entry = nfqnl_test_alloc(ctx, sequence);
+ int err;
+
+ if (!entry)
+ return -ENOMEM;
+ spin_lock_bh(&ctx->queue->lock);
+ ctx->queue->id_sequence = sequence;
+ err = __enqueue_entry(ctx->queue, entry);
+ spin_unlock_bh(&ctx->queue->lock);
+ if (err) {
+ kfree_skb(entry->skb);
+ nf_queue_entry_free(entry);
+ }
+ return err;
+}
+
+static int nfqnl_test_verdict(struct nfqnl_test_ctx *ctx, u32 boundary)
+{
+ struct nfqnl_msg_verdict_hdr vhdr = {
+ .verdict = htonl(NF_DROP),
+ .id = htonl(boundary),
+ };
+ struct nlattr *attrs[NFQA_MAX + 1];
+ struct nfnl_info info = { .net = ctx->net };
+ struct nlmsghdr *nlh;
+ struct sk_buff *skb;
+ u16 type;
+ int err;
+
+ skb = alloc_skb(256, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+ type = nfnl_msg_type(NFNL_SUBSYS_QUEUE, NFQNL_MSG_VERDICT_BATCH);
+ nlh = nfnl_msg_put(skb, NFQNL_TEST_PORTID, 1, type, NLM_F_REQUEST,
+ AF_INET, NFNETLINK_V0, htons(NFQNL_TEST_QUEUE));
+ if (!nlh || nla_put(skb, NFQA_VERDICT_HDR, sizeof(vhdr), &vhdr) ||
+ nla_put_be32(skb, NFQA_MARK, htonl(NFQNL_TEST_MARK)) ||
+ nla_put_be32(skb, NFQA_PRIORITY, htonl(NFQNL_TEST_PRIORITY))) {
+ err = -EMSGSIZE;
+ goto out;
+ }
+ nlmsg_end(skb, nlh);
+ err = nlmsg_parse_deprecated(nlh, sizeof(struct nfgenmsg), attrs,
+ NFQA_MAX, nfqa_verdict_batch_policy, NULL);
+ if (err)
+ goto out;
+ NETLINK_CB(skb).portid = NFQNL_TEST_PORTID;
+ info.nlh = nlh;
+ info.nfmsg = nlmsg_data(nlh);
+ rcu_read_lock();
+ err = nfqnl_recv_verdict_batch(skb, &info,
+ (const struct nlattr *const *)attrs);
+ rcu_read_unlock();
+out:
+ kfree_skb(skb);
+ return err;
+}
+
+static void nfqnl_test_expect_queue(struct kunit *test,
+ const struct nfqnl_batch_case *param,
+ unsigned int removed_mask, u64 sequence)
+{
+ struct nfqnl_test_result remaining[NFQNL_TEST_MAX_ENTRIES];
+ bool in_hash[NFQNL_TEST_MAX_ENTRIES];
+ struct nfqnl_test_ctx *ctx = test->priv;
+ struct nf_queue_entry *entry;
+ unsigned int i, expected = 0, count = 0, queue_total;
+ unsigned int dropped, user_dropped;
+ u64 actual_sequence;
+
+ rcu_read_lock();
+ spin_lock_bh(&ctx->queue->lock);
+ queue_total = ctx->queue->queue_total;
+ dropped = ctx->queue->queue_dropped;
+ user_dropped = ctx->queue->queue_user_dropped;
+ actual_sequence = ctx->queue->id_sequence;
+ list_for_each_entry(entry, &ctx->queue->queue_list, list) {
+ if (count == ARRAY_SIZE(remaining))
+ break;
+ remaining[count].id = entry->id;
+ remaining[count].mark = entry->skb->mark;
+ remaining[count].priority = entry->skb->priority;
+ count++;
+ }
+ for (i = 0; i < param->count; i++)
+ in_hash[i] =
+ rhashtable_lookup_fast(&ctx->queue->nfqnl_packet_map,
+ ¶m->ids[i],
+ nfqnl_rhashtable_params) != NULL;
+ spin_unlock_bh(&ctx->queue->lock);
+ rcu_read_unlock();
+
+ KUNIT_EXPECT_EQ(test, dropped, NFQNL_TEST_DROPPED);
+ KUNIT_EXPECT_EQ(test, user_dropped, NFQNL_TEST_USER_DROPPED);
+ KUNIT_EXPECT_EQ(test, actual_sequence, sequence);
+ for (i = 0; i < param->count; i++) {
+ bool present = !(removed_mask & BIT(i));
+
+ KUNIT_EXPECT_EQ(test, in_hash[i], present);
+ if (!present)
+ continue;
+ if (expected < count)
+ KUNIT_EXPECT_EQ(test, remaining[expected].id,
+ param->ids[i]);
+ expected++;
+ }
+ KUNIT_EXPECT_EQ(test, queue_total, expected);
+ KUNIT_EXPECT_EQ(test, count, expected);
+ for (i = 0; i < count; i++) {
+ KUNIT_EXPECT_EQ(test, remaining[i].mark, 10);
+ KUNIT_EXPECT_EQ(test, remaining[i].priority, 20);
+ }
+}
+
+static int nfqnl_test_remove(struct nfqnl_test_ctx *ctx, u32 id)
+{
+ struct nf_queue_entry *entry;
+ int err;
+
+ rcu_read_lock();
+ entry = find_dequeue_entry(ctx->queue, id);
+ err = entry ? 0 : -ENOENT;
+ /* find_dequeue_entry() releases the queue lock before reinjection. */
+ if (entry)
+ nfqnl_reinject(entry, NF_DROP);
+ rcu_read_unlock();
+ return err;
+}
+
+static void nfqnl_batch_selection_test(struct kunit *test)
+{
+ const struct nfqnl_batch_case *param = test->param_value;
+ struct nfqnl_test_ctx *ctx = test->priv;
+ unsigned int removed_mask = 0, removed_count = 0, selected = 0;
+ u64 sequence = param->sequence_offset;
+ unsigned int i;
+ int err;
+
+ /* Seed chronological positions without generating billions of packets. */
+ for (i = 0; i < param->count; i++) {
+ sequence += i ? (u32)(param->ids[i] - param->ids[i - 1]) :
+ param->ids[i];
+ err = nfqnl_test_enqueue(ctx, sequence);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ }
+
+ /* Failed notifications can assign IDs without adding queued entries. */
+ sequence += param->issued_advance;
+ spin_lock_bh(&ctx->queue->lock);
+ ctx->queue->id_sequence = sequence;
+ spin_unlock_bh(&ctx->queue->lock);
+ nfqnl_test_expect_queue(test, param, removed_mask, sequence);
+
+ for (i = 0; i < param->count; i++) {
+ if (!(param->remove_mask & BIT(i)))
+ continue;
+ err = nfqnl_test_remove(ctx, param->ids[i]);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ removed_mask |= BIT_U32(i);
+ KUNIT_ASSERT_EQ(test, ctx->freed_count, removed_count + 1);
+ KUNIT_EXPECT_EQ(test, ctx->freed[removed_count].id,
+ param->ids[i]);
+ KUNIT_EXPECT_EQ(test, ctx->freed[removed_count].mark, 10);
+ KUNIT_EXPECT_EQ(test, ctx->freed[removed_count].priority, 20);
+ removed_count++;
+ nfqnl_test_expect_queue(test, param, removed_mask, sequence);
+ }
+
+ err = nfqnl_test_verdict(ctx, param->boundary);
+ KUNIT_EXPECT_EQ(test, err,
+ param->error ?: (param->selected ? 0 : -ENOENT));
+ KUNIT_EXPECT_EQ(test, ctx->freed_count,
+ removed_count + param->selected);
+ for (i = 0; i < param->count && selected < param->selected; i++) {
+ unsigned int freed = removed_count + selected;
+
+ if (removed_mask & BIT(i))
+ continue;
+ removed_mask |= BIT_U32(i);
+ selected++;
+ if (freed >= ctx->freed_count)
+ continue;
+ KUNIT_EXPECT_EQ(test, ctx->freed[freed].id, param->ids[i]);
+ KUNIT_EXPECT_EQ(test, ctx->freed[freed].mark, NFQNL_TEST_MARK);
+ KUNIT_EXPECT_EQ(test, ctx->freed[freed].priority,
+ NFQNL_TEST_PRIORITY);
+ }
+ nfqnl_test_expect_queue(test, param, removed_mask, sequence);
+}
+
+static void nfqnl_test_expect_entries(struct kunit *test, const u64 *sequences,
+ unsigned int expected, u64 issued,
+ unsigned int expected_dropped)
+{
+ struct nfqnl_test_result remaining[NFQNL_TEST_MAX_ENTRIES];
+ bool in_hash[NFQNL_TEST_MAX_ENTRIES];
+ struct nfqnl_test_ctx *ctx = test->priv;
+ struct nf_queue_entry *entry, *found;
+ unsigned int count = 0, i, total, dropped, user_dropped;
+ u64 actual_issued;
+ int mapped;
+
+ rcu_read_lock();
+ spin_lock_bh(&ctx->queue->lock);
+ total = ctx->queue->queue_total;
+ mapped = atomic_read(&ctx->queue->nfqnl_packet_map.nelems);
+ dropped = ctx->queue->queue_dropped;
+ user_dropped = ctx->queue->queue_user_dropped;
+ actual_issued = ctx->queue->id_sequence;
+ list_for_each_entry(entry, &ctx->queue->queue_list, list) {
+ if (count == ARRAY_SIZE(remaining))
+ break;
+ remaining[count].sequence = ((u64)entry->id_epoch << 32) |
+ entry->id;
+ remaining[count].id = entry->id;
+ remaining[count].mark = entry->skb->mark;
+ remaining[count].priority = entry->skb->priority;
+ found = rhashtable_lookup_fast(&ctx->queue->nfqnl_packet_map,
+ &entry->id,
+ nfqnl_rhashtable_params);
+ in_hash[count] = found == entry;
+ count++;
+ }
+ spin_unlock_bh(&ctx->queue->lock);
+ rcu_read_unlock();
+
+ KUNIT_EXPECT_EQ(test, total, expected);
+ KUNIT_EXPECT_EQ(test, mapped, (int)expected);
+ KUNIT_EXPECT_EQ(test, count, expected);
+ KUNIT_EXPECT_EQ(test, actual_issued, issued);
+ KUNIT_EXPECT_EQ(test, dropped, expected_dropped);
+ KUNIT_EXPECT_EQ(test, user_dropped, NFQNL_TEST_USER_DROPPED);
+ for (i = 0; i < min(count, expected); i++) {
+ KUNIT_EXPECT_EQ(test, remaining[i].sequence, sequences[i]);
+ KUNIT_EXPECT_EQ(test, remaining[i].id, (u32)sequences[i]);
+ KUNIT_EXPECT_EQ(test, remaining[i].mark, 10);
+ KUNIT_EXPECT_EQ(test, remaining[i].priority, 20);
+ KUNIT_EXPECT_TRUE(test, in_hash[i]);
+ }
+}
+
+static void nfqnl_test_expect_freed(struct kunit *test, unsigned int index,
+ u64 sequence, bool batch)
+{
+ struct nfqnl_test_ctx *ctx = test->priv;
+
+ KUNIT_ASSERT_LT(test, index, ctx->freed_count);
+ KUNIT_EXPECT_EQ(test, ctx->freed[index].sequence, sequence);
+ KUNIT_EXPECT_EQ(test, ctx->freed[index].id, (u32)sequence);
+ KUNIT_EXPECT_EQ(test, ctx->freed[index].mark,
+ batch ? NFQNL_TEST_MARK : 10);
+ KUNIT_EXPECT_EQ(test, ctx->freed[index].priority,
+ batch ? NFQNL_TEST_PRIORITY : 20);
+}
+
+struct nfqnl_collision_case {
+ const char *name;
+ u32 id;
+ bool batch;
+};
+
+static const struct nfqnl_collision_case nfqnl_collision_cases[] = {
+ { "individual", 1, false },
+ { "batch", 1, true },
+ { "individual_zero", 0, false },
+ { "batch_zero", 0, true },
+};
+
+static void nfqnl_collision_case_desc(const struct nfqnl_collision_case *param,
+ char *desc)
+{
+ strscpy(desc, param->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(nfqnl_collision, nfqnl_collision_cases,
+ nfqnl_collision_case_desc);
+
+static void nfqnl_id_collision_test(struct kunit *test)
+{
+ const struct nfqnl_collision_case *param = test->param_value;
+ struct nfqnl_test_ctx *ctx = test->priv;
+ u64 old = param->id, collision = (1ULL << 32) + old;
+ u64 reused[] = { (2ULL << 32) + old, (2ULL << 32) + old + 1 };
+ int err;
+
+ err = nfqnl_test_enqueue(ctx, old);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ err = nfqnl_test_enqueue(ctx, collision);
+ KUNIT_ASSERT_EQ(test, err, -EEXIST);
+ KUNIT_ASSERT_EQ(test, ctx->freed_count, 1);
+ nfqnl_test_expect_freed(test, 0, collision, false);
+ nfqnl_test_expect_entries(test, &old, 1, collision, NFQNL_TEST_DROPPED);
+
+ /* Both verdict forms still identify the retained original object. */
+ err = param->batch ? nfqnl_test_verdict(ctx, param->id) :
+ nfqnl_test_remove(ctx, param->id);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, ctx->freed_count, 2);
+ nfqnl_test_expect_freed(test, 1, old, param->batch);
+ nfqnl_test_expect_entries(test, NULL, 0, collision, NFQNL_TEST_DROPPED);
+
+ /* Freed IDs can be reused; the following distinct ID is admitted too. */
+ err = nfqnl_test_enqueue(ctx, reused[0]);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ err = nfqnl_test_enqueue(ctx, reused[1]);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ nfqnl_test_expect_entries(test, reused, 2, reused[1],
+ NFQNL_TEST_DROPPED);
+ err = nfqnl_test_verdict(ctx, (u32)reused[1]);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, ctx->freed_count, 4);
+ nfqnl_test_expect_freed(test, 2, reused[0], true);
+ nfqnl_test_expect_freed(test, 3, reused[1], true);
+ nfqnl_test_expect_entries(test, NULL, 0, reused[1], NFQNL_TEST_DROPPED);
+}
+
+static unsigned int nfqnl_test_hook(void *priv, struct sk_buff *skb,
+ const struct nf_hook_state *state)
+{
+ (void)priv;
+ (void)skb;
+ (void)state;
+ return NF_ACCEPT;
+}
+
+static const struct nf_hook_ops nfqnl_test_hook_ops = {
+ .hook = nfqnl_test_hook,
+ .pf = NFPROTO_IPV4,
+ .hooknum = NF_INET_PRE_ROUTING,
+ .priority = NF_IP_PRI_LAST,
+};
+
+static int nfqnl_test_accept(struct net *net, struct sock *sk,
+ struct sk_buff *skb)
+{
+ struct nfqnl_test_skb_cb *cb = (void *)skb->cb;
+
+ (void)net;
+ (void)sk;
+ cb->ctx->accepted_count++;
+ cb->ctx->accepted_sequence = cb->sequence;
+ kfree_skb(skb);
+ return 0;
+}
+
+static int nfqnl_test_packet_enqueue(struct nfqnl_test_ctx *ctx, u64 sequence)
+{
+ struct nf_queue_entry *entry = nfqnl_test_alloc(ctx, sequence);
+ const struct nf_hook_entries *hooks;
+ struct sk_buff *skb;
+ unsigned int i;
+ int err;
+
+ if (!entry)
+ return -ENOMEM;
+ skb = entry->skb;
+ /* Verify production assignment, independently of fixture seeding. */
+ entry->id = ~(u32)sequence;
+ entry->id_epoch = ~(u32)(sequence >> 32);
+ entry->state.okfn = nfqnl_test_accept;
+ rcu_read_lock();
+ hooks = nf_hook_entries_head(ctx->net, NFPROTO_IPV4,
+ NF_INET_PRE_ROUTING);
+ for (i = 0; hooks && i < hooks->num_hook_entries; i++) {
+ if (hooks->hooks[i].hook == nfqnl_test_hook)
+ break;
+ }
+ if (!hooks || i == hooks->num_hook_entries) {
+ err = -ENOENT;
+ } else {
+ entry->hook_index = i;
+ err = __nfqnl_enqueue_packet(ctx->net, ctx->queue, entry);
+ }
+ rcu_read_unlock();
+ /* Mirror the NFQUEUE caller's ownership on an enqueue error. */
+ if (err < 0) {
+ nf_queue_entry_free(entry);
+ kfree_skb(skb);
+ }
+ return err;
+}
+
+static void nfqnl_test_notification(struct kunit *test, bool expected, u32 id)
+{
+ struct nfqnl_test_ctx *ctx = test->priv;
+ struct nlattr *attrs[NFQA_MAX + 1];
+ struct nfqnl_msg_packet_hdr *phdr;
+ struct sk_buff *skb;
+ int err;
+
+ skb = skb_recv_datagram(ctx->socket->sk, MSG_DONTWAIT, &err);
+ if (!expected) {
+ KUNIT_EXPECT_PTR_EQ(test, skb, NULL);
+ KUNIT_EXPECT_EQ(test, err, -EAGAIN);
+ if (skb)
+ skb_free_datagram(ctx->socket->sk, skb);
+ return;
+ }
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ err = nlmsg_parse_deprecated(nlmsg_hdr(skb), sizeof(struct nfgenmsg),
+ attrs, NFQA_MAX, NULL, NULL);
+ KUNIT_EXPECT_EQ(test, err, 0);
+ if (!err) {
+ KUNIT_EXPECT_NOT_NULL(test, attrs[NFQA_PACKET_HDR]);
+ if (attrs[NFQA_PACKET_HDR]) {
+ phdr = nla_data(attrs[NFQA_PACKET_HDR]);
+ KUNIT_EXPECT_EQ(test, ntohl(phdr->packet_id), id);
+ }
+ }
+ skb_free_datagram(ctx->socket->sk, skb);
+}
+
+struct nfqnl_enqueue_case {
+ const char *name;
+ bool failopen;
+ u64 offset;
+};
+
+static const struct nfqnl_enqueue_case nfqnl_enqueue_cases[] = {
+ { "default", false, 0 },
+ { "fail_open", true, 0 },
+ { "high_epoch_default", false, 0x8000000000000000ULL },
+ { "high_epoch_fail_open", true, 0x8000000000000000ULL },
+ { "last_epoch_default", false, 0xfffffffe00000000ULL },
+ { "last_epoch_fail_open", true, 0xfffffffe00000000ULL },
+ { "counter_wrap_default", false, 0xffffffff00000000ULL },
+ { "counter_wrap_fail_open", true, 0xffffffff00000000ULL },
+};
+
+static void nfqnl_enqueue_case_desc(const struct nfqnl_enqueue_case *param,
+ char *desc)
+{
+ strscpy(desc, param->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(nfqnl_enqueue, nfqnl_enqueue_cases, nfqnl_enqueue_case_desc);
+
+static void nfqnl_collision_enqueue_test(struct kunit *test)
+{
+ struct nfqnl_test_ctx *ctx = test->priv;
+ const struct nfqnl_enqueue_case *param = test->param_value;
+ bool failopen = param->failopen;
+ u64 offset = param->offset;
+ struct sockaddr_nl addr = {
+ .nl_family = AF_NETLINK,
+ .nl_pid = NFQNL_TEST_PORTID,
+ };
+ u64 sequences[] = { offset + 1, offset + (1ULL << 32) + 2 };
+ u64 collision = offset + (1ULL << 32) + 1;
+ unsigned int dropped = NFQNL_TEST_DROPPED + !failopen;
+ int err;
+
+ err = sock_create_kern(ctx->net, AF_NETLINK, SOCK_RAW,
+ NETLINK_NETFILTER, &ctx->socket);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ err = kernel_bind(ctx->socket, (struct sockaddr_unsized *)&addr,
+ sizeof(addr));
+ KUNIT_ASSERT_EQ(test, err, 0);
+ err = nf_register_net_hook(ctx->net, &nfqnl_test_hook_ops);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ ctx->hook_registered = true;
+ ctx->queue->copy_mode = NFQNL_COPY_META;
+ ctx->queue->flags = failopen ? NFQA_CFG_F_FAIL_OPEN : 0;
+
+ spin_lock_bh(&ctx->queue->lock);
+ ctx->queue->id_sequence = offset;
+ spin_unlock_bh(&ctx->queue->lock);
+
+ /* Prove the local notification socket works before testing suppression. */
+ err = nfqnl_test_packet_enqueue(ctx, sequences[0]);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ nfqnl_test_notification(test, true, 1);
+ nfqnl_test_notification(test, false, 0);
+ nfqnl_test_expect_entries(test, sequences, 1, sequences[0],
+ NFQNL_TEST_DROPPED);
+
+ spin_lock_bh(&ctx->queue->lock);
+ ctx->queue->id_sequence = offset + (1ULL << 32);
+ spin_unlock_bh(&ctx->queue->lock);
+ err = nfqnl_test_packet_enqueue(ctx, collision);
+ KUNIT_ASSERT_EQ(test, err, failopen ? 0 : -EEXIST);
+ KUNIT_ASSERT_EQ(test, ctx->freed_count, 1);
+ nfqnl_test_expect_freed(test, 0, collision, false);
+ KUNIT_EXPECT_EQ(test, ctx->accepted_count, failopen ? 1U : 0U);
+ if (failopen)
+ KUNIT_EXPECT_EQ(test, ctx->accepted_sequence, collision);
+ nfqnl_test_expect_entries(test, sequences, 1, collision, dropped);
+ nfqnl_test_notification(test, false, 0);
+
+ /* Collision refusal advances issuance; the next ID can be notified. */
+ err = nfqnl_test_packet_enqueue(ctx, sequences[1]);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ nfqnl_test_notification(test, true, 2);
+ nfqnl_test_notification(test, false, 0);
+ nfqnl_test_expect_entries(test, sequences, 2, sequences[1], dropped);
+ err = nfqnl_test_verdict(ctx, 2);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, ctx->freed_count, 3);
+ nfqnl_test_expect_freed(test, 1, sequences[0], true);
+ nfqnl_test_expect_freed(test, 2, sequences[1], true);
+ nfqnl_test_expect_entries(test, NULL, 0, sequences[1], dropped);
+ KUNIT_EXPECT_EQ(test, ctx->accepted_count, failopen ? 1U : 0U);
+}
+
+static int nfqnl_batch_test_init(struct kunit *test)
+{
+ struct nfqnl_test_ctx *ctx;
+ int err;
+
+ ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+ /* Hook teardown must not flush queues outside this test. */
+ ctx->net = copy_net_ns(CLONE_NEWNET, &init_user_ns, &init_net);
+ if (IS_ERR(ctx->net))
+ return PTR_ERR(ctx->net);
+ ctx->queue = instance_create(nfnl_queue_pernet(ctx->net),
+ NFQNL_TEST_QUEUE, NFQNL_TEST_PORTID);
+ err = PTR_ERR_OR_ZERO(ctx->queue);
+ if (err) {
+ put_net(ctx->net);
+ return err;
+ }
+ ctx->queue->queue_dropped = NFQNL_TEST_DROPPED;
+ ctx->queue->queue_user_dropped = NFQNL_TEST_USER_DROPPED;
+ test->priv = ctx;
+ return 0;
+}
+
+static void nfqnl_batch_test_exit(struct kunit *test)
+{
+ struct nfqnl_test_ctx *ctx = test->priv;
+
+ if (ctx->hook_registered)
+ nf_unregister_net_hook(ctx->net, &nfqnl_test_hook_ops);
+ instance_destroy(nfnl_queue_pernet(ctx->net), ctx->queue);
+ /* Drain queued-packet destructors before releasing the test context. */
+ rcu_barrier();
+ flush_workqueue(nfq_cleanup_wq);
+ if (ctx->socket)
+ sock_release(ctx->socket);
+ put_net(ctx->net);
+}
+
+static struct kunit_case nfqnl_batch_test_cases[] = {
+ KUNIT_CASE_PARAM(nfqnl_batch_selection_test, nfqnl_batch_gen_params),
+ KUNIT_CASE_PARAM(nfqnl_id_collision_test, nfqnl_collision_gen_params),
+ KUNIT_CASE_PARAM(nfqnl_collision_enqueue_test,
+ nfqnl_enqueue_gen_params),
+ {}
+};
+
+static struct kunit_suite nfqnl_batch_test_suite = {
+ .name = "nfnetlink_queue_batch",
+ .init = nfqnl_batch_test_init,
+ .exit = nfqnl_batch_test_exit,
+ .test_cases = nfqnl_batch_test_cases,
+};
+
+kunit_test_suite(nfqnl_batch_test_suite);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
2026-11-06 0:37 [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps Alireza Asgari via B4 Relay
` (2 preceding siblings ...)
2026-11-06 0:37 ` [PATCH RFC net 3/3] netfilter: nfnetlink_queue: test packet ID ordering and collisions Alireza Asgari via B4 Relay
@ 2026-11-06 0:41 ` netdev-bot+sinfo
2026-11-06 4:54 ` Florian Westphal
4 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sinfo @ 2026-11-06 0:41 UTC (permalink / raw)
To: alireza
Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, netfilter-devel,
netdev, kunit-dev
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- How the issue was discovered, e.g. hit in production, hit during
development, syzbot report, manual code inspection, LLM or static
analysis tool scan.
Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net 2/3] netfilter: nfnetlink_queue: fix batch verdict ordering across large ID gaps
2026-11-06 0:37 ` [PATCH RFC net 2/3] netfilter: nfnetlink_queue: fix batch verdict ordering across large ID gaps Alireza Asgari via B4 Relay
@ 2026-11-06 4:45 ` Florian Westphal
0 siblings, 0 replies; 10+ messages in thread
From: Florian Westphal @ 2026-11-06 4:45 UTC (permalink / raw)
To: alireza; +Cc: Pablo Neira Ayuso, Phil Sutter, netfilter-devel, netdev,
kunit-dev
Alireza Asgari via B4 Relay <devnull+alireza.asgari.net@kernel.org> wrote:
> From: Alireza Asgari <alireza@asgari.net>
>
> Signed 32-bit ID differences do not order packets separated by half an
> ID cycle or more. Queue length does not bound this distance: an old
> packet can remain queued while newer packets receive individual verdicts.
> For IDs [1, 0x80000002], batching through 1 incorrectly selects both.
> For [1, 2, 0x80000003], removing 2 before batching through it also selects
> both remaining packets instead of only 1.
BATCH verdicts require strict in-sequence ordering.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
2026-11-06 0:37 [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps Alireza Asgari via B4 Relay
` (3 preceding siblings ...)
2026-11-06 0:41 ` [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps netdev-bot+sinfo
@ 2026-11-06 4:54 ` Florian Westphal
2026-11-06 9:46 ` Alireza Asgari
4 siblings, 1 reply; 10+ messages in thread
From: Florian Westphal @ 2026-11-06 4:54 UTC (permalink / raw)
To: alireza; +Cc: Pablo Neira Ayuso, Phil Sutter, netfilter-devel, netdev,
kunit-dev
Alireza Asgari via B4 Relay <devnull+alireza.asgari.net@kernel.org> wrote:
> This series addresses packet-ID reuse and batch-verdict ordering in
> NFQUEUE. I am posting it as an RFC to discuss the handling of ambiguous
> batch boundaries and the additional sequence state used to resolve them.
-v. I see 0 bugs here. Only lots of code churn.
> An old packet can remain queued while newer packets receive individual
> verdicts.
Not really, that would be a userspace bug. Its not adviseable to do
out-of-order verdicts. I know some people do that though :-/
> Queue length therefore does not bound the distance between
> pending packet IDs. After a complete 32-bit ID cycle, two pending packets
> can have the same ID, making verdict lookup ambiguous. Even before reuse,
> the signed 32-bit comparison in the batch handler can select the wrong
> FIFO prefix when IDs are separated by half an ID cycle or more.
WTF, whats the point of doing that?
> Patch 1 rejects an incoming duplicate ID before list insertion or userspace
> notification, preserving the existing pending packet. The incoming packet
> follows the existing enqueue-failure policy: drop by default, or accept
> with FAIL_OPEN.
OK, but really... who needs this?
> Patch 2 uses an exact queued boundary to select the FIFO prefix. For a
> boundary no longer queued, it uses a 64-bit queue sequence and a per-entry
> 32-bit epoch to recover issuance order. Ambiguous missing boundaries
> return -ERANGE without changing the queue. The wire format is unchanged,
> but the rejection behavior is userspace-visible. This does not distinguish
> stale verdicts after an ID has been released and reused.
Insane... tons of code added for what? Batch verdicts are incompatible
with out-of-order verdicts. I fail to understand why the kernel needs
to become more complex and more complex and more complex.
LLMs make it oh-so-easy but whats left is even more unmaintanable than
the current state.
> I would appreciate feedback on:
>
> - Whether -ERANGE with the queue unchanged is the appropriate response
> for an ambiguous missing batch boundary.
> - Whether the per-entry epoch and widened queue counter are appropriate
> for handling boundaries that have already been removed.
Sorry, I have no idea what these patches should accomplish.
> - Whether the existing drop/FAIL_OPEN policy is the preferred way to
> handle an incoming packet whose ID is still pending.
That one is fine, even though I would really like to understand what use
case requires holding on to a packet for 2**32 (!) Ids to pass.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
2026-11-06 4:54 ` Florian Westphal
@ 2026-11-06 9:46 ` Alireza Asgari
2026-11-06 14:55 ` Florian Westphal
0 siblings, 1 reply; 10+ messages in thread
From: Alireza Asgari @ 2026-11-06 9:46 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Phil Sutter, netfilter-devel, netdev,
kunit-dev
Thanks for the review.
I understand that batch verdicts are intended to be used in strict sequence. The scenarios motivating this RFC did not establish a failure under that usage model, and the evidence presented does not justify the additional epoch and missing-boundary handling.
I’ll drop the current series. I also understand that the proposed collision disposition being reasonable does not by itself establish a need for the duplicate-ID guard. I would revisit that separately only if I can provide a concrete use case.
Thanks for taking the time to review it.
From: Florian Westphal <fw@strlen.de>
To: <alireza@asgari.net>
Cc: "Pablo Neira Ayuso"<pablo@netfilter.org>, "Phil Sutter"<phil@nwl.cc>, <netfilter-devel@vger.kernel.org>, <netdev@vger.kernel.org>, <kunit-dev@googlegroups.com>
Date: Fri, 06 Nov 2026 05:54:33 +0100
Subject: Re: [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
> Alireza Asgari via B4 Relay <devnull+alireza.asgari.net@kernel.org> wrote:
> > This series addresses packet-ID reuse and batch-verdict ordering in
> > NFQUEUE. I am posting it as an RFC to discuss the handling of ambiguous
> > batch boundaries and the additional sequence state used to resolve them.
>
> -v. I see 0 bugs here. Only lots of code churn.
>
> > An old packet can remain queued while newer packets receive individual
> > verdicts.
>
> Not really, that would be a userspace bug. Its not adviseable to do
> out-of-order verdicts. I know some people do that though :-/
>
> > Queue length therefore does not bound the distance between
> > pending packet IDs. After a complete 32-bit ID cycle, two pending packets
> > can have the same ID, making verdict lookup ambiguous. Even before reuse,
> > the signed 32-bit comparison in the batch handler can select the wrong
> > FIFO prefix when IDs are separated by half an ID cycle or more.
>
> WTF, whats the point of doing that?
>
> > Patch 1 rejects an incoming duplicate ID before list insertion or userspace
> > notification, preserving the existing pending packet. The incoming packet
> > follows the existing enqueue-failure policy: drop by default, or accept
> > with FAIL_OPEN.
>
> OK, but really... who needs this?
>
> > Patch 2 uses an exact queued boundary to select the FIFO prefix. For a
> > boundary no longer queued, it uses a 64-bit queue sequence and a per-entry
> > 32-bit epoch to recover issuance order. Ambiguous missing boundaries
> > return -ERANGE without changing the queue. The wire format is unchanged,
> > but the rejection behavior is userspace-visible. This does not distinguish
> > stale verdicts after an ID has been released and reused.
>
> Insane... tons of code added for what? Batch verdicts are incompatible
> with out-of-order verdicts. I fail to understand why the kernel needs
> to become more complex and more complex and more complex.
>
> LLMs make it oh-so-easy but whats left is even more unmaintanable than
> the current state.
>
> > I would appreciate feedback on:
> >
> > - Whether -ERANGE with the queue unchanged is the appropriate response
> > for an ambiguous missing batch boundary.
> > - Whether the per-entry epoch and widened queue counter are appropriate
> > for handling boundaries that have already been removed.
>
> Sorry, I have no idea what these patches should accomplish.
>
> > - Whether the existing drop/FAIL_OPEN policy is the preferred way to
> > handle an incoming packet whose ID is still pending.
>
> That one is fine, even though I would really like to understand what use
> case requires holding on to a packet for 2**32 (!) Ids to pass.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
2026-11-06 9:46 ` Alireza Asgari
@ 2026-11-06 14:55 ` Florian Westphal
2026-11-06 15:42 ` Alireza Asgari
0 siblings, 1 reply; 10+ messages in thread
From: Florian Westphal @ 2026-11-06 14:55 UTC (permalink / raw)
To: Alireza Asgari
Cc: Pablo Neira Ayuso, Phil Sutter, netfilter-devel, netdev,
kunit-dev
Alireza Asgari <alireza@asgari.net> wrote:
> Thanks for the review.
>
> I understand that batch verdicts are intended to be used in strict sequence. The scenarios motivating this RFC did not establish a failure under that usage model, and the evidence presented does not justify the additional epoch and missing-boundary handling.
>
> I’ll drop the current series. I also understand that the proposed collision disposition being reasonable does not by itself establish a need for the duplicate-ID guard. I would revisit that separately only if I can provide a concrete use case.
>
> Thanks for taking the time to review it.
If you want to work on something else that is actually a problem, then
you could investigate the mem exhaustion issue.
The current limitation is queue length based, this is very old from
a time when GSO/TSO aggregate skbs did not exist.
Untested idea:
1. compute a memory size limit based off the old NFQA_CFG_QUEUE_MAXLEN:
sizeof(sk_buff shell structure) + 2048 (half a 4k page).
2. store skb->truesize in the entry struct when queueing.
3. charge the truesize to the queue entry (and uncharge on reinject).
4. in addition to the queue length check, also check if the new
skb->truesize would bring us over the limit, refuse queueing if thats
the case.
5. Make sure NFQA_CFG_QUEUE_MAXLEN updates continue to work.
This leaves on major issue: userspace can request 64k queues, so these
limits don't help.
One possible solution:
1. For init netns, no changes.
2. For non-init netns:
at queue creation time resp. when limit is altered, pre-charge
the worst case occupancy to kmemcg.
Refuse queue creation if that fails.
Ideally this could be split into several patches to ease review.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
2026-11-06 14:55 ` Florian Westphal
@ 2026-11-06 15:42 ` Alireza Asgari
0 siblings, 0 replies; 10+ messages in thread
From: Alireza Asgari @ 2026-11-06 15:42 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Phil Sutter, netfilter-devel, netdev,
kunit-dev
Hi Florian,
Thanks for the pointers. I’d like to give this a try. I’ll take some time to understand the code first and follow up with what I find.
From: Florian Westphal <fw@strlen.de>
To: "Alireza Asgari"<alireza@asgari.net>
Cc: "Pablo Neira Ayuso"<pablo@netfilter.org>, "Phil Sutter"<phil@nwl.cc>, "netfilter-devel"<netfilter-devel@vger.kernel.org>, "netdev"<netdev@vger.kernel.org>, "kunit-dev"<kunit-dev@googlegroups.com>
Date: Fri, 06 Nov 2026 15:55:52 +0100
Subject: Re: [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps
> Alireza Asgari <alireza@asgari.net> wrote:
> > Thanks for the review.
> >
> > I understand that batch verdicts are intended to be used in strict sequence. The scenarios motivating this RFC did not establish a failure under that usage model, and the evidence presented does not justify the additional epoch and missing-boundary handling.
> >
> > I’ll drop the current series. I also understand that the proposed collision disposition being reasonable does not by itself establish a need for the duplicate-ID guard. I would revisit that separately only if I can provide a concrete use case.
> >
> > Thanks for taking the time to review it.
>
> If you want to work on something else that is actually a problem, then
> you could investigate the mem exhaustion issue.
>
> The current limitation is queue length based, this is very old from
> a time when GSO/TSO aggregate skbs did not exist.
>
> Untested idea:
>
> 1. compute a memory size limit based off the old NFQA_CFG_QUEUE_MAXLEN:
> sizeof(sk_buff shell structure) + 2048 (half a 4k page).
> 2. store skb->truesize in the entry struct when queueing.
> 3. charge the truesize to the queue entry (and uncharge on reinject).
> 4. in addition to the queue length check, also check if the new
> skb->truesize would bring us over the limit, refuse queueing if thats
> the case.
> 5. Make sure NFQA_CFG_QUEUE_MAXLEN updates continue to work.
>
> This leaves on major issue: userspace can request 64k queues, so these
> limits don't help.
>
> One possible solution:
> 1. For init netns, no changes.
> 2. For non-init netns:
> at queue creation time resp. when limit is altered, pre-charge
> the worst case occupancy to kmemcg.
>
> Refuse queue creation if that fails.
> Ideally this could be split into several patches to ease review.
>
> Thanks.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-06 15:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06 0:37 [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps Alireza Asgari via B4 Relay
2026-11-06 0:37 ` [PATCH RFC net 1/3] netfilter: nfnetlink_queue: reject duplicate pending packet IDs Alireza Asgari via B4 Relay
2026-11-06 0:37 ` [PATCH RFC net 2/3] netfilter: nfnetlink_queue: fix batch verdict ordering across large ID gaps Alireza Asgari via B4 Relay
2026-11-06 4:45 ` Florian Westphal
2026-11-06 0:37 ` [PATCH RFC net 3/3] netfilter: nfnetlink_queue: test packet ID ordering and collisions Alireza Asgari via B4 Relay
2026-11-06 0:41 ` [PATCH RFC net 0/3] netfilter: nfnetlink_queue: handle packet ID reuse and large gaps netdev-bot+sinfo
2026-11-06 4:54 ` Florian Westphal
2026-11-06 9:46 ` Alireza Asgari
2026-11-06 14:55 ` Florian Westphal
2026-11-06 15:42 ` Alireza Asgari
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox