[PATCH] tcp_cubic: enable TCP timestamps

[PATCH] tcp_cubic: enable TCP timestamps

[Lucas Nussbaum]

The Hystart slow start algorithm requires precise RTT delay measurements
to decide when to leave slow start. However, currently, CUBIC doesn't
enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
and to leave slow start too early, generating bad performance since
convergence to the optimal cwnd is slower.

Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.

Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>

-- 
| Lucas Nussbaum             MCF Université Nancy 2 |
| lucas.nussbaum@loria.fr         LORIA / AlGorille |
| http://www.loria.fr/~lnussbau/  +33 3 54 95 86 19 |

diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 71d5f2f..3a73509 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -406,6 +406,7 @@ static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt_us)
 }
 
 static struct tcp_congestion_ops cubictcp = {
+       .flags          = TCP_CONG_RTT_STAMP,
        .init           = bictcp_init,
        .ssthresh       = bictcp_recalc_ssthresh,
        .cong_avoid     = bictcp_cong_avoid,

Re: [PATCH] tcp_cubic: enable TCP timestamps

[Stephen Hemminger]

On Tue, 8 Mar 2011 09:09:26 +0100
Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:

> The Hystart slow start algorithm requires precise RTT delay measurements
> to decide when to leave slow start. However, currently, CUBIC doesn't
> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
> and to leave slow start too early, generating bad performance since
> convergence to the optimal cwnd is slower.
> 
> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
> 
> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>

Just to explain what RTT_STAMP does. It causes the tcp receive code
to compute the rtt using high resolution clocks rather than just
jiffies. It requires access to ktime_get_real which means accessing
clock source. This is cheap for TSC, a little expensive for HPET but
expensive for PIT. I worry that enabling it may hurt regular users
on old desktops.  But without it enabling RTT_STAMP, packets that
get acked in less than a jiffie (1 - 10 ms) will 

Also I should have used ktime_get rather than ktime_get_real
because real time is altered by NTP and other actions.

Re: [PATCH] tcp_cubic: enable TCP timestamps

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 8 Mar 2011 10:42:11 -0800

> On Tue, 8 Mar 2011 09:09:26 +0100
> Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> 
>> The Hystart slow start algorithm requires precise RTT delay measurements
>> to decide when to leave slow start. However, currently, CUBIC doesn't
>> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
>> and to leave slow start too early, generating bad performance since
>> convergence to the optimal cwnd is slower.
>> 
>> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
>> 
>> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
> 
> Just to explain what RTT_STAMP does. It causes the tcp receive code
> to compute the rtt using high resolution clocks rather than just
> jiffies. It requires access to ktime_get_real which means accessing
> clock source. This is cheap for TSC, a little expensive for HPET but
> expensive for PIT. I worry that enabling it may hurt regular users
> on old desktops.  But without it enabling RTT_STAMP, packets that
> get acked in less than a jiffie (1 - 10 ms) will 
> 
> Also I should have used ktime_get rather than ktime_get_real
> because real time is altered by NTP and other actions.

This also means that whatever testing was originally done on Hystart
is dependent upon whatever value CONFIG_HZ had in the test kernel.

Re: [PATCH] tcp_cubic: enable TCP timestamps

[Sangtae Ha]

Yes. I remember that CONFIG_HZ was 1000 at that time and the value of
CONFIG_HZ could affect the algorithm.
I don't think HyStart needs this extra RTT_STAMP since we only need a
rough delay estimate.
Let me check HyStart with the latest git and with different CONFIG_HZ values.

Sangtae

On Tue, Mar 8, 2011 at 1:55 PM, David Miller <davem@davemloft.net> wrote:
>
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 8 Mar 2011 10:42:11 -0800
>
> > On Tue, 8 Mar 2011 09:09:26 +0100
> > Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> >
> >> The Hystart slow start algorithm requires precise RTT delay measurements
> >> to decide when to leave slow start. However, currently, CUBIC doesn't
> >> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
> >> and to leave slow start too early, generating bad performance since
> >> convergence to the optimal cwnd is slower.
> >>
> >> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
> >>
> >> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
> >
> > Just to explain what RTT_STAMP does. It causes the tcp receive code
> > to compute the rtt using high resolution clocks rather than just
> > jiffies. It requires access to ktime_get_real which means accessing
> > clock source. This is cheap for TSC, a little expensive for HPET but
> > expensive for PIT. I worry that enabling it may hurt regular users
> > on old desktops.  But without it enabling RTT_STAMP, packets that
> > get acked in less than a jiffie (1 - 10 ms) will
> >
> > Also I should have used ktime_get rather than ktime_get_real
> > because real time is altered by NTP and other actions.
>
> This also means that whatever testing was originally done on Hystart
> is dependent upon whatever value CONFIG_HZ had in the test kernel.

Re: [PATCH] tcp_cubic: enable TCP timestamps

[Lucas Nussbaum]

On 08/03/11 at 14:15 -0500, Sangtae Ha wrote:
> Yes. I remember that CONFIG_HZ was 1000 at that time and the value of
> CONFIG_HZ could affect the algorithm.
> I don't think HyStart needs this extra RTT_STAMP since we only need a
> rough delay estimate.
> Let me check HyStart with the latest git and with different CONFIG_HZ values.

One of the problems I discovered was that ca->delay_min was completely
off if RTT_STAMP is disabled. For example, on a link with a 11ms RTT, I
could get delay_min = 4ms. But even with RTT_STAMP, there's the problem
that the code rounds the computed RTT value to a jiffie (in
bictcp_acked()).

My other patch mitigates the performance problem by making it harder for
Hystart to abort slow start. But after a few days working on this issue,
I'm wondering whether Hystart shouldn't be completely rewritten to work
on the usec values, or disabled by default.
-- 
| Lucas Nussbaum             MCF Université Nancy 2 |
| lucas.nussbaum@loria.fr         LORIA / AlGorille |
| http://www.loria.fr/~lnussbau/  +33 3 54 95 86 19 |

Re: [PATCH] tcp_cubic: enable TCP timestamps

[David Miller]

From: Lucas Nussbaum <lucas.nussbaum@loria.fr>
Date: Tue, 8 Mar 2011 20:36:02 +0100

> My other patch mitigates the performance problem by making it harder for
> Hystart to abort slow start. But after a few days working on this issue,
> I'm wondering whether Hystart shouldn't be completely rewritten to work
> on the usec values, or disabled by default.

I am of the opinion that Hystart, like TCP VEGAS, fundamentally cannot
work.

It's disabled in several major distributions because of the performance
problems you noticed, and this was on my own personal recommendation.

Re: [PATCH] tcp_cubic: enable TCP timestamps

[Stephen Hemminger]

On Tue, 08 Mar 2011 10:55:03 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

Complete the sentence...

>  on old desktops.  

   But without it enabling RTT_STAMP, packets that
   get acked in less than a jiffie (1 - 10 ms) will 
   be processed with no rtt value (-1); this causes hystart
   never to be invoked on local connections.

Maybe the following would be better, it passes 0 as RTT
when not using RTT_STAMP for quick packets.

It turns out the Cubic ends up converting rtt_us back
to jiffies anyway, so RTT_STAMP is not really needed.

----
Subject: tcp: fix RTT for quick packets in congestion control

In the congestion control interface, the callback for each ACK
includes an estimated round trip time in microseconds.
Some algorithms need high resolution (Vegas style) but most only
need jiffie resolution.  If RTT is not accurate (like a retransmission)
-1 is used as a flag value.

When doing coarse resolution if RTT is less than a a jiffie
then 0 should be returned rather than no estimate. Otherwise algorithms
that expect good ack's to trigger slow start (like CUBIC Hystart)
will be confused.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/ipv4/tcp_input.c	2011-03-08 11:11:26.093183654 -0800
+++ b/net/ipv4/tcp_input.c	2011-03-08 11:11:46.641404939 -0800
@@ -3350,7 +3350,7 @@ static int tcp_clean_rtx_queue(struct so
 						 net_invalid_timestamp()))
 					rtt_us = ktime_us_delta(ktime_get_real(),
 								last_ackt);
-				else if (ca_seq_rtt > 0)
+				else if (ca_seq_rtt >= 0)
 					rtt_us = jiffies_to_usecs(ca_seq_rtt);
 			}