"stretch ACKs" (caused by LRO, GRO, delayed ACKs or middleboxes)
can cause serious performance shortfalls in common congestion
control algorithms. Neal Cardwell submitted a series of patches
starting with commit e73ebb0881ea ("tcp: stretch ACK fixes prep")
to handle stretch ACKs and fixed stretch ACK bugs in Reno and
CUBIC congestion control algorithms.
This patch series continues to fix bic, scalable, veno and yeah
congestion control algorithms to handle stretch ACKs.
Changes in v2:
- Provide [PATCH 0/N] to describe the modifications of this patch series
Pengcheng Yang (5):
tcp: fix stretch ACK bugs in BIC
tcp: fix stretch ACK bugs in Scalable
tcp: stretch ACK fixes in Veno prep
tcp: fix stretch ACK bugs in Veno
tcp: fix stretch ACK bugs in Yeah
net/ipv4/tcp_bic.c | 11 ++++++-----
net/ipv4/tcp_scalable.c | 17 +++++++++--------
net/ipv4/tcp_veno.c | 47 +++++++++++++++++++++++++----------------------
net/ipv4/tcp_yeah.c | 41 +++++++++++------------------------------
4 files changed, 51 insertions(+), 65 deletions(-)
--
1.8.3.1
Change Yeah to properly handle stretch ACKs in additive
increase mode by passing in the count of ACKed packets
to tcp_cong_avoid_ai().
In addition, we re-implemented the scalable path using
tcp_cong_avoid_ai() and removed the pkts_acked variable.
Signed-off-by: Pengcheng Yang <[email protected]>
---
net/ipv4/tcp_yeah.c | 41 +++++++++++------------------------------
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
index e00570d..3bb4487 100644
--- a/net/ipv4/tcp_yeah.c
+++ b/net/ipv4/tcp_yeah.c
@@ -36,8 +36,6 @@ struct yeah {
u32 reno_count;
u32 fast_count;
-
- u32 pkts_acked;
};
static void tcp_yeah_init(struct sock *sk)
@@ -57,18 +55,6 @@ static void tcp_yeah_init(struct sock *sk)
tp->snd_cwnd_clamp = min_t(u32, tp->snd_cwnd_clamp, 0xffffffff/128);
}
-static void tcp_yeah_pkts_acked(struct sock *sk,
- const struct ack_sample *sample)
-{
- const struct inet_connection_sock *icsk = inet_csk(sk);
- struct yeah *yeah = inet_csk_ca(sk);
-
- if (icsk->icsk_ca_state == TCP_CA_Open)
- yeah->pkts_acked = sample->pkts_acked;
-
- tcp_vegas_pkts_acked(sk, sample);
-}
-
static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 acked)
{
struct tcp_sock *tp = tcp_sk(sk);
@@ -77,24 +63,19 @@ static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 acked)
if (!tcp_is_cwnd_limited(sk))
return;
- if (tcp_in_slow_start(tp))
- tcp_slow_start(tp, acked);
+ if (tcp_in_slow_start(tp)) {
+ acked = tcp_slow_start(tp, acked);
+ if (!acked)
+ goto do_vegas;
+ }
- else if (!yeah->doing_reno_now) {
+ if (!yeah->doing_reno_now) {
/* Scalable */
-
- tp->snd_cwnd_cnt += yeah->pkts_acked;
- if (tp->snd_cwnd_cnt > min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT)) {
- if (tp->snd_cwnd < tp->snd_cwnd_clamp)
- tp->snd_cwnd++;
- tp->snd_cwnd_cnt = 0;
- }
-
- yeah->pkts_acked = 1;
-
+ tcp_cong_avoid_ai(tp, min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT),
+ acked);
} else {
/* Reno */
- tcp_cong_avoid_ai(tp, tp->snd_cwnd, 1);
+ tcp_cong_avoid_ai(tp, tp->snd_cwnd, acked);
}
/* The key players are v_vegas.beg_snd_una and v_beg_snd_nxt.
@@ -118,7 +99,7 @@ static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 acked)
* of bytes we send in an RTT is often less than our cwnd will allow.
* So we keep track of our cwnd separately, in v_beg_snd_cwnd.
*/
-
+do_vegas:
if (after(ack, yeah->vegas.beg_snd_nxt)) {
/* We do the Vegas calculations only if we got enough RTT
* samples that we can be reasonably sure that we got
@@ -232,7 +213,7 @@ static u32 tcp_yeah_ssthresh(struct sock *sk)
.set_state = tcp_vegas_state,
.cwnd_event = tcp_vegas_cwnd_event,
.get_info = tcp_vegas_get_info,
- .pkts_acked = tcp_yeah_pkts_acked,
+ .pkts_acked = tcp_vegas_pkts_acked,
.owner = THIS_MODULE,
.name = "yeah",
--
1.8.3.1
Changes BIC to properly handle stretch ACKs in additive
increase mode by passing in the count of ACKed packets
to tcp_cong_avoid_ai().
Signed-off-by: Pengcheng Yang <[email protected]>
---
net/ipv4/tcp_bic.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
index 645cc30..f5f588b 100644
--- a/net/ipv4/tcp_bic.c
+++ b/net/ipv4/tcp_bic.c
@@ -145,12 +145,13 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked)
if (!tcp_is_cwnd_limited(sk))
return;
- if (tcp_in_slow_start(tp))
- tcp_slow_start(tp, acked);
- else {
- bictcp_update(ca, tp->snd_cwnd);
- tcp_cong_avoid_ai(tp, ca->cnt, 1);
+ if (tcp_in_slow_start(tp)) {
+ acked = tcp_slow_start(tp, acked);
+ if (!acked)
+ return;
}
+ bictcp_update(ca, tp->snd_cwnd);
+ tcp_cong_avoid_ai(tp, ca->cnt, acked);
}
/*
--
1.8.3.1
Change Veno to properly handle stretch ACKs in additive
increase mode by passing in the count of ACKed packets
to tcp_cong_avoid_ai().
Signed-off-by: Pengcheng Yang <[email protected]>
---
net/ipv4/tcp_veno.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index 857491c..50a9a6e 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -154,8 +154,9 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 acked)
if (tcp_in_slow_start(tp)) {
/* Slow start. */
- tcp_slow_start(tp, acked);
- goto done;
+ acked = tcp_slow_start(tp, acked);
+ if (!acked)
+ goto done;
}
/* Congestion avoidance. */
@@ -163,7 +164,7 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 acked)
/* In the "non-congestive state", increase cwnd
* every rtt.
*/
- tcp_cong_avoid_ai(tp, tp->snd_cwnd, 1);
+ tcp_cong_avoid_ai(tp, tp->snd_cwnd, acked);
} else {
/* In the "congestive state", increase cwnd
* every other rtt.
@@ -177,7 +178,7 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 acked)
veno->inc = 1;
tp->snd_cwnd_cnt = 0;
} else
- tp->snd_cwnd_cnt++;
+ tp->snd_cwnd_cnt += acked;
}
done:
if (tp->snd_cwnd < 2)
--
1.8.3.1
Change Scalable to properly handle stretch ACKs in additive
increase mode by passing in the count of ACKed packets to
tcp_cong_avoid_ai().
In addition, because we are now precisely accounting for
stretch ACKs, including delayed ACKs, we can now change
TCP_SCALABLE_AI_CNT to 100.
Signed-off-by: Pengcheng Yang <[email protected]>
---
net/ipv4/tcp_scalable.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/tcp_scalable.c b/net/ipv4/tcp_scalable.c
index 471571e..6cebf41 100644
--- a/net/ipv4/tcp_scalable.c
+++ b/net/ipv4/tcp_scalable.c
@@ -10,10 +10,9 @@
#include <net/tcp.h>
/* These factors derived from the recommended values in the aer:
- * .01 and and 7/8. We use 50 instead of 100 to account for
- * delayed ack.
+ * .01 and and 7/8.
*/
-#define TCP_SCALABLE_AI_CNT 50U
+#define TCP_SCALABLE_AI_CNT 100U
#define TCP_SCALABLE_MD_SCALE 3
static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked)
@@ -23,11 +22,13 @@ static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked)
if (!tcp_is_cwnd_limited(sk))
return;
- if (tcp_in_slow_start(tp))
- tcp_slow_start(tp, acked);
- else
- tcp_cong_avoid_ai(tp, min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT),
- 1);
+ if (tcp_in_slow_start(tp)) {
+ acked = tcp_slow_start(tp, acked);
+ if (!acked)
+ return;
+ }
+ tcp_cong_avoid_ai(tp, min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT),
+ acked);
}
static u32 tcp_scalable_ssthresh(struct sock *sk)
--
1.8.3.1
No code logic has been changed in this patch.
Signed-off-by: Pengcheng Yang <[email protected]>
---
net/ipv4/tcp_veno.c | 44 +++++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index 3b36bb1..857491c 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -153,31 +153,33 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 acked)
veno->diff = (tp->snd_cwnd << V_PARAM_SHIFT) - target_cwnd;
if (tcp_in_slow_start(tp)) {
- /* Slow start. */
+ /* Slow start. */
tcp_slow_start(tp, acked);
+ goto done;
+ }
+
+ /* Congestion avoidance. */
+ if (veno->diff < beta) {
+ /* In the "non-congestive state", increase cwnd
+ * every rtt.
+ */
+ tcp_cong_avoid_ai(tp, tp->snd_cwnd, 1);
} else {
- /* Congestion avoidance. */
- if (veno->diff < beta) {
- /* In the "non-congestive state", increase cwnd
- * every rtt.
- */
- tcp_cong_avoid_ai(tp, tp->snd_cwnd, 1);
- } else {
- /* In the "congestive state", increase cwnd
- * every other rtt.
- */
- if (tp->snd_cwnd_cnt >= tp->snd_cwnd) {
- if (veno->inc &&
- tp->snd_cwnd < tp->snd_cwnd_clamp) {
- tp->snd_cwnd++;
- veno->inc = 0;
- } else
- veno->inc = 1;
- tp->snd_cwnd_cnt = 0;
+ /* In the "congestive state", increase cwnd
+ * every other rtt.
+ */
+ if (tp->snd_cwnd_cnt >= tp->snd_cwnd) {
+ if (veno->inc &&
+ tp->snd_cwnd < tp->snd_cwnd_clamp) {
+ tp->snd_cwnd++;
+ veno->inc = 0;
} else
- tp->snd_cwnd_cnt++;
- }
+ veno->inc = 1;
+ tp->snd_cwnd_cnt = 0;
+ } else
+ tp->snd_cwnd_cnt++;
}
+done:
if (tp->snd_cwnd < 2)
tp->snd_cwnd = 2;
else if (tp->snd_cwnd > tp->snd_cwnd_clamp)
--
1.8.3.1
On Mon, Mar 16, 2020 at 2:36 AM Pengcheng Yang <[email protected]> wrote:
>
> Changes BIC to properly handle stretch ACKs in additive
> increase mode by passing in the count of ACKed packets
> to tcp_cong_avoid_ai().
>
> Signed-off-by: Pengcheng Yang <[email protected]>
> ---
> net/ipv4/tcp_bic.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
> index 645cc30..f5f588b 100644
> --- a/net/ipv4/tcp_bic.c
> +++ b/net/ipv4/tcp_bic.c
> @@ -145,12 +145,13 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked)
> if (!tcp_is_cwnd_limited(sk))
> return;
>
> - if (tcp_in_slow_start(tp))
> - tcp_slow_start(tp, acked);
> - else {
> - bictcp_update(ca, tp->snd_cwnd);
> - tcp_cong_avoid_ai(tp, ca->cnt, 1);
> + if (tcp_in_slow_start(tp)) {
> + acked = tcp_slow_start(tp, acked);
> + if (!acked)
> + return;
> }
> + bictcp_update(ca, tp->snd_cwnd);
> + tcp_cong_avoid_ai(tp, ca->cnt, acked);
> }
>
> /*
> --
Acked-by: Neal Cardwell <[email protected]>
thanks,
neal
On Mon, Mar 16, 2020 at 2:37 AM Pengcheng Yang <[email protected]> wrote:
>
> Change Scalable to properly handle stretch ACKs in additive
> increase mode by passing in the count of ACKed packets to
> tcp_cong_avoid_ai().
>
> In addition, because we are now precisely accounting for
> stretch ACKs, including delayed ACKs, we can now change
> TCP_SCALABLE_AI_CNT to 100.
>
> Signed-off-by: Pengcheng Yang <[email protected]>
> ---
> net/ipv4/tcp_scalable.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv4/tcp_scalable.c b/net/ipv4/tcp_scalable.c
> index 471571e..6cebf41 100644
> --- a/net/ipv4/tcp_scalable.c
> +++ b/net/ipv4/tcp_scalable.c
> @@ -10,10 +10,9 @@
> #include <net/tcp.h>
>
> /* These factors derived from the recommended values in the aer:
> - * .01 and and 7/8. We use 50 instead of 100 to account for
> - * delayed ack.
> + * .01 and and 7/8.
> */
> -#define TCP_SCALABLE_AI_CNT 50U
> +#define TCP_SCALABLE_AI_CNT 100U
> #define TCP_SCALABLE_MD_SCALE 3
>
> static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked)
> @@ -23,11 +22,13 @@ static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked)
> if (!tcp_is_cwnd_limited(sk))
> return;
>
> - if (tcp_in_slow_start(tp))
> - tcp_slow_start(tp, acked);
> - else
> - tcp_cong_avoid_ai(tp, min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT),
> - 1);
> + if (tcp_in_slow_start(tp)) {
> + acked = tcp_slow_start(tp, acked);
> + if (!acked)
> + return;
> + }
> + tcp_cong_avoid_ai(tp, min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT),
> + acked);
> }
>
> static u32 tcp_scalable_ssthresh(struct sock *sk)
> --
Acked-by: Neal Cardwell <[email protected]>
thanks,
neal
On Mon, Mar 16, 2020 at 2:37 AM Pengcheng Yang <[email protected]> wrote:
>
> No code logic has been changed in this patch.
>
> Signed-off-by: Pengcheng Yang <[email protected]>
> ---
> net/ipv4/tcp_veno.c | 44 +++++++++++++++++++++++---------------------
> 1 file changed, 23 insertions(+), 21 deletions(-)
Indeed this looks like a pure refactor.
Acked-by: Neal Cardwell <[email protected]>
thanks,
neal
On Mon, Mar 16, 2020 at 2:36 AM Pengcheng Yang <[email protected]> wrote:
>
> Change Veno to properly handle stretch ACKs in additive
> increase mode by passing in the count of ACKed packets
> to tcp_cong_avoid_ai().
>
> Signed-off-by: Pengcheng Yang <[email protected]>
> ---
> net/ipv4/tcp_veno.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
Acked-by: Neal Cardwell <[email protected]>
thanks,
neal
From: Pengcheng Yang <[email protected]>
Date: Mon, 16 Mar 2020 14:35:06 +0800
> "stretch ACKs" (caused by LRO, GRO, delayed ACKs or middleboxes)
> can cause serious performance shortfalls in common congestion
> control algorithms. Neal Cardwell submitted a series of patches
> starting with commit e73ebb0881ea ("tcp: stretch ACK fixes prep")
> to handle stretch ACKs and fixed stretch ACK bugs in Reno and
> CUBIC congestion control algorithms.
>
> This patch series continues to fix bic, scalable, veno and yeah
> congestion control algorithms to handle stretch ACKs.
>
> Changes in v2:
> - Provide [PATCH 0/N] to describe the modifications of this patch series
Series applied, thanks.