public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Ilya Maximets <i.maximets@ovn.org>
Cc: netfilter-devel@vger.kernel.org, fw@strlen.de,
	davem@davemloft.net, netdev@vger.kernel.org, kuba@kernel.org,
	pabeni@redhat.com, edumazet@google.com, horms@kernel.org,
	Eelco Chaudron <echaudro@redhat.com>,
	Aaron Conole <aconole@redhat.com>
Subject: Re: [PATCH net 06/12] netfilter: nf_conntrack_expect: honor expectation helper field
Date: Fri, 1 May 2026 12:37:16 +0200	[thread overview]
Message-ID: <afSCXEg-X-ieL9cY@chamomile> (raw)
In-Reply-To: <8fd5d3a3-d1d7-4542-a0db-1678989940d4@ovn.org>

Hi Ilya,

On Thu, Apr 30, 2026 at 10:58:38PM +0200, Ilya Maximets wrote:
> On 3/26/26 1:51 PM, Pablo Neira Ayuso wrote:
> > The expectation helper field is mostly unused. As a result, the
> > netfilter codebase relies on accessing the helper through exp->master.
> > 
> > Always set on the expectation helper field so it can be used to reach
> > the helper.
> > 
> > nf_ct_expect_init() is called from packet path where the skb owns
> > the ct object, therefore accessing exp->master for the newly created
> > expectation is safe. This saves a lot of updates in all callsites
> > to pass the ct object as parameter to nf_ct_expect_init().
> > 
> > This is a preparation patches for follow up fixes.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> 
> Hi, Pablo and Florian.
> 
> I was investigating FTP test failures in OVS with 7.0 kernel and bisected
> the issue down to this commit.  AFAIU, with this change all the related
> connections over time gain their parents' helpers,.  This is causing a change
> visible to the userspace, because FTP data connections are now reported to
> have helpers in the conntrack dump:
> 
> # conntrack -L
> tcp      6 119 TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=59534 dport=21 \
>                          src=10.1.1.2 dst=10.1.1.1 sport=21    dport=59534 \
>            [ASSURED] mark=0 helper=ftp use=2
> tcp      6 119 TIME_WAIT src=10.1.1.2 dst=10.1.1.1 sport=52709 dport=52381 \
>                          src=10.1.1.1 dst=10.1.1.2 sport=52381 dport=52709 \
>            [ASSURED] mark=0 helper=ftp use=1
> 
> Before this commit only the control connection had helper=ftp reported in
> the dump.  The traffic seems to work fine, but our tests fail because we
> do not expect the helper attached.
> 
> AFAIU, it's generally not something that should be happening, as helpers
> on data connections do not really make much sense.  But I'm just trying to
> figure out if you would consider this as a regression and fix in the kernel
> or if we should adjust our userspace components for this new dump content,
> which would not be very straightforward to do if we want to be able to run
> tests on both old and the new versions.
> 
> What do you think?

It seems previous behaviour to 9c42bc9db90a was inconsistent, ie. only
the h323 helper sets on exp->helper, then it shows helper= in expected
connections via ctnetlink. I guess this is for debugging given that
h323 is actually a family of helpers.

To consistently skip dumping this for expected connections, probably
this is the way to do:

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conn
index eda5fe4a75c8..9491ae9e080e 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -226,7 +226,7 @@ static int ctnetlink_dump_helpinfo(struct sk_buff *sk
        const struct nf_conn_help *help = nfct_help(ct);
        struct nf_conntrack_helper *helper;
 
-       if (!help)
+       if (!help || ct->status & IPS_EXPECTED)
                return 0;
 
        rcu_read_lock();

  reply	other threads:[~2026-05-01 10:37 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-26 12:51 [PATCH net,v3 00/12] Netfilter for net Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 01/12] netfilter: nft_set_pipapo_avx2: don't return non-matching entry on expiry Pablo Neira Ayuso
2026-03-26 15:30   ` patchwork-bot+netdevbpf
2026-03-26 12:51 ` [PATCH net 02/12] selftests: netfilter: nft_concat_range.sh: add check for flush+reload bug Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 03/12] netfilter: nfnetlink_log: fix uninitialized padding leak in NFULA_PAYLOAD Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 04/12] netfilter: ip6t_rt: reject oversized addrnr in rt_mt6_check() Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 05/12] netfilter: nft_set_rbtree: revisit array resize logic Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 06/12] netfilter: nf_conntrack_expect: honor expectation helper field Pablo Neira Ayuso
2026-04-30 20:58   ` Ilya Maximets
2026-05-01 10:37     ` Pablo Neira Ayuso [this message]
2026-05-04 12:19       ` Ilya Maximets
2026-05-04 23:16         ` Pablo Neira Ayuso
2026-05-04 23:40           ` Pablo Neira Ayuso
2026-05-05 11:01             ` Ilya Maximets
2026-05-05 11:26               ` Pablo Neira Ayuso
2026-05-05 11:01           ` Ilya Maximets
2026-03-26 12:51 ` [PATCH net 07/12] netfilter: nf_conntrack_expect: use expect->helper Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 08/12] netfilter: ctnetlink: ensure safe access to master conntrack Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 09/12] netfilter: nf_conntrack_expect: store netns and zone in expectation Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 10/12] netfilter: nf_conntrack_expect: skip expectations in other netns via proc Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 11/12] netfilter: nf_conntrack_sip: fix use of uninitialized rtp_addr in process_sdp Pablo Neira Ayuso
2026-03-26 12:51 ` [PATCH net 12/12] netfilter: ctnetlink: use netlink policy range checks Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afSCXEg-X-ieL9cY@chamomile \
    --to=pablo@netfilter.org \
    --cc=aconole@redhat.com \
    --cc=davem@davemloft.net \
    --cc=echaudro@redhat.com \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=horms@kernel.org \
    --cc=i.maximets@ovn.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox