[PATCH] TCP window tracking retransmission handling

[PATCH] TCP window tracking retransmission handling

[Phil Oester]

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

Under certain circumstances (high latency WAN links for instance), ack
packets get stacked up and arrive in bulk.  The current TCP window
tracking code interprets these numerous acks as retransmits, and
if there are >= 3 retransmits sequentially, it resets the timeout on
a conntrack to 5 minutes.

This is trivially reproducible on a high latency link by an 'ls -lR'
on a large-ish tree.  In my test case, 8 ack packets arrived sequentially
at the end of the listing.  While the seq numbers on those packets
were indeed identical (which the current code tests for), they were acking
unique packets, and thus clearly do not qualify as retransmissions.

The problem lies in the fact that the code currently only examines
the seq number of the arriving packet, but does not also look at the
seq number being acked.  The patch below adds this additional check.
Unfortunately, it adds another int32 to ip_ct_tcp, but I could think
of no other fool-proof way of fixing it (short of ripping out the
retransmission test altogether).

Phil

Signed-off-by: Phil Oester <kernel@linuxace.com>



[-- Attachment #2: patch-ack --]
[-- Type: text/plain, Size: 1305 bytes --]

diff -ru linux-orig/include/linux/netfilter_ipv4/ip_conntrack_tcp.h linux-new/include/linux/netfilter_ipv4/ip_conntrack_tcp.h
--- linux-orig/include/linux/netfilter_ipv4/ip_conntrack_tcp.h	2004-12-24 16:34:31.000000000 -0500
+++ linux-new/include/linux/netfilter_ipv4/ip_conntrack_tcp.h	2005-01-25 00:31:46.772442512 -0500
@@ -41,6 +41,7 @@
 	u_int8_t	retrans;	/* Number of retransmitted packets */
 	u_int8_t	last_index;	/* Index of the last packet */
 	u_int32_t	last_seq;	/* Last sequence number seen in dir */
+	u_int32_t	last_ack;	/* Last sequence number seen in opposite dir */
 	u_int32_t	last_end;	/* Last seq + len */
 };
 
diff -ru linux-orig/net/ipv4/netfilter/ip_conntrack_proto_tcp.c linux-new/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
--- linux-orig/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2005-01-25 00:46:13.192726608 -0500
+++ linux-new/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2005-01-25 00:43:35.340723760 -0500
@@ -665,11 +665,13 @@
 		if (*index == TCP_ACK_SET) {
 			if (state->last_dir == dir
 			    && state->last_seq == seq
+			    && state->last_ack == ack
 			    && state->last_end == end)
 				state->retrans++;
 			else {
 				state->last_dir = dir;
 				state->last_seq = seq;
+				state->last_ack = ack;
 				state->last_end = end;
 				state->retrans = 0;
 			}

Re: [PATCH] TCP window tracking retransmission handling

[Jozsef Kadlecsik]

Hi Phil,

On Mon, 24 Jan 2005, Phil Oester wrote:

> Under certain circumstances (high latency WAN links for instance), ack
> packets get stacked up and arrive in bulk.  The current TCP window
> tracking code interprets these numerous acks as retransmits, and
> if there are >= 3 retransmits sequentially, it resets the timeout on
> a conntrack to 5 minutes.
>
> This is trivially reproducible on a high latency link by an 'ls -lR'
> on a large-ish tree.  In my test case, 8 ack packets arrived sequentially
> at the end of the listing.  While the seq numbers on those packets
> were indeed identical (which the current code tests for), they were acking
> unique packets, and thus clearly do not qualify as retransmissions.
>
> The problem lies in the fact that the code currently only examines
> the seq number of the arriving packet, but does not also look at the
> seq number being acked.  The patch below adds this additional check.
> Unfortunately, it adds another int32 to ip_ct_tcp, but I could think
> of no other fool-proof way of fixing it (short of ripping out the
> retransmission test altogether).

If we really want to attempt to detect resent packets, then I agree with
you and the proposed solution. I was reluctant to add another field to
ip_ct_tcp, but that cannot be avoided in order to do the job properly.

[However, from another point of view, we actually could get rid of the
feeble attempt of detecting resent packets (and thus the retrans and
last_end fields) with the price of possible dangling connections in the
conntrack table. The current code handles just fine "reopening" stuck
connections.]

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] TCP window tracking retransmission handling

[Martin Josefsson]

On Tue, 25 Jan 2005, Jozsef Kadlecsik wrote:

> If we really want to attempt to detect resent packets, then I agree with
> you and the proposed solution. I was reluctant to add another field to
> ip_ct_tcp, but that cannot be avoided in order to do the job properly.
>
> [However, from another point of view, we actually could get rid of the
> feeble attempt of detecting resent packets (and thus the retrans and
> last_end fields) with the price of possible dangling connections in the
> conntrack table. The current code handles just fine "reopening" stuck
> connections.]

I think this is one of the features that really help keep the hashtable
small and nice. And of course the ability to disable connection pickup by
disabling ip_conntrack_tcp_loose. With the old code I easily had ~100k
entries in the hashtable, with the new code and loose disabled I see
35-40k entries unless there's a lot of udp scanning going on.

I'd like for it to continue existing unless it bloats the code and
datastructures too much.

/Martin

Re: [PATCH] TCP window tracking retransmission handling

[Jozsef Kadlecsik]

On Tue, 25 Jan 2005, Martin Josefsson wrote:

> > [However, from another point of view, we actually could get rid of the
> > feeble attempt of detecting resent packets (and thus the retrans and
> > last_end fields) with the price of possible dangling connections in the
> > conntrack table. The current code handles just fine "reopening" stuck
> > connections.]
>
> I think this is one of the features that really help keep the hashtable
> small and nice. And of course the ability to disable connection pickup by
> disabling ip_conntrack_tcp_loose. With the old code I easily had ~100k
> entries in the hashtable, with the new code and loose disabled I see
> 35-40k entries unless there's a lot of udp scanning going on.

Thank you, such reports are always wanted.

> I'd like for it to continue existing unless it bloats the code and
> datastructures too much.

Patrick, please submit the patch for kernel inclusion.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] TCP window tracking retransmission handling

[Patrick McHardy]

Jozsef Kadlecsik wrote:

>Patrick, please submit the patch for kernel inclusion.
>
I've added it to my tree, I'm going to push it to Dave this weekend.

Regards
Patrick