mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: flowtable: do not clear the offload bit twice
@ 2026-11-03  9:52 João Silva via B4 Relay
  2026-11-03  9:59 ` netdev-bot+sinfo
  2026-11-03 13:59 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 4+ messages in thread
From: João Silva via B4 Relay @ 2026-11-03  9:52 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Sven Auhagen, Oz Shlomo
  Cc: netfilter-devel, coreteam, netdev, linux-kernel, João Silva

From: João Silva <jgc3silva@gmail.com>

An expired hardware flow remains in the flowtable until its asynchronous
hardware deletion completes. Teardown clears IPS_OFFLOAD_BIT, so a packet
can create a replacement flow referencing the same conntrack entry while
the old flow is still waiting for deletion.

The next GC pass calls flow_offload_teardown() on the expired old flow
again, clearing the replacement's offload bit. More packets can then
create additional flows for the same conntrack entry. A pending hardware
work item can also delay deletion and expose the same repeated teardown.

Commit d2d31ea8cd80 ("netfilter: conntrack: fix erronous removal of
offload bit") closed this window for software flows by unlinking them in
the same GC pass and made the conntrack state fixup one-shot, but
hardware flows stay linked until NF_FLOW_HW_DEAD is set.

Make clearing IPS_OFFLOAD_BIT part of the existing one-time teardown
transition, alongside the conntrack state fixup. Subsequent teardown calls
must not alter the offload state now owned by a replacement flow.

Fixes: e5eaac2beb54 ("netfilter: flowtable: fix TCP flow teardown")
Signed-off-by: João Silva <jgc3silva@gmail.com>
---
 net/netfilter/nf_flow_table_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 0d72dc8ae47a..709b55b74cc7 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -398,9 +398,11 @@ static void flow_offload_del(struct nf_flowtable *flow_table,
 
 void flow_offload_teardown(struct flow_offload *flow)
 {
+	if (test_and_set_bit(NF_FLOW_TEARDOWN, &flow->flags))
+		return;
+
 	clear_bit(IPS_OFFLOAD_BIT, &flow->ct->status);
-	if (!test_and_set_bit(NF_FLOW_TEARDOWN, &flow->flags))
-		flow_offload_fixup_ct(flow);
+	flow_offload_fixup_ct(flow);
 }
 EXPORT_SYMBOL_GPL(flow_offload_teardown);
 

---
base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
change-id: 20261103-flowtable-offload-bit-f15bfd1d38ed

Best regards,
-- 
João Silva <jgc3silva@gmail.com>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: flowtable: do not clear the offload bit twice
  2026-11-03  9:52 [PATCH nf] netfilter: flowtable: do not clear the offload bit twice João Silva via B4 Relay
@ 2026-11-03  9:59 ` netdev-bot+sinfo
  2026-11-03 13:59 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 4+ messages in thread
From: netdev-bot+sinfo @ 2026-11-03  9:59 UTC (permalink / raw)
  To: João Silva
  Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Sven Auhagen, Oz Shlomo, netfilter-devel, coreteam,
	netdev, linux-kernel

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: flowtable: do not clear the offload bit twice
  2026-11-03  9:52 [PATCH nf] netfilter: flowtable: do not clear the offload bit twice João Silva via B4 Relay
  2026-11-03  9:59 ` netdev-bot+sinfo
@ 2026-11-03 13:59 ` Pablo Neira Ayuso
  2026-11-03 18:56   ` João Silva
  1 sibling, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2026-11-03 13:59 UTC (permalink / raw)
  To: jgc3silva
  Cc: Florian Westphal, Phil Sutter, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Sven Auhagen,
	Oz Shlomo, netfilter-devel, coreteam, netdev, linux-kernel

On Tue, Nov 03, 2026 at 09:52:56AM +0000, João Silva via B4 Relay wrote:
> From: João Silva <jgc3silva@gmail.com>
> 
> An expired hardware flow remains in the flowtable until its asynchronous
> hardware deletion completes. Teardown clears IPS_OFFLOAD_BIT, so a packet
> can create a replacement flow referencing the same conntrack entry while
> the old flow is still waiting for deletion.
> 
> The next GC pass calls flow_offload_teardown() on the expired old flow
> again, clearing the replacement's offload bit. More packets can then
> create additional flows for the same conntrack entry. A pending hardware
> work item can also delay deletion and expose the same repeated teardown.
> 
> Commit d2d31ea8cd80 ("netfilter: conntrack: fix erronous removal of
> offload bit") closed this window for software flows by unlinking them in
> the same GC pass and made the conntrack state fixup one-shot, but
> hardware flows stay linked until NF_FLOW_HW_DEAD is set.
> 
> Make clearing IPS_OFFLOAD_BIT part of the existing one-time teardown
> transition, alongside the conntrack state fixup. Subsequent teardown calls
> must not alter the offload state now owned by a replacement flow.

Let's better target nf-next for this, this looks like LLM triggered
for correctness.

Thanks.

> Fixes: e5eaac2beb54 ("netfilter: flowtable: fix TCP flow teardown")
> Signed-off-by: João Silva <jgc3silva@gmail.com>
> ---
>  net/netfilter/nf_flow_table_core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index 0d72dc8ae47a..709b55b74cc7 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -398,9 +398,11 @@ static void flow_offload_del(struct nf_flowtable *flow_table,
>  
>  void flow_offload_teardown(struct flow_offload *flow)
>  {
> +	if (test_and_set_bit(NF_FLOW_TEARDOWN, &flow->flags))
> +		return;
> +
>  	clear_bit(IPS_OFFLOAD_BIT, &flow->ct->status);
> -	if (!test_and_set_bit(NF_FLOW_TEARDOWN, &flow->flags))
> -		flow_offload_fixup_ct(flow);
> +	flow_offload_fixup_ct(flow);
>  }
>  EXPORT_SYMBOL_GPL(flow_offload_teardown);
>  
> 
> ---
> base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
> change-id: 20261103-flowtable-offload-bit-f15bfd1d38ed
> 
> Best regards,
> -- 
> João Silva <jgc3silva@gmail.com>
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: flowtable: do not clear the offload bit twice
  2026-11-03 13:59 ` Pablo Neira Ayuso
@ 2026-11-03 18:56   ` João Silva
  0 siblings, 0 replies; 4+ messages in thread
From: João Silva @ 2026-11-03 18:56 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Florian Westphal, Phil Sutter, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Sven Auhagen,
	Oz Shlomo, netfilter-devel, coreteam, netdev, linux-kernel

Hi,

The fix was LLM generated but I wouldn't say it's just "for correctness".
For some context, this issue was hit while developing hardware-offloaded 
flows for the RTL930x target in openwrt. There was a point in 
development where this was causing flows to pile up and eventually 
exhaust the TCAM of the PIE engine on the switch.

Thanks for considering.

João Silva


On 03/11/26 13:59, Pablo Neira Ayuso wrote:
> On Tue, Nov 03, 2026 at 09:52:56AM +0000, João Silva via B4 Relay wrote:
>> From: João Silva <jgc3silva@gmail.com>
>>
>> An expired hardware flow remains in the flowtable until its asynchronous
>> hardware deletion completes. Teardown clears IPS_OFFLOAD_BIT, so a packet
>> can create a replacement flow referencing the same conntrack entry while
>> the old flow is still waiting for deletion.
>>
>> The next GC pass calls flow_offload_teardown() on the expired old flow
>> again, clearing the replacement's offload bit. More packets can then
>> create additional flows for the same conntrack entry. A pending hardware
>> work item can also delay deletion and expose the same repeated teardown.
>>
>> Commit d2d31ea8cd80 ("netfilter: conntrack: fix erronous removal of
>> offload bit") closed this window for software flows by unlinking them in
>> the same GC pass and made the conntrack state fixup one-shot, but
>> hardware flows stay linked until NF_FLOW_HW_DEAD is set.
>>
>> Make clearing IPS_OFFLOAD_BIT part of the existing one-time teardown
>> transition, alongside the conntrack state fixup. Subsequent teardown calls
>> must not alter the offload state now owned by a replacement flow.
> Let's better target nf-next for this, this looks like LLM triggered
> for correctness.
>
> Thanks.
>
>> Fixes: e5eaac2beb54 ("netfilter: flowtable: fix TCP flow teardown")
>> Signed-off-by: João Silva <jgc3silva@gmail.com>
>> ---
>>   net/netfilter/nf_flow_table_core.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
>> index 0d72dc8ae47a..709b55b74cc7 100644
>> --- a/net/netfilter/nf_flow_table_core.c
>> +++ b/net/netfilter/nf_flow_table_core.c
>> @@ -398,9 +398,11 @@ static void flow_offload_del(struct nf_flowtable *flow_table,
>>   
>>   void flow_offload_teardown(struct flow_offload *flow)
>>   {
>> +	if (test_and_set_bit(NF_FLOW_TEARDOWN, &flow->flags))
>> +		return;
>> +
>>   	clear_bit(IPS_OFFLOAD_BIT, &flow->ct->status);
>> -	if (!test_and_set_bit(NF_FLOW_TEARDOWN, &flow->flags))
>> -		flow_offload_fixup_ct(flow);
>> +	flow_offload_fixup_ct(flow);
>>   }
>>   EXPORT_SYMBOL_GPL(flow_offload_teardown);
>>   
>>
>> ---
>> base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
>> change-id: 20261103-flowtable-offload-bit-f15bfd1d38ed
>>
>> Best regards,
>> -- 
>> João Silva <jgc3silva@gmail.com>
>>
>>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-03 18:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03  9:52 [PATCH nf] netfilter: flowtable: do not clear the offload bit twice João Silva via B4 Relay
2026-11-03  9:59 ` netdev-bot+sinfo
2026-11-03 13:59 ` Pablo Neira Ayuso
2026-11-03 18:56   ` João Silva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®