* [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease
@ 2020-06-24 16:42 Neal Cardwell
2020-06-24 16:42 ` [PATCH net 1/2] tcp_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT Neal Cardwell
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Neal Cardwell @ 2020-06-24 16:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell
This series fixes a long-standing bug in the TCP CUBIC
HYSTART_DELAY mechanim recently reported by Mirja Kuehlewind. The
code can cause a spurious exit of slow start in some particular
cases: upon an RTT decrease that happens on the 9th or later ACK
in a round trip. This series fixes the original Hystart code and
also the recent BPF implementation.
Neal Cardwell (2):
tcp_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT
bpf: tcp: bpf_cubic: fix spurious HYSTART_DELAY exit upon drop in min
RTT
net/ipv4/tcp_cubic.c | 5 ++---
tools/testing/selftests/bpf/progs/bpf_cubic.c | 5 ++---
2 files changed, 4 insertions(+), 6 deletions(-)
--
2.27.0.111.gc72c7da667-goog
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net 1/2] tcp_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT
2020-06-24 16:42 [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease Neal Cardwell
@ 2020-06-24 16:42 ` Neal Cardwell
2020-06-24 16:42 ` [PATCH net 2/2] bpf: tcp: bpf_cubic: " Neal Cardwell
2020-06-25 23:09 ` [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Neal Cardwell @ 2020-06-24 16:42 UTC (permalink / raw)
To: David Miller
Cc: netdev, Neal Cardwell, Mirja Kuehlewind, Eric Dumazet,
Soheil Hassas Yeganeh
Mirja Kuehlewind reported a bug in Linux TCP CUBIC Hystart, where
Hystart HYSTART_DELAY mechanism can exit Slow Start spuriously on an
ACK when the minimum rtt of a connection goes down. From inspection it
is clear from the existing code that this could happen in an example
like the following:
o The first 8 RTT samples in a round trip are 150ms, resulting in a
curr_rtt of 150ms and a delay_min of 150ms.
o The 9th RTT sample is 100ms. The curr_rtt does not change after the
first 8 samples, so curr_rtt remains 150ms. But delay_min can be
lowered at any time, so delay_min falls to 100ms. The code executes
the HYSTART_DELAY comparison between curr_rtt of 150ms and delay_min
of 100ms, and the curr_rtt is declared far enough above delay_min to
force a (spurious) exit of Slow start.
The fix here is simple: allow every RTT sample in a round trip to
lower the curr_rtt.
Fixes: ae27e98a5152 ("[TCP] CUBIC v2.3")
Reported-by: Mirja Kuehlewind <mirja.kuehlewind@ericsson.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
net/ipv4/tcp_cubic.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 8f8eefd3a3ce..c7bf5b26bf0c 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -432,10 +432,9 @@ static void hystart_update(struct sock *sk, u32 delay)
if (hystart_detect & HYSTART_DELAY) {
/* obtain the minimum delay of more than sampling packets */
+ if (ca->curr_rtt > delay)
+ ca->curr_rtt = delay;
if (ca->sample_cnt < HYSTART_MIN_SAMPLES) {
- if (ca->curr_rtt > delay)
- ca->curr_rtt = delay;
-
ca->sample_cnt++;
} else {
if (ca->curr_rtt > ca->delay_min +
--
2.27.0.111.gc72c7da667-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH net 2/2] bpf: tcp: bpf_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT
2020-06-24 16:42 [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease Neal Cardwell
2020-06-24 16:42 ` [PATCH net 1/2] tcp_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT Neal Cardwell
@ 2020-06-24 16:42 ` Neal Cardwell
2020-06-25 23:09 ` [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Neal Cardwell @ 2020-06-24 16:42 UTC (permalink / raw)
To: David Miller
Cc: netdev, Neal Cardwell, Mirja Kuehlewind, Eric Dumazet,
Soheil Hassas Yeganeh
Apply the fix from:
"tcp_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT"
to the BPF implementation of TCP CUBIC congestion control.
Repeating the commit description here for completeness:
Mirja Kuehlewind reported a bug in Linux TCP CUBIC Hystart, where
Hystart HYSTART_DELAY mechanism can exit Slow Start spuriously on an
ACK when the minimum rtt of a connection goes down. From inspection it
is clear from the existing code that this could happen in an example
like the following:
o The first 8 RTT samples in a round trip are 150ms, resulting in a
curr_rtt of 150ms and a delay_min of 150ms.
o The 9th RTT sample is 100ms. The curr_rtt does not change after the
first 8 samples, so curr_rtt remains 150ms. But delay_min can be
lowered at any time, so delay_min falls to 100ms. The code executes
the HYSTART_DELAY comparison between curr_rtt of 150ms and delay_min
of 100ms, and the curr_rtt is declared far enough above delay_min to
force a (spurious) exit of Slow start.
The fix here is simple: allow every RTT sample in a round trip to
lower the curr_rtt.
Fixes: 6de4a9c430b5 ("bpf: tcp: Add bpf_cubic example")
Reported-by: Mirja Kuehlewind <mirja.kuehlewind@ericsson.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
tools/testing/selftests/bpf/progs/bpf_cubic.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/bpf_cubic.c b/tools/testing/selftests/bpf/progs/bpf_cubic.c
index 7897c8f4d363..ef574087f1e1 100644
--- a/tools/testing/selftests/bpf/progs/bpf_cubic.c
+++ b/tools/testing/selftests/bpf/progs/bpf_cubic.c
@@ -480,10 +480,9 @@ static __always_inline void hystart_update(struct sock *sk, __u32 delay)
if (hystart_detect & HYSTART_DELAY) {
/* obtain the minimum delay of more than sampling packets */
+ if (ca->curr_rtt > delay)
+ ca->curr_rtt = delay;
if (ca->sample_cnt < HYSTART_MIN_SAMPLES) {
- if (ca->curr_rtt > delay)
- ca->curr_rtt = delay;
-
ca->sample_cnt++;
} else {
if (ca->curr_rtt > ca->delay_min +
--
2.27.0.111.gc72c7da667-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease
2020-06-24 16:42 [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease Neal Cardwell
2020-06-24 16:42 ` [PATCH net 1/2] tcp_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT Neal Cardwell
2020-06-24 16:42 ` [PATCH net 2/2] bpf: tcp: bpf_cubic: " Neal Cardwell
@ 2020-06-25 23:09 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-06-25 23:09 UTC (permalink / raw)
To: ncardwell; +Cc: netdev
From: Neal Cardwell <ncardwell@google.com>
Date: Wed, 24 Jun 2020 12:42:01 -0400
> This series fixes a long-standing bug in the TCP CUBIC
> HYSTART_DELAY mechanim recently reported by Mirja Kuehlewind. The
> code can cause a spurious exit of slow start in some particular
> cases: upon an RTT decrease that happens on the 9th or later ACK
> in a round trip. This series fixes the original Hystart code and
> also the recent BPF implementation.
Series applied and queued up for -stable, thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-25 23:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 16:42 [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease Neal Cardwell
2020-06-24 16:42 ` [PATCH net 1/2] tcp_cubic: fix spurious HYSTART_DELAY exit upon drop in min RTT Neal Cardwell
2020-06-24 16:42 ` [PATCH net 2/2] bpf: tcp: bpf_cubic: " Neal Cardwell
2020-06-25 23:09 ` [PATCH net 0/2] tcp_cubic: fix spurious HYSTART_DELAY on RTT decrease David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).