Skip to contentSkip to conversation

net: TCP thin linear timeouts

PATCH Superseded Andreas Petlund 4 participants 9 messages Feb 16, 2010 – Feb 18, 2010

Subsystems (4)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

PATCH v4 2/3 net: TCP thin linear timeouts

Major changes:
-Renamed variables, ioctl and sysctl.
-Added sysctl documentation.
-Removed redundant allocation.

---
 Documentation/networking/ip-sysctl.txt |   12 ++++++++++++
 include/linux/tcp.h                    |    3 +++
 include/net/tcp.h                      |    4 ++++
 net/ipv4/sysctl_net_ipv4.c             |    7 +++++++
 net/ipv4/tcp.c                         |    7 +++++++
 net/ipv4/tcp_timer.c                   |   20 +++++++++++++++++++-
 6 files changed, 52 insertions(+), 1 deletions(-)
Documentation/networking/ip-sysctl.txt +12 -0
index 2dc7a1d..f147310 100644
@@ -487,6 +487,18 @@ tcp_dma_copybreak - INTEGER
 	and CONFIG_NET_DMA is enabled.
 	Default: 4096
 
+tcp_thin_linear_timeouts - BOOLEAN
+	Enable dynamic triggering of linear timeouts for thin streams.
+	If set, a check is performed upon retransmission by timeout to
+	determine if the stream is thin (less than 4 packets in flight).
+	As long as the stream is found to be thin, up to 6 linear
+	timeouts may be performed before exponential backoff mode is
+	initiated. This improves retransmission latency for
+	non-aggressive thin streams, often found to be time-dependent.
+	For more information on thin streams, see
+	Documentation/networking/tcp-thin.txt
+	Default: 0
+
 UDP variables:
 
 udp_mem - vector of 3 INTEGERs: min, pressure, max
include/linux/tcp.h +3 -0
index 7fee8a4..42885af 100644
@@ -103,6 +103,7 @@ enum {
 #define TCP_CONGESTION		13	/* Congestion control algorithm */
 #define TCP_MD5SIG		14	/* TCP MD5 Signature (RFC2385) */
 #define TCP_COOKIE_TRANSACTIONS	15	/* TCP Cookie Transactions */
+#define TCP_THIN_LINEAR_TIMEOUTS 16      /* Use linear timeouts for thin streams*/
 
 /* for TCP_INFO socket option */
 #define TCPI_OPT_TIMESTAMPS	1
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS			*/
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?             */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin streams */
+		thin_undef  : 7;
 
 /* RTT measurement */
 	u32	srtt;		/* smoothed round trip time << 3	*/
include/net/tcp.h +4 -0
index e5e2056..be49356 100644
@@ -196,6 +196,9 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
 #define TCP_NAGLE_CORK		2	/* Socket is corked	    */
 #define TCP_NAGLE_PUSH		4	/* Cork is overridden for already queued data */
 
+/* TCP thin-stream limits */
+#define TCP_THIN_LINEAR_RETRIES 6       /* After 6 linear retries, do exp. backoff */
+
 extern struct inet_timewait_death_row tcp_death_row;
 
 /* sysctl variables for tcp */
@@ -241,6 +244,7 @@ extern int sysctl_tcp_workaround_signed_windows;
 extern int sysctl_tcp_slow_start_after_idle;
 extern int sysctl_tcp_max_ssthresh;
 extern int sysctl_tcp_cookie_size;
+extern int sysctl_tcp_thin_linear_timeouts;
 
 extern atomic_t tcp_memory_allocated;
 extern struct percpu_counter tcp_sockets_allocated;
net/ipv4/sysctl_net_ipv4.c +7 -0
index 7e3712c..e6a2460 100644
@@ -576,6 +576,13 @@ static struct ctl_table ipv4_table[] = {
 		.proc_handler	= proc_dointvec
 	},
 	{
+		.procname       = "tcp_thin_linear_timeouts",
+		.data           = &sysctl_tcp_thin_linear_timeouts,
+		.maxlen         = sizeof(int),
+		.mode           = 0644,
+		.proc_handler   = proc_dointvec
+	},
+	{
 		.procname	= "udp_mem",
 		.data		= &sysctl_udp_mem,
 		.maxlen		= sizeof(sysctl_udp_mem),
net/ipv4/tcp.c +7 -0
index d5d69ea..a159c0d 100644
@@ -2229,6 +2229,13 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
 		}
 		break;
 
+	case TCP_THIN_LINEAR_TIMEOUTS:
+		if (val < 0 || val > 1)
+			err = -EINVAL;
+		else
+			tp->thin_lto = val;
+		break;
+
 	case TCP_CORK:
 		/* When set indicates to always queue non-full frames.
 		 * Later the user clears this option and we transmit
net/ipv4/tcp_timer.c +19 -1
index de7d1bf..00f7caa 100644
@@ -29,6 +29,7 @@ int sysctl_tcp_keepalive_intvl __read_mostly = TCP_KEEPALIVE_INTVL;
 int sysctl_tcp_retries1 __read_mostly = TCP_RETR1;
 int sysctl_tcp_retries2 __read_mostly = TCP_RETR2;
 int sysctl_tcp_orphan_retries __read_mostly;
+int sysctl_tcp_thin_linear_timeouts __read_mostly;
 
 static void tcp_write_timer(unsigned long);
 static void tcp_delack_timer(unsigned long);
@@ -415,7 +416,24 @@ void tcp_retransmit_timer(struct sock *sk)
 	icsk->icsk_retransmits++;
 
 out_reset_timer:
-	icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
+	/* If stream is thin, use linear timeouts. Since 'icsk_backoff' is
+	 * used to reset timer, set to 0. Recalculate 'icsk_rto' as this
+	 * might be increased if the stream oscillates between thin and thick,
+	 * thus the old value might already be too high compared to the value
+	 * set by 'tcp_set_rto' in tcp_input.c which resets the rto without
+	 * backoff. Limit to TCP_THIN_LINEAR_RETRIES before initiating
+	 * exponential backoff behaviour to avoid continue hammering
+	 * linear-timeout retransmissions into a black hole*/
+	if (sk->sk_state == TCP_ESTABLISHED &&
+	    (tp->thin_lto || sysctl_tcp_thin_linear_timeouts) &&
+	    tcp_stream_is_thin(sk) &&
+	    icsk->icsk_retransmits <= TCP_THIN_LINEAR_RETRIES) {
+		icsk->icsk_backoff = 0;
+		icsk->icsk_rto = min(__tcp_set_rto(tp), TCP_RTO_MAX);
+	} else {
+		/* Use normal (exponential) backoff */
+		icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
+	}
 	inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
 	if (retransmits_timed_out(sk, sysctl_tcp_retries1 + 1))
 		__sk_dst_reset(sk);
Signature · 1 line
1.6.3.3
maintainer
#1 lore raw

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

include/linux/tcp.h struct tcp_sock { · line 347

Quoted 7 lines
include/linux/tcp.h +2 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS			*/
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?             */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin streams */
+		thin_undef  : 7;
 

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Please either find a way to avoid this hole, or document it with a comment.

#2 lore raw
Quoted 14 lines · David Miller

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

include/linux/tcp.h struct tcp_sock { · line 347

Quoted 7 lines
include/linux/tcp.h +2 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS			*/
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?             */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin streams */
+		thin_undef  : 7;
 

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Quoted 2 lines

Please either find a way to avoid this hole, or document it with a comment.

There would be multiple bits free for use in both frto_counter and nonagle byte.

Signature · 1 line
 i.
#3 lore raw
Quoted 16 lines · Ilpo Järvinen
Quoted 14 lines · David Miller

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

include/linux/tcp.h struct tcp_sock { · line 347

Quoted 7 lines
include/linux/tcp.h +2 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS			*/
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?             */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin streams */
+		thin_undef  : 7;
 

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Quoted 6 lines
Quoted 2 lines

Please either find a way to avoid this hole, or document it with a comment.

There would be multiple bits free for use in both frto_counter and nonagle byte.

I was playing aroud with this setup:

=========
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
thin_undef : 2;
=========

Do you think that would do the trick?

Regards,
Andreas

#4 lore raw

Hi,

Quoted 29 lines · Andreas Petlund
Quoted 18 lines · Ilpo Järvinen
Quoted 15 lines · David Miller

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

include/linux/tcp.h struct tcp_sock { · line 348

Quoted 8 lines
include/linux/tcp.h +2 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS			*/
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?             */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin streams */
+		thin_undef  : 7;
 
 

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Quoted 6 lines
Quoted 3 lines

Please either find a way to avoid this hole, or document it with a comment.

There would be multiple bits free for use in both frto_counter and nonagle byte.

I was playing aroud with this setup:

Quoted 9 lines

=========
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
thin_undef : 2;
=========

Do you think that would do the trick?

According to Ilpo, it would be ok to reduce both ftro_counter and nonagle, so why not join all these into u16 and leave the remaining free bits documented for other people. Like this:

u16 frto_counter:x; /* Number of new acks after RTO */ u16 nonagle:y; /* Disable Nagle algorithm? */ u16 thin_lto:1; /* Use linear timeouts for thin streams */ u16 unused:15-x-y;

Not sure about the y and x. Ilpo, can you comment on those values?

Thanks,
Franco

#5 lore raw
Quoted 47 lines · Franco Fichtner
Quoted 32 lines · Andreas Petlund
Quoted 20 lines · Ilpo Järvinen
Quoted 17 lines · David Miller

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

include/linux/tcp.h struct tcp_sock { · line 346

Quoted 10 lines
(quoted diff) +1 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS
 */
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?
 */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin

streams */

include/linux/tcp.h +1 -0
+		thin_undef  : 7;

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Quoted 7 lines
Quoted 3 lines

Please either find a way to avoid this hole, or document it with a comment.

There would be multiple bits free for use in both frto_counter and nonagle byte.

I was playing aroud with this setup:

Quoted 9 lines

=========
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
thin_undef : 2;
=========

Do you think that would do the trick?

According to Ilpo, it would be ok to reduce both ftro_counter and nonagle, so why not join all these into u16 and leave the remaining free bits documented for other people. Like this:

Quoted 6 lines

u16 frto_counter:x; /* Number of new acks after RTO */ u16 nonagle:y; /* Disable Nagle algorithm? */ u16 thin_lto:1; /* Use linear timeouts for thin streams */ u16 unused:15-x-y;

Not sure about the y and x. Ilpo, can you comment on those values?

I don't remember top of the hat how much of nonagle used, but for frto_counter max value was 3 iirc. However, I'm unsure if compiler is nowadays wise enough to handle bitfields in some not all so stupid way. Btw, also in ecn_flags we have some empty space. But all in all that didn't vacate enough to get 4 bytes so I haven't taken effort of combining.

Signature · 1 line
 i.
#6 lore raw
Quoted 54 lines · Ilpo Järvinen
Quoted 52 lines · Franco Fichtner
Quoted 32 lines · Andreas Petlund
Quoted 20 lines · Ilpo Järvinen
Quoted 17 lines · David Miller

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

include/linux/tcp.h struct tcp_sock { · line 346

Quoted 10 lines
(quoted diff) +1 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS
 */
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?
 */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin

streams */

include/linux/tcp.h +1 -0
+		thin_undef  : 7;

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Quoted 7 lines
Quoted 3 lines

Please either find a way to avoid this hole, or document it with a comment.

There would be multiple bits free for use in both frto_counter and nonagle byte.

I was playing aroud with this setup:

Quoted 9 lines

=========
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
thin_undef : 2;
=========

Do you think that would do the trick?

According to Ilpo, it would be ok to reduce both ftro_counter and nonagle, so why not join all these into u16 and leave the remaining free bits documented for other people. Like this:

u16 frto_counter:x; /* Number of new acks after RTO */ u16 nonagle:y; /* Disable Nagle algorithm? */ u16 thin_lto:1; /* Use linear timeouts for thin streams */ u16 unused:15-x-y;

Quoted 4 lines
Quoted 1 line

Not sure about the y and x. Ilpo, can you comment on those values?

I don't remember top of the hat how much of nonagle used, but for frto_counter max value was 3 iirc.

I think nonagle uses 4 bits:
======

#define TCP_NAGLE_OFF           1       /* Nagle's algo is disabled */
#define TCP_NAGLE_CORK          2       /* Socket is corked         */
#define TCP_NAGLE_PUSH          4       /* Cork is overridden for already queued data */

======

Quoted 2 lines

However, I'm unsure if compiler is
nowadays wise enough to handle bitfields in some not all so stupid way.

Would you then recommend to use a byte for each value, thus avoiding the bitfields?

Cheers,
Andreas

#7 lore raw
Quoted 65 lines · Andreas Petlund
Quoted 54 lines · Ilpo Järvinen
Quoted 52 lines · Franco Fichtner
Quoted 32 lines · Andreas Petlund
Quoted 20 lines · Ilpo Järvinen
Quoted 17 lines · David Miller

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

Review

Quoted 10 lines
(quoted diff) +1 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS
 */
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?
 */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin

streams */

(quoted diff) +1 -0
+		thin_undef  : 7;

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Quoted 7 lines
Quoted 3 lines

Please either find a way to avoid this hole, or document it with a comment.

There would be multiple bits free for use in both frto_counter and nonagle byte.

I was playing aroud with this setup:

Quoted 9 lines

=========
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
thin_undef : 2;
=========

Do you think that would do the trick?

According to Ilpo, it would be ok to reduce both ftro_counter and nonagle, so why not join all these into u16 and leave the remaining free bits documented for other people. Like this:

u16 frto_counter:x; /* Number of new acks after RTO */ u16 nonagle:y; /* Disable Nagle algorithm? */ u16 thin_lto:1; /* Use linear timeouts for thin streams */ u16 unused:15-x-y;

Quoted 4 lines
Quoted 1 line

Not sure about the y and x. Ilpo, can you comment on those values?

I don't remember top of the hat how much of nonagle used, but for frto_counter max value was 3 iirc.

I think nonagle uses 4 bits:
======

#define TCP_NAGLE_OFF           1       /* Nagle's algo is disabled */
#define TCP_NAGLE_CORK          2       /* Socket is corked         */
#define TCP_NAGLE_PUSH          4       /* Cork is overridden for already queued data */
Quoted 7 lines

======

Quoted 2 lines

However, I'm unsure if compiler is
nowadays wise enough to handle bitfields in some not all so stupid way.

Would you then recommend to use a byte for each value, thus avoiding the bitfields?

I don't know about the current compilers but at least in past there has been a bias against bitfields. Alternative would be to combine and code the accessors manually (thus bypassing any "too clever" compiler intelligence).

Signature · 1 line
 i.
#8 lore raw
In reply to Andreas Petlund
Quoted 68 lines · Andreas Petlund
Quoted 56 lines · Ilpo Järvinen
Quoted 53 lines · Franco Fichtner
Quoted 37 lines · Andreas Petlund
Quoted 23 lines · Ilpo Järvinen
Quoted 19 lines · David Miller

From: Andreas Petlund <[email protected]>
Date: Tue, 16 Feb 2010 15:40:41 +0100

Review

Quoted 11 lines
(quoted diff) +1 -0
@@ -341,6 +342,8 @@ struct tcp_sock {
 	u16	advmss;		/* Advertised MSS
 */
 	u8	frto_counter;	/* Number of new acks after RTO */
 	u8	nonagle;	/* Disable Nagle algorithm?
 */
+	u8      thin_lto    : 1,/* Use linear timeouts for thin

streams */

(quoted diff) +1 -0
+		thin_undef  : 7;

There is now a gap of 3 unused bytes here in this critical core TCP socket data structure.

Quoted 9 lines
Quoted 4 lines

Please either find a way to avoid this hole, or document it with a comment.

There would be multiple bits free for use in both frto_counter and nonagle byte.

I was playing aroud with this setup:

Quoted 10 lines

=========
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
thin_undef : 2;
=========

Do you think that would do the trick?

According to Ilpo, it would be ok to reduce both ftro_counter and nonagle, so why not join all these into u16 and leave the remaining free bits documented for other people. Like this:

Quoted 9 lines
Quoted 7 lines

u16 frto_counter:x; /* Number of new acks after RTO */ u16 nonagle:y; /* Disable Nagle algorithm? */ u16 thin_lto:1; /* Use linear timeouts for thin streams */ u16 unused:15-x-y;

Not sure about the y and x. Ilpo, can you comment on those values?

I don't remember top of the hat how much of nonagle used, but for frto_counter max value was 3 iirc.

Quoted 8 lines

I think nonagle uses 4 bits:
======

#define TCP_NAGLE_OFF           1       /* Nagle's algo is disabled */
#define TCP_NAGLE_CORK          2       /* Socket is corked         */
#define TCP_NAGLE_PUSH          4       /* Cork is overridden for already queued data */

======

That would be 3 bits. :)

Quoted 5 lines
Quoted 2 lines

However, I'm unsure if compiler is nowadays wise enough to handle bitfields in some not all so stupid way.

Would you then recommend to use a byte for each value, thus avoiding the bitfields?

No, he meant he's not sure if the compiler can merge the bitfields in a clever way if written like this.

I use this style all the time at work and according to pahole it's okay. But then, I'm not sure if pahole can be trusted or if there is some compiler twist on non-intel architectures.

Franco