All of lore.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: 26+ 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
2026-03-26 13:16 ` [PATCH net,v3 00/12] Netfilter for net Florian Westphal
2026-03-26 14:44   ` Pablo Neira Ayuso
2026-03-26 14:46     ` Florian Westphal
2026-03-26 15:00       ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.