Re: [PATCH nft 0/2] src: add conntrack information to trace monitor mode

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jul 07, 2025 at 11:47:12AM +0200, Florian Westphal wrote:
> > First patch is a preparation patch that moves the trace code
> > from netlink.c to the new trace.c file.
> > 
> > Second patch adds the ct info to the trace output.
> > 
> > This patch exposes the 'clash' bit to userspace.
> > (Technically its the kernel counterpart).
> > 
> > If you dislike this, I can send a kernel patch that removes
> > the bit before dumping ct status bits to userspace, let me
> > know.
> 
> If this is intentional, then
> 
> +             SYMBOL("clash",         IPS_UNTRACKED_BIT),
> 
> hiding clash bit is probably a good idea.

Currently the existence of 'clash' entries are a kernel-internal
implemenation detail.

Neither /proc or ctnetlink exposes them, the dump handlers only
emit ORIGINAL direction, but the clash entries are only inserted
into the hashes for the reply tuple.

Hence, they are not visible so far.
With this change however, a packet that matches a clash entry (reply
dir), will have skb->_nfct set to a 'clash' entry and so its ct->status
and ID are exposed to userspace.

This isn't a problem, but it does mean that the IPS_UNTRACKED_BIT is
set in ct->status.

IPS_UNTRACKED isn't used anymore in the kernel, it has been re-purposed
to flag the clash entries (IPS_NAT_CLASH_BIT = IPS_UNTRACKED_BIT, but
the former constant isn't exposed via UAPI).

Thats the reason for this awkward

  SYMBOL("clash",         IPS_UNTRACKED_BIT),

> Just hide it from userspace nftables in this series, later I'd suggest
> you proceed with the kernel update.

If I remove this line from the patch, then I can skip/ignore the value
in userspace, e.g.:

diff --git a/src/trace.c b/src/trace.c
index b270951025b8..b3b2c8fdf1b9 100644
--- a/src/trace.c
+++ b/src/trace.c
@@ -264,7 +264,7 @@ static struct expr *trace_alloc_list(const struct datatype *dtype,
        for (i = 0; i < 32; i++) {
                uint32_t bitv = v & (1 << i);

-               if (bitv == 0)
+               if (bitv == 0 || i == IPS_UNTRACKED_BIT)
                        continue;

and remove the IPS_UNTRACKED_BIT from the symbol table.

Then followup with a kernel patch that removes IPS_UNTRACKED_BIT before
dumping ct->status.

Does that sound ok?
If so, I'll apply the first patch in this series before resending 2/2.

Thanks!