Re: [PATCH nf] netfilter: flowtable: infer TCP state and timeout before flow teardown

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Sven,

On Wed, Mar 20, 2024 at 09:39:16AM +0100, Sven Auhagen wrote:
> On Mon, Mar 18, 2024 at 10:39:15AM +0100, Pablo Neira Ayuso wrote:
[...]
> > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> > index a0571339239c..481fe3d96bbc 100644
> > --- a/net/netfilter/nf_flow_table_core.c
> > +++ b/net/netfilter/nf_flow_table_core.c
> > @@ -165,10 +165,22 @@ void flow_offload_route_init(struct flow_offload *flow,
> >  }
> >  EXPORT_SYMBOL_GPL(flow_offload_route_init);
> >  
> > -static void flow_offload_fixup_tcp(struct ip_ct_tcp *tcp)
> > +static s32 flow_offload_fixup_tcp(struct net *net, struct nf_conn *ct,
> > +				  enum tcp_conntrack tcp_state)
> >  {
> > -	tcp->seen[0].td_maxwin = 0;
> > -	tcp->seen[1].td_maxwin = 0;
> > +	struct nf_tcp_net *tn = nf_tcp_pernet(net);
> > +
> > +	ct->proto.tcp.state = tcp_state;
> > +	ct->proto.tcp.seen[0].td_maxwin = 0;
> > +	ct->proto.tcp.seen[1].td_maxwin = 0;
> > +
> > +	/* Similar to mid-connection pickup with loose=1.
> > +	 * Avoid large ESTABLISHED timeout.
> > +	 */
> > +	if (tcp_state == TCP_CONNTRACK_ESTABLISHED)
> > +		return tn->timeouts[TCP_CONNTRACK_UNACK];
> 
> Hi Pablo,
> 
> I tested the patch but the part that sets the timout to UNACK is not
> very practical.
> For example my long running SSH connections get killed off by the firewall
> regularly now while beeing ESTABLISHED:
> 
> [NEW] tcp      6 120 SYN_SENT src=192.168.6.55 dst=192.168.10.22 sport=55582 dport=22 [UNREPLIED] src=192.168.10.22 dst=192.168.6.55 sport=22 dport=55582 mark=16777216
> [UPDATE] tcp      6 60 SYN_RECV src=192.168.6.55 dst=192.168.10.22 sport=55582 dport=22 src=192.168.10.22 dst=192.168.6.55 sport=22 dport=55582 mark=16777216
> [UPDATE] tcp      6 86400 ESTABLISHED src=192.168.6.55 dst=192.168.10.22 sport=55582 dport=22 src=192.168.10.22 dst=192.168.6.55 sport=22 dport=55582 [OFFLOAD] mark=16777216
> 
> [DESTROY] tcp      6 ESTABLISHED src=192.168.6.55 dst=192.168.10.22 sport=54941 dport=22 packets=133 bytes=13033 src=192.168.10.22 dst=192.168.6.55 sport=22 dport=54941 packets=95 bytes=15004 [ASSURED] mark=16777216 delta-time=1036
> 
> I would remove the if case here.

OK, I remove it and post a v2. Thanks!

> > +
> > +	return tn->timeouts[tcp_state];
> >  }
> >  
> >  static void flow_offload_fixup_ct(struct nf_conn *ct)