[nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

These chains are indirectly attached to the hook since they are
not called for packets belonging to an established connection.

Introduce NF_HOOK_OP_NAT to identify the container and dump attached
entries instead of the container itself.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/linux/netfilter.h      |  7 +++++++
 net/netfilter/nf_nat_core.c    |  6 ------
 net/netfilter/nf_nat_proto.c   |  8 ++++++++
 net/netfilter/nfnetlink_hook.c | 31 +++++++++++++++++++++++++++++--
 4 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index efbbfa770d66..e99afc1414cd 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -93,6 +93,7 @@ enum nf_hook_ops_type {
 	NF_HOOK_OP_NF_TABLES,
 	NF_HOOK_OP_BPF,
 	NF_HOOK_OP_NFT_FT,
+	NF_HOOK_OP_NAT,
 };
 
 struct nf_hook_ops {
@@ -140,6 +141,12 @@ struct nf_hook_entries {
 	 */
 };
 
+struct nf_nat_lookup_hook_priv {
+	struct nf_hook_entries __rcu *entries;
+
+	struct rcu_head rcu_head;
+};
+
 #ifdef CONFIG_NETFILTER
 static inline struct nf_hook_ops **nf_hook_entries_get_hook_ops(const struct nf_hook_entries *e)
 {
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 3b5434e4ec9c..1f9576056e2d 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -39,12 +39,6 @@ static struct hlist_head *nf_nat_bysource __read_mostly;
 static unsigned int nf_nat_htable_size __read_mostly;
 static siphash_aligned_key_t nf_nat_hash_rnd;
 
-struct nf_nat_lookup_hook_priv {
-	struct nf_hook_entries __rcu *entries;
-
-	struct rcu_head rcu_head;
-};
-
 struct nf_nat_hooks_net {
 	struct nf_hook_ops *nat_hook_ops;
 	unsigned int users;
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index 97c0f841fc96..f29e896e62c9 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -790,6 +790,7 @@ static const struct nf_hook_ops nf_nat_ipv4_ops[] = {
 		.pf		= NFPROTO_IPV4,
 		.hooknum	= NF_INET_PRE_ROUTING,
 		.priority	= NF_IP_PRI_NAT_DST,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 	/* After packet filtering, change source */
 	{
@@ -797,6 +798,7 @@ static const struct nf_hook_ops nf_nat_ipv4_ops[] = {
 		.pf		= NFPROTO_IPV4,
 		.hooknum	= NF_INET_POST_ROUTING,
 		.priority	= NF_IP_PRI_NAT_SRC,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 	/* Before packet filtering, change destination */
 	{
@@ -804,6 +806,7 @@ static const struct nf_hook_ops nf_nat_ipv4_ops[] = {
 		.pf		= NFPROTO_IPV4,
 		.hooknum	= NF_INET_LOCAL_OUT,
 		.priority	= NF_IP_PRI_NAT_DST,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 	/* After packet filtering, change source */
 	{
@@ -811,6 +814,7 @@ static const struct nf_hook_ops nf_nat_ipv4_ops[] = {
 		.pf		= NFPROTO_IPV4,
 		.hooknum	= NF_INET_LOCAL_IN,
 		.priority	= NF_IP_PRI_NAT_SRC,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 };
 
@@ -1051,6 +1055,7 @@ static const struct nf_hook_ops nf_nat_ipv6_ops[] = {
 		.pf		= NFPROTO_IPV6,
 		.hooknum	= NF_INET_PRE_ROUTING,
 		.priority	= NF_IP6_PRI_NAT_DST,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 	/* After packet filtering, change source */
 	{
@@ -1058,6 +1063,7 @@ static const struct nf_hook_ops nf_nat_ipv6_ops[] = {
 		.pf		= NFPROTO_IPV6,
 		.hooknum	= NF_INET_POST_ROUTING,
 		.priority	= NF_IP6_PRI_NAT_SRC,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 	/* Before packet filtering, change destination */
 	{
@@ -1065,6 +1071,7 @@ static const struct nf_hook_ops nf_nat_ipv6_ops[] = {
 		.pf		= NFPROTO_IPV6,
 		.hooknum	= NF_INET_LOCAL_OUT,
 		.priority	= NF_IP6_PRI_NAT_DST,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 	/* After packet filtering, change source */
 	{
@@ -1072,6 +1079,7 @@ static const struct nf_hook_ops nf_nat_ipv6_ops[] = {
 		.pf		= NFPROTO_IPV6,
 		.hooknum	= NF_INET_LOCAL_IN,
 		.priority	= NF_IP6_PRI_NAT_SRC,
+		.hook_ops_type	= NF_HOOK_OP_NAT,
 	},
 };
 
diff --git a/net/netfilter/nfnetlink_hook.c b/net/netfilter/nfnetlink_hook.c
index 531706982859..924316e673cb 100644
--- a/net/netfilter/nfnetlink_hook.c
+++ b/net/netfilter/nfnetlink_hook.c
@@ -337,6 +337,29 @@ nfnl_hook_entries_head(u8 pf, unsigned int hook, struct net *net, const char *de
 	return hook_head;
 }
 
+static int nfnl_hook_dump_nat(struct sk_buff *nlskb,
+			      const struct nfnl_dump_hook_data *ctx,
+			      const struct nf_hook_ops *ops,
+			      int family, unsigned int seq)
+{
+	struct nf_nat_lookup_hook_priv *priv = ops->priv;
+	struct nf_hook_entries *e = rcu_dereference(priv->entries);
+	struct nf_hook_ops **nat_ops;
+	int i, err;
+
+	if (!e)
+		return 0;
+
+	nat_ops = nf_hook_entries_get_hook_ops(e);
+
+	for (i = 0; i < e->num_hook_entries; i++) {
+		err = nfnl_hook_dump_one(nlskb, ctx, nat_ops[i], family, seq);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
 static int nfnl_hook_dump(struct sk_buff *nlskb,
 			  struct netlink_callback *cb)
 {
@@ -365,8 +388,12 @@ static int nfnl_hook_dump(struct sk_buff *nlskb,
 	ops = nf_hook_entries_get_hook_ops(e);
 
 	for (; i < e->num_hook_entries; i++) {
-		err = nfnl_hook_dump_one(nlskb, ctx, ops[i], family,
-					 cb->nlh->nlmsg_seq);
+		if (ops[i]->hook_ops_type == NF_HOOK_OP_NAT)
+			err = nfnl_hook_dump_nat(nlskb, ctx, ops[i], family,
+						 cb->nlh->nlmsg_seq);
+		else
+			err = nfnl_hook_dump_one(nlskb, ctx, ops[i], family,
+						 cb->nlh->nlmsg_seq);
 		if (err)
 			break;
 	}
-- 
2.51.0

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> These chains are indirectly attached to the hook since they are
> not called for packets belonging to an established connection.
> 
> Introduce NF_HOOK_OP_NAT to identify the container and dump attached
> entries instead of the container itself.

Please lets not do this.

Before:

        hook postrouting {
                +0000000100 nf_nat_ipv6_out [nf_nat]
                +2147483647 nf_confirm [nf_conntrack]
        }

After:
        hook postrouting {
                +0000200000 chain inet nat postrouting [nft_chain_nat]
                +2147483647 nf_confirm [nf_conntrack]
        }

... and thats not true.  The nat chain isn't hooked at 200000.

It hooks at +100, this is a dispatcher.
Concealing the actual hook location and then unrolling the embedded
nat hooks gives a wrong impression and will mislead users wrt. the
actual ordering.

If we really want the ability to list the nat hooks, I think this
needs a new command to dump them.

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

On Thu, Mar 19, 2026 at 05:04:18PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > These chains are indirectly attached to the hook since they are
> > not called for packets belonging to an established connection.
> > 
> > Introduce NF_HOOK_OP_NAT to identify the container and dump attached
> > entries instead of the container itself.
> 
> Please lets not do this.
> 
> Before:
> 
>         hook postrouting {
>                 +0000000100 nf_nat_ipv6_out [nf_nat]
>                 +2147483647 nf_confirm [nf_conntrack]
>         }
> 
> After:
>         hook postrouting {
>                 +0000200000 chain inet nat postrouting [nft_chain_nat]
>                 +2147483647 nf_confirm [nf_conntrack]
>         }
> 
> ... and thats not true.  The nat chain isn't hooked at 200000.
> 
> It hooks at +100, this is a dispatcher.
> Concealing the actual hook location and then unrolling the embedded
> nat hooks gives a wrong impression and will mislead users wrt. the
> actual ordering.

Ah, so the nat-type chain's priority value orders it inside the
dispatcher's list.

Maybe I should print them below the dispatcher hook with extra
indentation? Maybe extra braces could further clarify, e.g.:

| hook postrouting {
|         +0000000100 nf_nat_ipv6_out [nf_nat] {
|                 +0000200000 chain inet nat postrouting [nft_chain_nat]
|         }
|         +2147483647 nf_confirm [nf_conntrack]
| }

> If we really want the ability to list the nat hooks, I think this
> needs a new command to dump them.

We may change 'nft list hooks' output arbitrarily, right? Or should we
fear braking some third-party parsers when doing too fancy stuff?

Thanks, Phil

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> Ah, so the nat-type chain's priority value orders it inside the
> dispatcher's list.

Yes.

> Maybe I should print them below the dispatcher hook with extra
> indentation? Maybe extra braces could further clarify, e.g.:
> 
> | hook postrouting {
> |         +0000000100 nf_nat_ipv6_out [nf_nat] {
> |                 +0000200000 chain inet nat postrouting [nft_chain_nat]
> |         }
> |         +2147483647 nf_confirm [nf_conntrack]
> | }

Actually  one could override the hook value with the one of the
nat base hook.  The ordering inside the dispatcher is whats important,
the exact numerical value isn't important.

> > If we really want the ability to list the nat hooks, I think this
> > needs a new command to dump them.
> 
> We may change 'nft list hooks' output arbitrarily, right? Or should we
> fear braking some third-party parsers when doing too fancy stuff?

Ugh. No mercy for screenscrapers.  But maybe my suggestion above is
enough.

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

On Thu, Mar 19, 2026 at 06:06:10PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > Ah, so the nat-type chain's priority value orders it inside the
> > dispatcher's list.
> 
> Yes.
> 
> > Maybe I should print them below the dispatcher hook with extra
> > indentation? Maybe extra braces could further clarify, e.g.:
> > 
> > | hook postrouting {
> > |         +0000000100 nf_nat_ipv6_out [nf_nat] {
> > |                 +0000200000 chain inet nat postrouting [nft_chain_nat]
> > |         }
> > |         +2147483647 nf_confirm [nf_conntrack]
> > | }
> 
> Actually  one could override the hook value with the one of the
> nat base hook.  The ordering inside the dispatcher is whats important,
> the exact numerical value isn't important.

Hmm. I like how one can use 'list hooks' output to find a good spot for
a new base chain. The real nat chain priority value is needed for that,
but no point in considering made up use-cases. Seeing the chains
attached to a given nat dispatcher is already a step forward, and having
their ordering is probably well enough.

> > > If we really want the ability to list the nat hooks, I think this
> > > needs a new command to dump them.
> > 
> > We may change 'nft list hooks' output arbitrarily, right? Or should we
> > fear braking some third-party parsers when doing too fancy stuff?
> 
> Ugh. No mercy for screenscrapers.  But maybe my suggestion above is
> enough.

ACK, I'll prepare a v2.

Thanks, Phil

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Pablo Neira Ayuso]

On Thu, Mar 19, 2026 at 10:08:30PM +0100, Phil Sutter wrote:
> On Thu, Mar 19, 2026 at 06:06:10PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > Ah, so the nat-type chain's priority value orders it inside the
> > > dispatcher's list.
> > 
> > Yes.
> > 
> > > Maybe I should print them below the dispatcher hook with extra
> > > indentation? Maybe extra braces could further clarify, e.g.:
> > > 
> > > | hook postrouting {
> > > |         +0000000100 nf_nat_ipv6_out [nf_nat] {
> > > |                 +0000200000 chain inet nat postrouting [nft_chain_nat]
> > > |         }
> > > |         +2147483647 nf_confirm [nf_conntrack]
> > > | }
> > 
> > Actually  one could override the hook value with the one of the
> > nat base hook.  The ordering inside the dispatcher is whats important,
> > the exact numerical value isn't important.
> 
> Hmm. I like how one can use 'list hooks' output to find a good spot for
> a new base chain. The real nat chain priority value is needed for that,
> but no point in considering made up use-cases. Seeing the chains
> attached to a given nat dispatcher is already a step forward, and having
> their ordering is probably well enough.

I guess the goal is to expose iptables and nftables in place?

Is it really needed to expose this internal +0000200000?

Maybe simply report instead?

         +0000000100 nf_nat_ipv6_out [nf_nat]: chain ip nat POSTROUTING [iptable_nat]
         +0000000100 nf_nat_ipv6_out [nf_nat]: chain inet nat postrouting [nft_chain_nat]

Yes, it looks like a duplicate, but it is sort of how it works now, no
need to expose dispatcher details.

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

Hi Pablo,

On Fri, Mar 20, 2026 at 01:24:40AM +0100, Pablo Neira Ayuso wrote:
> On Thu, Mar 19, 2026 at 10:08:30PM +0100, Phil Sutter wrote:
> > On Thu, Mar 19, 2026 at 06:06:10PM +0100, Florian Westphal wrote:
> > > Phil Sutter <phil@nwl.cc> wrote:
> > > > Ah, so the nat-type chain's priority value orders it inside the
> > > > dispatcher's list.
> > > 
> > > Yes.
> > > 
> > > > Maybe I should print them below the dispatcher hook with extra
> > > > indentation? Maybe extra braces could further clarify, e.g.:
> > > > 
> > > > | hook postrouting {
> > > > |         +0000000100 nf_nat_ipv6_out [nf_nat] {
> > > > |                 +0000200000 chain inet nat postrouting [nft_chain_nat]
> > > > |         }
> > > > |         +2147483647 nf_confirm [nf_conntrack]
> > > > | }
> > > 
> > > Actually  one could override the hook value with the one of the
> > > nat base hook.  The ordering inside the dispatcher is whats important,
> > > the exact numerical value isn't important.
> > 
> > Hmm. I like how one can use 'list hooks' output to find a good spot for
> > a new base chain. The real nat chain priority value is needed for that,
> > but no point in considering made up use-cases. Seeing the chains
> > attached to a given nat dispatcher is already a step forward, and having
> > their ordering is probably well enough.
> 
> I guess the goal is to expose iptables and nftables in place?

No, I merely want to expose nat-type nftables chains. With iptables, I
see that we only get ipt_do_table instead of a chain name, but "fixing"
that is probably not worth the effort.

> Is it really needed to expose this internal +0000200000?
> 
> Maybe simply report instead?
> 
>          +0000000100 nf_nat_ipv6_out [nf_nat]: chain ip nat POSTROUTING [iptable_nat]
>          +0000000100 nf_nat_ipv6_out [nf_nat]: chain inet nat postrouting [nft_chain_nat]
> 
> Yes, it looks like a duplicate, but it is sort of how it works now, no
> need to expose dispatcher details.

A remark from a practical perspective: Florian's suggestion to dump the
nat-type chains in their order with the dispatcher's priority value is
super-easy to implement (just have to pass the priority value to
nfnl_hook_dump_one() via parameter) and does not require adjustments in
user space.

Given the benefit for current and older user space, I'd go with that
first and decide if we add extra netlink attributes to nat-type chain
messages for user space to print more details.

Cheers, Phil

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

On Fri, Mar 20, 2026 at 11:17:00AM +0100, Phil Sutter wrote:
[...]
> A remark from a practical perspective: Florian's suggestion to dump the
> nat-type chains in their order with the dispatcher's priority value is
> super-easy to implement (just have to pass the priority value to
> nfnl_hook_dump_one() via parameter) and does not require adjustments in
> user space.

Famous last words. :(

In fact, user space calls basehook_list_add_tail() for each received
hook message which (contrary to its name) inserts sorted by ascending
priority value. It does this by skipping as long as the cursor's
priority is lower than the new element's, i.e. hooks with same priority
value are inserted in reverse order than received. This is a no-go since
ordering matters, right?

Cheers, Phil

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> On Fri, Mar 20, 2026 at 11:17:00AM +0100, Phil Sutter wrote:
> [...]
> > A remark from a practical perspective: Florian's suggestion to dump the
> > nat-type chains in their order with the dispatcher's priority value is
> > super-easy to implement (just have to pass the priority value to
> > nfnl_hook_dump_one() via parameter) and does not require adjustments in
> > user space.
> 
> Famous last words. :(

diff --git a/src/mnl.c b/src/mnl.c
index 4893af8322ae..b9efd3cfd3ce 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -2520,7 +2520,7 @@ static void basehook_list_add_tail(struct basehook *b, struct list_head *head)
                        continue;
                if (!basehook_eq(hook, b))
                        continue;
-               if (hook->prio < b->prio)
+               if (hook->prio <= b->prio)
                        continue;
 
                list_add(&b->list, &hook->list);

?

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

On Fri, Mar 20, 2026 at 12:18:31PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > On Fri, Mar 20, 2026 at 11:17:00AM +0100, Phil Sutter wrote:
> > [...]
> > > A remark from a practical perspective: Florian's suggestion to dump the
> > > nat-type chains in their order with the dispatcher's priority value is
> > > super-easy to implement (just have to pass the priority value to
> > > nfnl_hook_dump_one() via parameter) and does not require adjustments in
> > > user space.
> > 
> > Famous last words. :(
> 
> diff --git a/src/mnl.c b/src/mnl.c
> index 4893af8322ae..b9efd3cfd3ce 100644
> --- a/src/mnl.c
> +++ b/src/mnl.c
> @@ -2520,7 +2520,7 @@ static void basehook_list_add_tail(struct basehook *b, struct list_head *head)
>                         continue;
>                 if (!basehook_eq(hook, b))
>                         continue;
> -               if (hook->prio < b->prio)
> +               if (hook->prio <= b->prio)
>                         continue;
>  
>                 list_add(&b->list, &hook->list);
> 
> ?

Sure, but <=nftables-1.1.6 will still get it wrong. Can we tolerate
that?

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

On Fri, Mar 20, 2026 at 12:26:24PM +0100, Phil Sutter wrote:
> On Fri, Mar 20, 2026 at 12:18:31PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > On Fri, Mar 20, 2026 at 11:17:00AM +0100, Phil Sutter wrote:
> > > [...]
> > > > A remark from a practical perspective: Florian's suggestion to dump the
> > > > nat-type chains in their order with the dispatcher's priority value is
> > > > super-easy to implement (just have to pass the priority value to
> > > > nfnl_hook_dump_one() via parameter) and does not require adjustments in
> > > > user space.
> > > 
> > > Famous last words. :(
> > 
> > diff --git a/src/mnl.c b/src/mnl.c
> > index 4893af8322ae..b9efd3cfd3ce 100644
> > --- a/src/mnl.c
> > +++ b/src/mnl.c
> > @@ -2520,7 +2520,7 @@ static void basehook_list_add_tail(struct basehook *b, struct list_head *head)
> >                         continue;
> >                 if (!basehook_eq(hook, b))
> >                         continue;
> > -               if (hook->prio < b->prio)
> > +               if (hook->prio <= b->prio)
> >                         continue;
> >  
> >                 list_add(&b->list, &hook->list);
> > 
> > ?
> 
> Sure, but <=nftables-1.1.6 will still get it wrong. Can we tolerate
> that?

The kernel could dump them in reverse. :D

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> > Sure, but <=nftables-1.1.6 will still get it wrong. Can we tolerate
> > that?
> 
> The kernel could dump them in reverse. :D

WTF, no no no.

The kernel is fine, this is a userspace bug.

It wasn't noticed because typical configurations don't add same-prio
hooks.

Userspace MUST dump in the order received so the dumped list reflects
the actual execution order.

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Pablo Neira Ayuso]

On Fri, Mar 20, 2026 at 12:47:23PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > > Sure, but <=nftables-1.1.6 will still get it wrong. Can we tolerate
> > > that?
> > 
> > The kernel could dump them in reverse. :D
> 
> WTF, no no no.
> 
> The kernel is fine, this is a userspace bug.

I think so too.

One of the main use-cases for 'list hooks' is to display the order in
which hooks are run.

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Phil Sutter]

On Fri, Mar 20, 2026 at 01:09:16PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Mar 20, 2026 at 12:47:23PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > > Sure, but <=nftables-1.1.6 will still get it wrong. Can we tolerate
> > > > that?
> > > 
> > > The kernel could dump them in reverse. :D
> > 
> > WTF, no no no.
> > 
> > The kernel is fine, this is a userspace bug.
> 
> I think so too.
> 
> One of the main use-cases for 'list hooks' is to display the order in
> which hooks are run.

Guys, you're obviously right! Took me a while to realize this behaviour
is not isolated to newly dumped nat-type chains with same prio, but
other chains may have same prio values as well. So: Fix for user space,
continue as planned in kernel space.

Thanks!

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> >                 list_add(&b->list, &hook->list);
> > 
> > ?
> 
> Sure, but <=nftables-1.1.6 will still get it wrong. Can we tolerate
> that?

Well, its a bug.

Re: [nf-next PATCH] netfilter: nfnetlink_hook: Dump nat type chains

[Pablo Neira Ayuso]

On Fri, Mar 20, 2026 at 12:26:24PM +0100, Phil Sutter wrote:
> On Fri, Mar 20, 2026 at 12:18:31PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > On Fri, Mar 20, 2026 at 11:17:00AM +0100, Phil Sutter wrote:
> > > [...]
> > > > A remark from a practical perspective: Florian's suggestion to dump the
> > > > nat-type chains in their order with the dispatcher's priority value is
> > > > super-easy to implement (just have to pass the priority value to
> > > > nfnl_hook_dump_one() via parameter) and does not require adjustments in
> > > > user space.
> > > 
> > > Famous last words. :(
> > 
> > diff --git a/src/mnl.c b/src/mnl.c
> > index 4893af8322ae..b9efd3cfd3ce 100644
> > --- a/src/mnl.c
> > +++ b/src/mnl.c
> > @@ -2520,7 +2520,7 @@ static void basehook_list_add_tail(struct basehook *b, struct list_head *head)
> >                         continue;
> >                 if (!basehook_eq(hook, b))
> >                         continue;
> > -               if (hook->prio < b->prio)
> > +               if (hook->prio <= b->prio)
> >                         continue;
> >  
> >                 list_add(&b->list, &hook->list);
> > 
> > ?
> 
> Sure, but <=nftables-1.1.6 will still get it wrong. Can we tolerate
> that?

I think this qualifies as a fix.