On Thu, Mar 14, 2024 at 01:56:12PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Mar 14, 2024 at 01:43:52PM +0100, Sven Auhagen wrote:
> > On Thu, Mar 14, 2024 at 01:38:54PM +0100, Pablo Neira Ayuso wrote:
> > > On Thu, Mar 14, 2024 at 12:30:30PM +0100, Sven Auhagen wrote:
> [...]
> > > > I found this out.
> > > > The state is deleted in the end because the flow_offload_fixup_ct
> > > > function is pulling the FIN_WAIT timeout and deducts the offload_timeout
> > > > from it. This is 0 or very close to 0 and therefore ct gc is deleting the state
> > > > more or less right away after the flow_offload_teardown is called
> > > > (for the second time).
> > >
> > > This used to be set to:
> > >
> > > timeout = tn->timeouts[TCP_CONNTRACK_ESTABLISHED];
> > >
> > > but after:
> > >
> > > commit e5eaac2beb54f0a16ff851125082d9faeb475572
> > > Author: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> > > Date: Tue May 17 10:44:14 2022 +0200
> > >
> > > netfilter: flowtable: fix TCP flow teardown
> > >
> > > it uses the current state.
> >
> > Yes since the TCP state might not be established anymore at that time.
> > Your patch will work but also give my TCP_FIN a very large timeout.
>
> Is that still possible? My patch also fixes up conntrack _before_
> the offload flag is cleared, so the packet in the reply direction
> either sees the already fixed up conntrack or it follows flowtable
> datapath.
>
> > I have successfully tested this version today:
> >
> > - if (timeout < 0)
> > - timeout = 0;
> > + // Have at least some time left on the state
> > + if (timeout < NF_FLOW_TIMEOUT)
> > + timeout = NF_FLOW_TIMEOUT;
> >
> > This makes sure that the timeout is not so big like ESTABLISHED but still enough
> > so the state does not time out right away.
>
> This also seems sensible to me. Currently it is using the last conntrack
> state that we have observed when conntrack handed over this flow to the
> flowtable, which is inaccurate in any case, and which could still be low
> depending on user-defined tcp conntrack timeouts (in case user decided
> to tweaks them).
If I take my patch and the part of your patch that moves up flow_offload_fixup_ct
to the top of the function it works now.
As expected, it still happens that a flow is ending up in FIN_WAIT OFFLOADED but
the state is no longer deleted right away and is eventually removed from the flowoffload
via gc and then goes on until the end.
[NEW] tcp 6 120 SYN_SENT src=192.168.7.100 dst=17.248.209.132 sport=54774 dport=443 [UNREPLIED] src=17.248.209.132 dst=87.138.198.79 sport=443 dport=30121 mark=25165825
[UPDATE] tcp 6 60 SYN_RECV src=192.168.7.100 dst=17.248.209.132 sport=54774 dport=443 src=17.248.209.132 dst=87.138.198.79 sport=443 dport=30121 mark=25165825
[UPDATE] tcp 6 86400 ESTABLISHED src=192.168.7.100 dst=17.248.209.132 sport=54774 dport=443 src=17.248.209.132 dst=87.138.198.79 sport=443 dport=30121 [OFFLOAD] mark=25165825
[DESTROY] tcp 6 TIME_WAIT src=192.168.4.81 dst=192.168.6.8 sport=54774 dport=8080 packets=6 bytes=1388 src=192.168.6.8 dst=192.168.4.81 sport=8080 dport=54774 packets=4 bytes=398 [ASSURED] mark=16777216 delta-time=120
[UPDATE] tcp 6 120 FIN_WAIT src=192.168.7.100 dst=17.248.209.132 sport=54774 dport=443 src=17.248.209.132 dst=87.138.198.79 sport=443 dport=30121 [OFFLOAD] mark=25165825
[UPDATE] tcp 6 30 LAST_ACK src=192.168.7.100 dst=17.248.209.132 sport=54774 dport=443 src=17.248.209.132 dst=87.138.198.79 sport=443 dport=30121 [OFFLOAD] mark=25165825
[UPDATE] tcp 6 10 CLOSE src=192.168.7.100 dst=17.248.209.132 sport=54774 dport=443 src=17.248.209.132 dst=87.138.198.79 sport=443 dport=30121 [ASSURED] mark=25165825
[DESTROY] tcp 6 CLOSE src=192.168.7.100 dst=17.248.209.132 sport=54774 dport=443 packets=31 bytes=5542 src=17.248.209.132 dst=87.138.198.79 sport=443 dport=30121 packets=35 bytes=8168 [ASSURED] mark=25165825 delta-time=50
