Re: [PATCH net 06/12] netfilter: nf_conntrack_expect: honor expectation helper field

Ilya Maximets
2026-05-04T12:19:24Z

On 5/1/26 12:37 PM, Pablo Neira Ayuso wrote:
> 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();

I'm not sure. I tried this change and it fixed one case but broke another.
Looking at what we're testing, the old behavior (at least for FTP) was:
"if helper was committed - report it, if not - don't". i.e. it's not really
about the connection being expected it's about if the user committed the
helper for the connection or not.

Let me explain a few scenarios that we have in the OVS system tests and what
I see with the old kernel (6.19), the new (7.0) and the patch above.

A) The first scenario has the following OpenFlow rules (simplified):

table=0,in_port=1,tcp,action=ct(alg=ftp,commit),2
table=0,in_port=2,tcp,action=ct(table=1)
table=1,in_port=2,tcp,ct_state=+trk+est,action=1
table=1,in_port=2,tcp,ct_state=+trk+rel,action=1

This set of rule blindly commits every packet coming from port 1 with the
helper and sends to port 2. Packets from port 2 are passed through ct and
only related or established traffic is passed to port 1. This is a very
rudimentary setup that users can make to allow ftp from port 1 towards port 2,
but not in the opposite direction.

For this scenario regardless of the kernel version or the patch above I see
that both the data and the control connections have a helper reported in the
ctnetlink dump.

B) The second scenario:

table=0,in_port=1,tcp,action=ct(table=1)
table=1,in_port=1,tcp,ct_state=+trk+new,action=ct(commit,alg=ftp),2
table=1,in_port=1,tcp,ct_state=+trk+est,action=2

table=0,in_port=2,tcp,action=ct(table=1)
table=1,in_port=2,tcp,ct_state=+trk+new+rel,action=ct(commit),1
table=1,in_port=2,tcp,ct_state=+trk+est,action=1

This is a more reasonable setup where new connections are committed on the
way from 1 to 2 and new related connections are committed on the way from
2 to 1. This allows port 1 to initiate the control connection and the port
2 to initiate the related data connection.

In case of active FTP (port 1 initiates control, port 2 initiates the data):
- old: control has the helper, data does not.
- new: both have the helper.
- patch: control has the helper, data does not.

In case of passive FTP (port 1 initiates both the control and data):
- old: both have the helper (because both are +new traffic from port 1).
- new: both have the helper.
- patch: control has the helper, data does not.

C) We can modify the scenario B to avoid committing the helper on related:

table=0,in_port=1,tcp,action=ct(table=1)
table=1,in_port=1,tcp,ct_state=+trk+new-rel,action=ct(commit,alg=ftp),2
table=1,in_port=1,tcp,ct_state=+trk+new+rel,action=ct(commit),2
table=1,in_port=1,tcp,ct_state=+trk+est,action=2

table=0,in_port=2,tcp,action=ct(table=1)
table=1,in_port=2,tcp,ct_state=+trk+new+rel,action=ct(commit),1
table=1,in_port=2,tcp,ct_state=+trk+est,action=1

Here we have (same for active and passive):
- old: control has the helper, data does not.
- new: both have the helper.
- patch: control has the helper, data does not.

I hope that clarifies the situation a little bit.

So, if we want to restore the old behavior, the we'd probably need to track
how connection gained the helper, i.e. was it via commit or was it inherited.

I'm also not sure why we see the helper with the patch above in scenario A that
commits established traffic, but not in B or C that only commits new traffic.

Best regards, Ilya Maximets.