[PATCH nf-next 0/2] netfilter: nf_tables: include conntrack state in trace messages

[PATCH nf-next 0/2] netfilter: nf_tables: include conntrack state in trace messages

[Florian Westphal]

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
 [..]

First patch is a needed prerequisite to avoid a module dependency.
Second patch adds the needed info.

Patches for libnftnl and nftables will follow shortly.

[PATCH nf-next 1/2] netfilter: conntrack: make nf_conntrack_id callable without a module dependency

[Florian Westphal]

While nf_conntrack_id() doesn't need any functionaliy from conntrack, it
does reside in nf_conntrack_core.c -- callers add a module
dependency on conntrack.

Followup patch will need to compute the conntrack id from nf_tables_trace.c
to include it in nf_trace messages emitted to userspace via netlink.

I don't want to introduce a module dependency between nf_tables and
conntrack for this.

Since trace is slowpath, the added indirection is ok.

One alternative is to move nf_conntrack_id to the netfilter/core.c,
but I don't see a compelling reason so far.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/netfilter.h         | 1 +
 net/netfilter/nf_conntrack_core.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 2b8aac2c70ad..10d95556befe 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -470,6 +470,7 @@ struct nf_ct_hook {
 	void (*attach)(struct sk_buff *nskb, const struct sk_buff *skb);
 	void (*set_closing)(struct nf_conntrack *nfct);
 	int (*confirm)(struct sk_buff *skb);
+	u32 (*get_id)(const struct nf_conntrack *nfct);
 };
 extern const struct nf_ct_hook __rcu *nf_ct_hook;
 
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index de8d50af9b5b..201d3c4ec623 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -505,6 +505,11 @@ u32 nf_ct_get_id(const struct nf_conn *ct)
 }
 EXPORT_SYMBOL_GPL(nf_ct_get_id);
 
+static u32 nf_conntrack_get_id(const struct nf_conntrack *nfct)
+{
+	return nf_ct_get_id(nf_ct_to_nf_conn(nfct));
+}
+
 static void
 clean_from_lists(struct nf_conn *ct)
 {
@@ -2710,6 +2715,7 @@ static const struct nf_ct_hook nf_conntrack_hook = {
 	.attach		= nf_conntrack_attach,
 	.set_closing	= nf_conntrack_set_closing,
 	.confirm	= __nf_conntrack_confirm,
+	.get_id		= nf_conntrack_get_id,
 };
 
 void nf_conntrack_init_end(void)
-- 
2.49.0

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

[Florian Westphal]

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/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 7d6bc19a0153..19cddbd1a315 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1841,6 +1841,7 @@ enum nft_xfrm_keys {
  * @NFTA_TRACE_MARK: nfmark (NLA_U32)
  * @NFTA_TRACE_NFPROTO: nf protocol processed (NLA_U32)
  * @NFTA_TRACE_POLICY: policy that decided fate of packet (NLA_U32)
+ * @NFTA_TRACE_CT: connection tracking information (NLA_NESTED: nft_ct_keys)
  */
 enum nft_trace_attributes {
 	NFTA_TRACE_UNSPEC,
@@ -1861,6 +1862,7 @@ enum nft_trace_attributes {
 	NFTA_TRACE_NFPROTO,
 	NFTA_TRACE_POLICY,
 	NFTA_TRACE_PAD,
+	NFTA_TRACE_CT,
 	__NFTA_TRACE_MAX
 };
 #define NFTA_TRACE_MAX (__NFTA_TRACE_MAX - 1)
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
@@ -15,6 +15,7 @@
 #include <linux/netfilter.h>
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nf_tables.h>
+#include <net/netfilter/nf_conntrack.h>
 #include <net/netfilter/nf_tables_core.h>
 #include <net/netfilter/nf_tables.h>
 
@@ -90,6 +91,59 @@ static int nf_trace_fill_dev_info(struct sk_buff *nlskb,
 	return 0;
 }
 
+static int nf_trace_fill_ct_info(struct sk_buff *nlskb,
+				 const struct sk_buff *skb)
+{
+	const struct nf_ct_hook *ct_hook;
+	enum ip_conntrack_info ctinfo;
+	const struct nf_conn *ct;
+	struct nlattr *nest;
+	u32 state;
+
+	ct_hook = rcu_dereference(nf_ct_hook);
+	if (!ct_hook)
+		return 0;
+
+	ct = nf_ct_get(skb, &ctinfo);
+	if (!ct) {
+		if (ctinfo != IP_CT_UNTRACKED) /* not seen by conntrack or invalid */
+			return 0;
+
+		state = NF_CT_STATE_UNTRACKED_BIT;
+	} else {
+		state = NF_CT_STATE_BIT(ctinfo);
+	}
+
+	nest = nla_nest_start(nlskb, NFTA_TRACE_CT);
+	if (!nest)
+		return -1;
+
+	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;
+	}
+
+	nla_nest_end(nlskb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(nlskb, nest);
+	return -1;
+}
+
 static int nf_trace_fill_pkt_info(struct sk_buff *nlskb,
 				  const struct nft_pktinfo *pkt)
 {
@@ -210,7 +264,12 @@ void nft_trace_notify(const struct nft_pktinfo *pkt,
 		nla_total_size(sizeof(__be32)) +	/* trace type */
 		nla_total_size(0) +			/* VERDICT, nested */
 			nla_total_size(sizeof(u32)) +	/* verdict code */
-		nla_total_size(sizeof(u32)) +		/* id */
+		nla_total_size(0) +			/* nft_ct_keys, nested */
+			nla_total_size(sizeof(u8)) +	/* direction */
+			nla_total_size(sizeof(u32)) +	/* state */
+			nla_total_size(sizeof(u32)) +	/* status */
+			nla_total_size(sizeof(u32)) +	/* id */
+		nla_total_size(sizeof(u32)) +		/* trace id */
 		nla_total_size(NFT_TRACETYPE_LL_HSIZE) +
 		nla_total_size(NFT_TRACETYPE_NETWORK_HSIZE) +
 		nla_total_size(NFT_TRACETYPE_TRANSPORT_HSIZE) +
@@ -291,6 +350,10 @@ void nft_trace_notify(const struct nft_pktinfo *pkt,
 
 		if (nf_trace_fill_pkt_info(skb, pkt))
 			goto nla_put_failure;
+
+		if (nf_trace_fill_ct_info(skb, pkt->skb))
+			goto nla_put_failure;
+
 		info->packet_dumped = true;
 	}
 
-- 
2.49.0

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

[Pablo Neira Ayuso]

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.

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

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > +		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.

In that case I would prefer not to use NESTED attribute for this, i.e.:

 * @NFTA_TRACE_CT_ID: connection tracking information (NLA_U32)
 * @NFTA_TRACE_CT_STATUS: connection tracking information (NLA_U32)
 * @NFTA_TRACE_CT_STATE: connection tracking information (NLA_U32)

... and so on.  I see no potential for attribute re-use.

The only argument for NESTED is that userspace can check for presence
of NFTA_TRACE_CT/NESTED instead of checking each ct trace attr in
sequence.

Whats you preference?

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

[Pablo Neira Ayuso]

On Wed, May 21, 2025 at 01:26:00PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > +		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.
> 
> In that case I would prefer not to use NESTED attribute for this, i.e.:
> 
>  * @NFTA_TRACE_CT_ID: connection tracking information (NLA_U32)
>  * @NFTA_TRACE_CT_STATUS: connection tracking information (NLA_U32)
>  * @NFTA_TRACE_CT_STATE: connection tracking information (NLA_U32)
> 
> ... and so on.  I see no potential for attribute re-use.
> 
> The only argument for NESTED is that userspace can check for presence
> of NFTA_TRACE_CT/NESTED instead of checking each ct trace attr in
> sequence.
> 
> Whats you preference?

Flat representation (no nesting) is fine with me in this case.

Thanks.