Re: [PATCH nf-next 2/2] netfilter: nf_tables: add packets conntrack state to debug trace info

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Florian,

On Thu, May 08, 2025 at 05:08:52PM +0200, Florian Westphal wrote:
> Add the minimal relevant info needed for userspace ("nftables monitor
> trace") to provide the conntrack view of the packet:
> 
> - state (new, related, established)
> - direction (original, reply)
> - status (e.g., if connection is subject to dnat)
> - id (allows to query ctnetlink for remaining conntrack state info)
> 
> Example:
> trace id a62 inet filter PRE_RAW packet: iif "enp0s3" ether [..]
>   [..]
> trace id a62 inet filter PRE_MANGLE conntrack: ct direction original ct state new ct id 32
> trace id a62 inet filter PRE_MANGLE packet: [..]
>  [..]
> trace id a62 inet filter IN conntrack: ct direction original ct state new ct status dnat-done ct id 32
>  [..]
> 
> In this case one can see that while NAT is active, the new connection
> isn't subject to a translation.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  include/uapi/linux/netfilter/nf_tables.h |  2 +
>  net/netfilter/nf_tables_trace.c          | 65 +++++++++++++++++++++++-
>  2 files changed, 66 insertions(+), 1 deletion(-)
[...]
> diff --git a/net/netfilter/nf_tables_trace.c b/net/netfilter/nf_tables_trace.c
> index 580c55268f65..ba8b0a8c00e6 100644
> --- a/net/netfilter/nf_tables_trace.c
> +++ b/net/netfilter/nf_tables_trace.c
[...]
> +	if (nla_put_be32(nlskb, NFT_CT_STATE, htonl(state)))
> +		goto nla_put_failure;
> +
> +	if (ct) {
> +		u32 id = ct_hook->get_id(&ct->ct_general);
> +		u32 status = READ_ONCE(ct->status);
> +		u8 dir = CTINFO2DIR(ctinfo);
> +
> +		if (nla_put_u8(nlskb, NFT_CT_DIRECTION, dir))
> +			goto nla_put_failure;
> +
> +		if (nla_put_be32(nlskb, NFT_CT_ID, (__force __be32)id))
> +			goto nla_put_failure;
> +
> +		if (status && nla_put_be32(nlskb, NFT_CT_STATUS, htonl(status)))
> +			goto nla_put_failure;

NFT_CT_* is enum nft_ct_keys which is not intended to be used as
netlink attribute.

NFT_CT_STATE is 0 which is usually reserved for _UNSPEC in netlink
attribute definitions.

My suggestion is that you define new attributes for this, it is
boilerplate code to be added to uapi.

Thanks.