Re: [PATCH nf-next v2] netfilter: nft_ct: support expectation creation for natted flows

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

Hi Florian,

On Tue, Jun 30, 2026 at 08:03:08AM +0200, Florian Westphal wrote:
> This feature only works for connections originating from the host
> and only if there no source address rewrite.
> 
> Add the needed nat glue to have the expectation follow the original
> nat binding.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  v2:
>  add missing nf_ct_expect_iterate_destroy() in exit handler
>  fix buld with CONFIG_NF_NAT=n
> 
>  net/netfilter/nft_ct.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
> index 03a88c77e0f0..95fc3d7c1edb 100644
> --- a/net/netfilter/nft_ct.c
> +++ b/net/netfilter/nft_ct.c
> @@ -1297,6 +1297,17 @@ static int nft_ct_expect_obj_dump(struct sk_buff *skb,
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_NF_NAT)
> +static void nft_ct_nat_follow_master(struct nf_conn *ct, struct nf_conntrack_expect *this)
> +{
> +	const struct nf_ct_helper_expectfn *expfn;
> +
> +	expfn = nf_ct_helper_expectfn_find_by_name("nat-follow-master");
> +	if (expfn)
> +		expfn->expectfn(ct, this);
> +}
> +#endif
> +
>  static void nft_ct_expect_obj_eval(struct nft_object *obj,
>  				   struct nft_regs *regs,
>  				   const struct nft_pktinfo *pkt)
> @@ -1342,6 +1353,13 @@ static void nft_ct_expect_obj_eval(struct nft_object *obj,
>  		          priv->l4proto, NULL, &priv->dport);
>  	exp->timeout += priv->timeout;
>  
> +#if IS_ENABLED(CONFIG_NF_NAT)
> +	if (ct->status & IPS_NAT_MASK) {
> +		exp->saved_proto.tcp.port = priv->dport;
> +		exp->dir = !dir;
> +		exp->expectfn = nft_ct_nat_follow_master;
> +	}
> +#endif
>  	if (nf_ct_expect_related(exp, 0) != 0)
>  		regs->verdict.code = NF_DROP;
>  
> @@ -1416,6 +1434,13 @@ static int __init nft_ct_module_init(void)
>  	return err;
>  }
>  
> +#if IS_ENABLED(CONFIG_NF_NAT)
> +static bool __exit expect_iter_nat(struct nf_conntrack_expect *exp, void *data)
> +{
> +	return exp->expectfn == nft_ct_nat_follow_master;
> +}
> +#endif
> +
>  static void __exit nft_ct_module_exit(void)
>  {
>  #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> @@ -1425,6 +1450,11 @@ static void __exit nft_ct_module_exit(void)
>  	nft_unregister_obj(&nft_ct_helper_obj_type);
>  	nft_unregister_expr(&nft_notrack_type);
>  	nft_unregister_expr(&nft_ct_type);
> +
> +#if IS_ENABLED(CONFIG_NF_NAT)
> +	nf_ct_expect_iterate_destroy(expect_iter_nat, NULL);
> +	synchronize_rcu();
> +#endif

Not sure sashiko is signalling a real issue here.

static void __exit nf_nat_cleanup(void) 
{
        struct nf_nat_proto_clean clean = {}; 
            
        nf_ct_iterate_destroy(nf_nat_proto_clean, &clean);

all NATted master conntracks are destroyed here, including their
expectations, which might have the expectations using
nat_follow_master.

nf_ct_iterate_cleanup()
  nf_ct_delete()
    clean_from_lists()
      nf_ct_remove_expectation()

And nf_nat_cleanup() can only be called if all there is no more nat
chains in place, correct?

If so, no new expectations can be created using nat_follow_master and
this nf_ct_iterate_destroy() is implicitly removing the remaining
expectations using nat_follow_master.

Sorry for the long writing, I don't see it where the issue is.

>  }
>  
>  module_init(nft_ct_module_init);
> -- 
> 2.53.0
> 
>