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

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

Hi Florian,

On Thu, May 08, 2025 at 05:33:56PM +0200, Florian Westphal wrote:
> Upcoming kernel change provides the packets conntrack state in the
> trace message data.
> 
> This allows to see if packet is seen as original or reply, the conntrack
> state (new, establieshed, related) and the status bits which show if e.g.
> NAT was applied.  Alsoi include conntrack ID so users can use conntrack
> tool to query the kernel for more information via ctnetlink.
> 
> This improves debugging when e.g. packets do not pick up the expected
> NAT mapping, which could e.g. also happen because of expectations
> following the NAT binding of the owning conntrack entry.

This feature will be present in the next kernel.

> Example output ("conntrack: " lines are new):
> 
> trace id 32 t PRE_RAW packet: iif "enp0s3" ether saddr [..]
> trace id 32 t PRE_RAW rule tcp flags syn meta nftrace set 1 (verdict continue)
> trace id 32 t PRE_RAW policy accept
> trace id 32 t PRE_MANGLE conntrack: ct direction original ct state new ct id 2641368242
> trace id 32 t PRE_MANGLE packet: iif "enp0s3" ether saddr [..]
> trace id 32 t ct_new_pre rule jump rpfilter (verdict jump rpfilter)
> trace id 32 t PRE_MANGLE policy accept
> trace id 32 t INPUT conntrack: ct direction original ct state new ct status dnat-done ct id 2641368242
> trace id 32 t INPUT packet: iif "enp0s3" [..]
> trace id 32 t public_in rule tcp dport 443 accept (verdict accept)
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

Comment below.

> ---
>  src/ct.c      |   4 ++
>  src/netlink.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 114 insertions(+)
> 
> diff --git a/src/ct.c b/src/ct.c
> index 71ebb9483893..fc43bf63f02c 100644
> --- a/src/ct.c
> +++ b/src/ct.c
> @@ -98,7 +98,11 @@ static const struct symbol_table ct_status_tbl = {
>  		SYMBOL("confirmed",	IPS_CONFIRMED),
>  		SYMBOL("snat",		IPS_SRC_NAT),
>  		SYMBOL("dnat",		IPS_DST_NAT),
> +		SYMBOL("seq-adjust",	IPS_SEQ_ADJUST),
> +		SYMBOL("snat-done",	IPS_SRC_NAT_DONE),
> +		SYMBOL("dnat-done",	IPS_DST_NAT_DONE),

These will be now exposed through 'ct status' forever, not sure we
have a usecase to allow users to match on this.

I know these are exposed through uapi, but I don't have a usecase for
them to allow users to match on them.

Maybe it is _not_ worth, those flags have been there since the
beginning.

If we go for exposing these flags through ct status, do you think it
is possible to provide terse description of these "new flags" in the
manpage?

>  		SYMBOL("dying",		IPS_DYING),
> +		SYMBOL("fixed-timeout",	IPS_FIXED_TIMEOUT),
>  		SYMBOL_LIST_END
>  	},
>  };
> diff --git a/src/netlink.c b/src/netlink.c
> index 86ca32144f02..b1d1dc7f4bd1 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -2116,6 +2116,114 @@ next:
>  	}
>  }
>  
> +static struct expr *trace_alloc_list(const struct datatype *dtype,
> +				     enum byteorder byteorder,
> +				     unsigned int len, const void *data)

Suggestion, not deal breaker: It would be good to start a new
src/trace.c file to add this and move tracing infrastructure?