[PATCH RESEND net-next v2 0/5] tcp: fix stretch ACK bugs in congestion control modules

[PATCH RESEND net-next v2 0/5] tcp: fix stretch ACK bugs in congestion control modules

[Pengcheng Yang]

"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

[PATCH RESEND net-next v2 1/5] tcp: fix stretch ACK bugs in BIC

[Pengcheng Yang]

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 <yangpc@wangsu.com>
---
 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

Re: [PATCH RESEND net-next v2 1/5] tcp: fix stretch ACK bugs in BIC

[Neal Cardwell]

On Mon, Mar 16, 2020 at 2:36 AM Pengcheng Yang <yangpc@wangsu.com> 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 <yangpc@wangsu.com>
> ---
>  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 <ncardwell@google.com>

thanks,
neal

[PATCH RESEND net-next v2 2/5] tcp: fix stretch ACK bugs in Scalable

[Pengcheng Yang]

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 <yangpc@wangsu.com>
---
 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

Re: [PATCH RESEND net-next v2 2/5] tcp: fix stretch ACK bugs in Scalable

[Neal Cardwell]

On Mon, Mar 16, 2020 at 2:37 AM Pengcheng Yang <yangpc@wangsu.com> 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 <yangpc@wangsu.com>
> ---
>  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 <ncardwell@google.com>

thanks,
neal

[PATCH RESEND net-next v2 3/5] tcp: stretch ACK fixes in Veno prep

[Pengcheng Yang]

No code logic has been changed in this patch.

Signed-off-by: Pengcheng Yang <yangpc@wangsu.com>
---
 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

Re: [PATCH RESEND net-next v2 3/5] tcp: stretch ACK fixes in Veno prep

[Neal Cardwell]

On Mon, Mar 16, 2020 at 2:37 AM Pengcheng Yang <yangpc@wangsu.com> wrote:
>
> No code logic has been changed in this patch.
>
> Signed-off-by: Pengcheng Yang <yangpc@wangsu.com>
> ---
>  net/ipv4/tcp_veno.c | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)

Indeed this looks like a pure refactor.

Acked-by: Neal Cardwell <ncardwell@google.com>

thanks,
neal

[PATCH RESEND net-next v2 4/5] tcp: fix stretch ACK bugs in Veno

[Pengcheng Yang]

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 <yangpc@wangsu.com>
---
 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

Re: [PATCH RESEND net-next v2 4/5] tcp: fix stretch ACK bugs in Veno

[Neal Cardwell]

On Mon, Mar 16, 2020 at 2:36 AM Pengcheng Yang <yangpc@wangsu.com> 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 <yangpc@wangsu.com>
> ---
>  net/ipv4/tcp_veno.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Acked-by: Neal Cardwell <ncardwell@google.com>

thanks,
neal

[PATCH RESEND net-next v2 5/5] tcp: fix stretch ACK bugs in Yeah

[Pengcheng Yang]

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 <yangpc@wangsu.com>
---
 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

Re: [PATCH RESEND net-next v2 0/5] tcp: fix stretch ACK bugs in congestion control modules

[David Miller]

From: Pengcheng Yang <yangpc@wangsu.com>
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.