On Wed, Mar 13, 2024 at 04:25:28PM +0100, Florian Westphal wrote:
> Sven Auhagen <sven.auhagen@xxxxxxxxxxxx> wrote:
> > On Wed, Mar 13, 2024 at 04:02:03PM +0100, Florian Westphal wrote:
> > > Florian Westphal <fw@xxxxxxxxx> wrote:
> > > > No idea, but it was intentional, see
> > > > b6f27d322a0a ("netfilter: nf_flow_table: tear down TCP flows if RST or FIN was seen")
> > >
> > > Maybe:
> > >
> > > diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> > > --- a/net/netfilter/nf_flow_table_ip.c
> > > +++ b/net/netfilter/nf_flow_table_ip.c
> > > @@ -28,10 +28,8 @@ static int nf_flow_state_check(struct flow_offload *flow, int proto,
> > > return 0;
> > >
> > > tcph = (void *)(skb_network_header(skb) + thoff);
> > > - if (unlikely(tcph->fin || tcph->rst)) {
> > > - flow_offload_teardown(flow);
> > > + if (unlikely(tcph->fin || tcph->rst))
> > > return -1;
> > > - }
> > >
> > > return 0;
> > > }
> > >
> > > ?
> > >
> > > This will let gc step clean the entry from the flowtable.
> > Thanks for your answer.
> >
> > I double checked and the problem is that the timeout in flow_offload_fixup_ct is set to a very small value
> > and the state is deleted immediately afterwards.
>
> but from where is the call to flow_offload_fixup_ct() made?
>
> I don't think tearing down the flowtable entry on first fin or rst makes
> any sense, its racy by design.
I tested your patch but that leads to other problems.
The ct gc relies on the fact that the flowtable code is setting
the ct timeout to a high value so it is not removed.
With your patch the state for TCP is going into CLOSE while
still beeing offloaded and eventuelly beeing destroyed while
still offloaded. So now bad things happen during flowtable gc
we are accessing the state in there and it has been destroyed already.
So either we need to make the ct gc aware of the offload flag or
need to keep the call to flow_offload_teardown.
Any thoughts?
