netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN
@ 2018-05-21 22:08 Eric Dumazet
  2018-05-21 22:08 ` [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode Eric Dumazet
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Eric Dumazet @ 2018-05-21 22:08 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
	Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet

Small patch series changing TCP behavior vs quickack and ECN

First patch is a refactoring, adding parameter to tcp_incr_quickack()
and tcp_enter_quickack_mode() helpers.

Second patch implements the change, lowering number of ACK packets
sent after an ECN event.

Eric Dumazet (2):
  tcp: add max_quickacks param to tcp_incr_quickack and
    tcp_enter_quickack_mode
  tcp: do not aggressively quick ack after ECN events

 net/ipv4/tcp_input.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

-- 
2.17.0.441.gb46fe60e1d-goog

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode
  2018-05-21 22:08 [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN Eric Dumazet
@ 2018-05-21 22:08 ` Eric Dumazet
  2018-05-22 14:03   ` Soheil Hassas Yeganeh
                     ` (2 more replies)
  2018-05-21 22:08 ` [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events Eric Dumazet
  2018-05-22 19:43 ` [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN David Miller
  2 siblings, 3 replies; 11+ messages in thread
From: Eric Dumazet @ 2018-05-21 22:08 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
	Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet

We want to add finer control of the number of ACK packets sent after
ECN events.

This patch is not changing current behavior, it only enables following
change.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_input.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index aebb29ab2fdf2ceaa182cd11928f145a886149ff..2e970e9f4e09d966b703af2d14d521a4328eba7e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -203,21 +203,23 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb)
 	}
 }
 
-static void tcp_incr_quickack(struct sock *sk)
+static void tcp_incr_quickack(struct sock *sk, unsigned int max_quickacks)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	unsigned int quickacks = tcp_sk(sk)->rcv_wnd / (2 * icsk->icsk_ack.rcv_mss);
 
 	if (quickacks == 0)
 		quickacks = 2;
+	quickacks = min(quickacks, max_quickacks);
 	if (quickacks > icsk->icsk_ack.quick)
-		icsk->icsk_ack.quick = min(quickacks, TCP_MAX_QUICKACKS);
+		icsk->icsk_ack.quick = quickacks;
 }
 
-static void tcp_enter_quickack_mode(struct sock *sk)
+static void tcp_enter_quickack_mode(struct sock *sk, unsigned int max_quickacks)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
-	tcp_incr_quickack(sk);
+
+	tcp_incr_quickack(sk, max_quickacks);
 	icsk->icsk_ack.pingpong = 0;
 	icsk->icsk_ack.ato = TCP_ATO_MIN;
 }
@@ -261,7 +263,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
 		 * it is probably a retransmit.
 		 */
 		if (tp->ecn_flags & TCP_ECN_SEEN)
-			tcp_enter_quickack_mode((struct sock *)tp);
+			tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
 		break;
 	case INET_ECN_CE:
 		if (tcp_ca_needs_ecn((struct sock *)tp))
@@ -269,7 +271,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
 
 		if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
 			/* Better not delay acks, sender can have a very low cwnd */
-			tcp_enter_quickack_mode((struct sock *)tp);
+			tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
 			tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
 		}
 		tp->ecn_flags |= TCP_ECN_SEEN;
@@ -686,7 +688,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
 		/* The _first_ data packet received, initialize
 		 * delayed ACK engine.
 		 */
-		tcp_incr_quickack(sk);
+		tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
 		icsk->icsk_ack.ato = TCP_ATO_MIN;
 	} else {
 		int m = now - icsk->icsk_ack.lrcvtime;
@@ -702,7 +704,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
 			/* Too long gap. Apparently sender failed to
 			 * restart window, so that we send ACKs quickly.
 			 */
-			tcp_incr_quickack(sk);
+			tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
 			sk_mem_reclaim(sk);
 		}
 	}
@@ -4179,7 +4181,7 @@ static void tcp_send_dupack(struct sock *sk, const struct sk_buff *skb)
 	if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
 	    before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
-		tcp_enter_quickack_mode(sk);
+		tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
 
 		if (tcp_is_sack(tp) && sock_net(sk)->ipv4.sysctl_tcp_dsack) {
 			u32 end_seq = TCP_SKB_CB(skb)->end_seq;
@@ -4706,7 +4708,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
 		tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
 
 out_of_window:
-		tcp_enter_quickack_mode(sk);
+		tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
 		inet_csk_schedule_ack(sk);
 drop:
 		tcp_drop(sk, skb);
@@ -5790,7 +5792,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 			 * to stand against the temptation 8)     --ANK
 			 */
 			inet_csk_schedule_ack(sk);
-			tcp_enter_quickack_mode(sk);
+			tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
 			inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
 						  TCP_DELACK_MAX, TCP_RTO_MAX);
 
-- 
2.17.0.441.gb46fe60e1d-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events
  2018-05-21 22:08 [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN Eric Dumazet
  2018-05-21 22:08 ` [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode Eric Dumazet
@ 2018-05-21 22:08 ` Eric Dumazet
  2018-05-22 14:04   ` Soheil Hassas Yeganeh
                     ` (2 more replies)
  2018-05-22 19:43 ` [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN David Miller
  2 siblings, 3 replies; 11+ messages in thread
From: Eric Dumazet @ 2018-05-21 22:08 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
	Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet

ECN signals currently forces TCP to enter quickack mode for
up to 16 (TCP_MAX_QUICKACKS) following incoming packets.

We believe this is not needed, and only sending one immediate ack
for the current packet should be enough.

This should reduce the extra load noticed in DCTCP environments,
after congestion events.

This is part 2 of our effort to reduce pure ACK packets.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_input.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2e970e9f4e09d966b703af2d14d521a4328eba7e..1191cac72109f2f7e2b688ddbc1d404151d274d6 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -263,7 +263,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
 		 * it is probably a retransmit.
 		 */
 		if (tp->ecn_flags & TCP_ECN_SEEN)
-			tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
+			tcp_enter_quickack_mode((struct sock *)tp, 1);
 		break;
 	case INET_ECN_CE:
 		if (tcp_ca_needs_ecn((struct sock *)tp))
@@ -271,7 +271,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
 
 		if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
 			/* Better not delay acks, sender can have a very low cwnd */
-			tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
+			tcp_enter_quickack_mode((struct sock *)tp, 1);
 			tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
 		}
 		tp->ecn_flags |= TCP_ECN_SEEN;
-- 
2.17.0.441.gb46fe60e1d-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode
  2018-05-21 22:08 ` [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode Eric Dumazet
@ 2018-05-22 14:03   ` Soheil Hassas Yeganeh
  2018-05-22 17:27   ` Neal Cardwell
  2018-05-23  0:30   ` kbuild test robot
  2 siblings, 0 replies; 11+ messages in thread
From: Soheil Hassas Yeganeh @ 2018-05-22 14:03 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Van Jacobson, Neal Cardwell,
	Yuchung Cheng, Eric Dumazet

On Mon, May 21, 2018 at 6:08 PM, Eric Dumazet <edumazet@google.com> wrote:
> We want to add finer control of the number of ACK packets sent after
> ECN events.
>
> This patch is not changing current behavior, it only enables following
> change.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

> ---
>  net/ipv4/tcp_input.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index aebb29ab2fdf2ceaa182cd11928f145a886149ff..2e970e9f4e09d966b703af2d14d521a4328eba7e 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -203,21 +203,23 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb)
>         }
>  }
>
> -static void tcp_incr_quickack(struct sock *sk)
> +static void tcp_incr_quickack(struct sock *sk, unsigned int max_quickacks)
>  {
>         struct inet_connection_sock *icsk = inet_csk(sk);
>         unsigned int quickacks = tcp_sk(sk)->rcv_wnd / (2 * icsk->icsk_ack.rcv_mss);
>
>         if (quickacks == 0)
>                 quickacks = 2;
> +       quickacks = min(quickacks, max_quickacks);
>         if (quickacks > icsk->icsk_ack.quick)
> -               icsk->icsk_ack.quick = min(quickacks, TCP_MAX_QUICKACKS);
> +               icsk->icsk_ack.quick = quickacks;
>  }
>
> -static void tcp_enter_quickack_mode(struct sock *sk)
> +static void tcp_enter_quickack_mode(struct sock *sk, unsigned int max_quickacks)
>  {
>         struct inet_connection_sock *icsk = inet_csk(sk);
> -       tcp_incr_quickack(sk);
> +
> +       tcp_incr_quickack(sk, max_quickacks);
>         icsk->icsk_ack.pingpong = 0;
>         icsk->icsk_ack.ato = TCP_ATO_MIN;
>  }
> @@ -261,7 +263,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
>                  * it is probably a retransmit.
>                  */
>                 if (tp->ecn_flags & TCP_ECN_SEEN)
> -                       tcp_enter_quickack_mode((struct sock *)tp);
> +                       tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
>                 break;
>         case INET_ECN_CE:
>                 if (tcp_ca_needs_ecn((struct sock *)tp))
> @@ -269,7 +271,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
>
>                 if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
>                         /* Better not delay acks, sender can have a very low cwnd */
> -                       tcp_enter_quickack_mode((struct sock *)tp);
> +                       tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
>                         tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
>                 }
>                 tp->ecn_flags |= TCP_ECN_SEEN;
> @@ -686,7 +688,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
>                 /* The _first_ data packet received, initialize
>                  * delayed ACK engine.
>                  */
> -               tcp_incr_quickack(sk);
> +               tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
>                 icsk->icsk_ack.ato = TCP_ATO_MIN;
>         } else {
>                 int m = now - icsk->icsk_ack.lrcvtime;
> @@ -702,7 +704,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
>                         /* Too long gap. Apparently sender failed to
>                          * restart window, so that we send ACKs quickly.
>                          */
> -                       tcp_incr_quickack(sk);
> +                       tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
>                         sk_mem_reclaim(sk);
>                 }
>         }
> @@ -4179,7 +4181,7 @@ static void tcp_send_dupack(struct sock *sk, const struct sk_buff *skb)
>         if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
>             before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
>                 NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
> -               tcp_enter_quickack_mode(sk);
> +               tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
>
>                 if (tcp_is_sack(tp) && sock_net(sk)->ipv4.sysctl_tcp_dsack) {
>                         u32 end_seq = TCP_SKB_CB(skb)->end_seq;
> @@ -4706,7 +4708,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
>                 tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
>
>  out_of_window:
> -               tcp_enter_quickack_mode(sk);
> +               tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
>                 inet_csk_schedule_ack(sk);
>  drop:
>                 tcp_drop(sk, skb);
> @@ -5790,7 +5792,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>                          * to stand against the temptation 8)     --ANK
>                          */
>                         inet_csk_schedule_ack(sk);
> -                       tcp_enter_quickack_mode(sk);
> +                       tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
>                         inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
>                                                   TCP_DELACK_MAX, TCP_RTO_MAX);
>
> --
> 2.17.0.441.gb46fe60e1d-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events
  2018-05-21 22:08 ` [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events Eric Dumazet
@ 2018-05-22 14:04   ` Soheil Hassas Yeganeh
  2018-05-22 16:54   ` Yuchung Cheng
  2018-05-22 17:36   ` Neal Cardwell
  2 siblings, 0 replies; 11+ messages in thread
From: Soheil Hassas Yeganeh @ 2018-05-22 14:04 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Van Jacobson, Neal Cardwell,
	Yuchung Cheng, Eric Dumazet

On Mon, May 21, 2018 at 6:08 PM, Eric Dumazet <edumazet@google.com> wrote:
> ECN signals currently forces TCP to enter quickack mode for
> up to 16 (TCP_MAX_QUICKACKS) following incoming packets.
>
> We believe this is not needed, and only sending one immediate ack
> for the current packet should be enough.
>
> This should reduce the extra load noticed in DCTCP environments,
> after congestion events.
>
> This is part 2 of our effort to reduce pure ACK packets.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Thanks for the patch!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events
  2018-05-21 22:08 ` [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events Eric Dumazet
  2018-05-22 14:04   ` Soheil Hassas Yeganeh
@ 2018-05-22 16:54   ` Yuchung Cheng
  2018-05-22 17:36   ` Neal Cardwell
  2 siblings, 0 replies; 11+ messages in thread
From: Yuchung Cheng @ 2018-05-22 16:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Van Jacobson, Neal Cardwell,
	Soheil Hassas Yeganeh, Eric Dumazet

On Mon, May 21, 2018 at 3:08 PM, Eric Dumazet <edumazet@google.com> wrote:
> ECN signals currently forces TCP to enter quickack mode for
> up to 16 (TCP_MAX_QUICKACKS) following incoming packets.
>
> We believe this is not needed, and only sending one immediate ack
> for the current packet should be enough.
>
> This should reduce the extra load noticed in DCTCP environments,
> after congestion events.
>
> This is part 2 of our effort to reduce pure ACK packets.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
Acked-by: Yuchung Cheng <ycheng@google.com>

Thanks for this patch. I am still wondering how much does the "funny
extension" help. but this patch definitely reduce the amount of
unnecessary immediate ACKs on ECN.

>  net/ipv4/tcp_input.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 2e970e9f4e09d966b703af2d14d521a4328eba7e..1191cac72109f2f7e2b688ddbc1d404151d274d6 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -263,7 +263,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
>                  * it is probably a retransmit.
>                  */
>                 if (tp->ecn_flags & TCP_ECN_SEEN)
> -                       tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
> +                       tcp_enter_quickack_mode((struct sock *)tp, 1);
>                 break;
>         case INET_ECN_CE:
>                 if (tcp_ca_needs_ecn((struct sock *)tp))
> @@ -271,7 +271,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const struct sk_buff *skb)
>
>                 if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
>                         /* Better not delay acks, sender can have a very low cwnd */
> -                       tcp_enter_quickack_mode((struct sock *)tp, TCP_MAX_QUICKACKS);
> +                       tcp_enter_quickack_mode((struct sock *)tp, 1);
>                         tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
>                 }
>                 tp->ecn_flags |= TCP_ECN_SEEN;
> --
> 2.17.0.441.gb46fe60e1d-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode
  2018-05-21 22:08 ` [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode Eric Dumazet
  2018-05-22 14:03   ` Soheil Hassas Yeganeh
@ 2018-05-22 17:27   ` Neal Cardwell
  2018-05-23  0:30   ` kbuild test robot
  2 siblings, 0 replies; 11+ messages in thread
From: Neal Cardwell @ 2018-05-22 17:27 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Netdev, Van Jacobson, Yuchung Cheng,
	Soheil Hassas Yeganeh, Eric Dumazet

On Mon, May 21, 2018 at 6:09 PM Eric Dumazet <edumazet@google.com> wrote:

> We want to add finer control of the number of ACK packets sent after
> ECN events.

> This patch is not changing current behavior, it only enables following
> change.

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---

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

Thanks!

neal

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events
  2018-05-21 22:08 ` [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events Eric Dumazet
  2018-05-22 14:04   ` Soheil Hassas Yeganeh
  2018-05-22 16:54   ` Yuchung Cheng
@ 2018-05-22 17:36   ` Neal Cardwell
  2 siblings, 0 replies; 11+ messages in thread
From: Neal Cardwell @ 2018-05-22 17:36 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Netdev, Van Jacobson, Yuchung Cheng,
	Soheil Hassas Yeganeh, Eric Dumazet

On Mon, May 21, 2018 at 6:09 PM Eric Dumazet <edumazet@google.com> wrote:

> ECN signals currently forces TCP to enter quickack mode for
> up to 16 (TCP_MAX_QUICKACKS) following incoming packets.

> We believe this is not needed, and only sending one immediate ack
> for the current packet should be enough.

> This should reduce the extra load noticed in DCTCP environments,
> after congestion events.

> This is part 2 of our effort to reduce pure ACK packets.

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---

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

Thanks!

neal

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN
  2018-05-21 22:08 [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN Eric Dumazet
  2018-05-21 22:08 ` [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode Eric Dumazet
  2018-05-21 22:08 ` [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events Eric Dumazet
@ 2018-05-22 19:43 ` David Miller
  2 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2018-05-22 19:43 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, vanj, ncardwell, ycheng, soheil, eric.dumazet

From: Eric Dumazet <edumazet@google.com>
Date: Mon, 21 May 2018 15:08:55 -0700

> Small patch series changing TCP behavior vs quickack and ECN
> 
> First patch is a refactoring, adding parameter to tcp_incr_quickack()
> and tcp_enter_quickack_mode() helpers.
> 
> Second patch implements the change, lowering number of ACK packets
> sent after an ECN event.

Series applied, thanks Eric.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode
  2018-05-21 22:08 ` [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode Eric Dumazet
  2018-05-22 14:03   ` Soheil Hassas Yeganeh
  2018-05-22 17:27   ` Neal Cardwell
@ 2018-05-23  0:30   ` kbuild test robot
  2018-05-23  1:37     ` Neal Cardwell
  2 siblings, 1 reply; 11+ messages in thread
From: kbuild test robot @ 2018-05-23  0:30 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: kbuild-all, David S . Miller, netdev, Van Jacobson,
	Neal Cardwell, Yuchung Cheng, Soheil Hassas Yeganeh,
	Eric Dumazet, Eric Dumazet

[-- Attachment #1: Type: text/plain, Size: 9440 bytes --]

Hi Eric,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v4.17-rc6 next-20180517]
[cannot apply to net-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/tcp-add-max_quickacks-param-to-tcp_incr_quickack-and-tcp_enter_quickack_mode/20180523-075103
config: i386-randconfig-x012-201820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net//ipv4/tcp_input.c: In function 'tcp_data_queue':
>> net//ipv4/tcp_input.c:4656:2: error: too few arguments to function 'tcp_enter_quickack_mode'
     tcp_enter_quickack_mode(sk);
     ^~~~~~~~~~~~~~~~~~~~~~~
   net//ipv4/tcp_input.c:199:13: note: declared here
    static void tcp_enter_quickack_mode(struct sock *sk, unsigned int max_quickacks)
                ^~~~~~~~~~~~~~~~~~~~~~~

vim +/tcp_enter_quickack_mode +4656 net//ipv4/tcp_input.c

292e8d8c8 Pavel Emelyanov          2012-05-10  4577  
^1da177e4 Linus Torvalds           2005-04-16  4578  static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
^1da177e4 Linus Torvalds           2005-04-16  4579  {
^1da177e4 Linus Torvalds           2005-04-16  4580  	struct tcp_sock *tp = tcp_sk(sk);
5357f0bd4 Eric Dumazet             2017-08-01  4581  	bool fragstolen;
5357f0bd4 Eric Dumazet             2017-08-01  4582  	int eaten;
^1da177e4 Linus Torvalds           2005-04-16  4583  
532182cd6 Eric Dumazet             2016-04-01  4584  	if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
532182cd6 Eric Dumazet             2016-04-01  4585  		__kfree_skb(skb);
532182cd6 Eric Dumazet             2016-04-01  4586  		return;
532182cd6 Eric Dumazet             2016-04-01  4587  	}
f84af32cb Eric Dumazet             2010-04-28  4588  	skb_dst_drop(skb);
155c6e1ad Peter Pan(潘卫平         2014-09-24  4589) 	__skb_pull(skb, tcp_hdr(skb)->doff * 4);
^1da177e4 Linus Torvalds           2005-04-16  4590  
735d38311 Florian Westphal         2014-09-29  4591  	tcp_ecn_accept_cwr(tp, skb);
^1da177e4 Linus Torvalds           2005-04-16  4592  
^1da177e4 Linus Torvalds           2005-04-16  4593  	tp->rx_opt.dsack = 0;
^1da177e4 Linus Torvalds           2005-04-16  4594  
^1da177e4 Linus Torvalds           2005-04-16  4595  	/*  Queue data for delivery to the user.
^1da177e4 Linus Torvalds           2005-04-16  4596  	 *  Packets in sequence go to the receive queue.
^1da177e4 Linus Torvalds           2005-04-16  4597  	 *  Out of sequence packets to the out_of_order_queue.
^1da177e4 Linus Torvalds           2005-04-16  4598  	 */
^1da177e4 Linus Torvalds           2005-04-16  4599  	if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
^1da177e4 Linus Torvalds           2005-04-16  4600  		if (tcp_receive_window(tp) == 0)
^1da177e4 Linus Torvalds           2005-04-16  4601  			goto out_of_window;
^1da177e4 Linus Torvalds           2005-04-16  4602  
^1da177e4 Linus Torvalds           2005-04-16  4603  		/* Ok. In sequence. In window. */
^1da177e4 Linus Torvalds           2005-04-16  4604  queue_and_out:
76dfa6082 Eric Dumazet             2015-05-15  4605  		if (skb_queue_len(&sk->sk_receive_queue) == 0)
76dfa6082 Eric Dumazet             2015-05-15  4606  			sk_forced_mem_schedule(sk, skb->truesize);
76dfa6082 Eric Dumazet             2015-05-15  4607  		else if (tcp_try_rmem_schedule(sk, skb, skb->truesize))
^1da177e4 Linus Torvalds           2005-04-16  4608  			goto drop;
5357f0bd4 Eric Dumazet             2017-08-01  4609  
b081f85c2 Eric Dumazet             2012-05-02  4610  		eaten = tcp_queue_rcv(sk, skb, 0, &fragstolen);
bdd1f9eda Eric Dumazet             2015-04-28  4611  		tcp_rcv_nxt_update(tp, TCP_SKB_CB(skb)->end_seq);
^1da177e4 Linus Torvalds           2005-04-16  4612  		if (skb->len)
9e412ba76 Ilpo Järvinen            2007-04-20  4613  			tcp_event_data_recv(sk, skb);
155c6e1ad Peter Pan(潘卫平         2014-09-24  4614) 		if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
20c4cb792 Eric Dumazet             2011-10-20  4615  			tcp_fin(sk);
^1da177e4 Linus Torvalds           2005-04-16  4616  
9f5afeae5 Yaogong Wang             2016-09-07  4617  		if (!RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
^1da177e4 Linus Torvalds           2005-04-16  4618  			tcp_ofo_queue(sk);
^1da177e4 Linus Torvalds           2005-04-16  4619  
^1da177e4 Linus Torvalds           2005-04-16  4620  			/* RFC2581. 4.2. SHOULD send immediate ACK, when
^1da177e4 Linus Torvalds           2005-04-16  4621  			 * gap in queue is filled.
^1da177e4 Linus Torvalds           2005-04-16  4622  			 */
9f5afeae5 Yaogong Wang             2016-09-07  4623  			if (RB_EMPTY_ROOT(&tp->out_of_order_queue))
463c84b97 Arnaldo Carvalho de Melo 2005-08-09  4624  				inet_csk(sk)->icsk_ack.pingpong = 0;
^1da177e4 Linus Torvalds           2005-04-16  4625  		}
^1da177e4 Linus Torvalds           2005-04-16  4626  
^1da177e4 Linus Torvalds           2005-04-16  4627  		if (tp->rx_opt.num_sacks)
^1da177e4 Linus Torvalds           2005-04-16  4628  			tcp_sack_remove(tp);
^1da177e4 Linus Torvalds           2005-04-16  4629  
31770e34e Florian Westphal         2017-08-30  4630  		tcp_fast_path_check(sk);
31770e34e Florian Westphal         2017-08-30  4631  
923dd347b Eric Dumazet             2012-05-02  4632  		if (eaten > 0)
923dd347b Eric Dumazet             2012-05-02  4633  			kfree_skb_partial(skb, fragstolen);
1d57f1953 Eric Dumazet             2012-09-17  4634  		if (!sock_flag(sk, SOCK_DEAD))
676d23690 David S. Miller          2014-04-11  4635  			sk->sk_data_ready(sk);
^1da177e4 Linus Torvalds           2005-04-16  4636  		return;
^1da177e4 Linus Torvalds           2005-04-16  4637  	}
^1da177e4 Linus Torvalds           2005-04-16  4638  
^1da177e4 Linus Torvalds           2005-04-16  4639  	if (!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt)) {
^1da177e4 Linus Torvalds           2005-04-16  4640  		/* A retransmit, 2nd most common case.  Force an immediate ack. */
c10d9310e Eric Dumazet             2016-04-29  4641  		NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
1ed834655 Pavel Emelyanov          2008-07-16  4642  		tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
^1da177e4 Linus Torvalds           2005-04-16  4643  
^1da177e4 Linus Torvalds           2005-04-16  4644  out_of_window:
265e9de2c Eric Dumazet             2018-05-21  4645  		tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
463c84b97 Arnaldo Carvalho de Melo 2005-08-09  4646  		inet_csk_schedule_ack(sk);
^1da177e4 Linus Torvalds           2005-04-16  4647  drop:
532182cd6 Eric Dumazet             2016-04-01  4648  		tcp_drop(sk, skb);
^1da177e4 Linus Torvalds           2005-04-16  4649  		return;
^1da177e4 Linus Torvalds           2005-04-16  4650  	}
^1da177e4 Linus Torvalds           2005-04-16  4651  
^1da177e4 Linus Torvalds           2005-04-16  4652  	/* Out of window. F.e. zero window probe. */
^1da177e4 Linus Torvalds           2005-04-16  4653  	if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
^1da177e4 Linus Torvalds           2005-04-16  4654  		goto out_of_window;
^1da177e4 Linus Torvalds           2005-04-16  4655  
463c84b97 Arnaldo Carvalho de Melo 2005-08-09 @4656  	tcp_enter_quickack_mode(sk);
^1da177e4 Linus Torvalds           2005-04-16  4657  
^1da177e4 Linus Torvalds           2005-04-16  4658  	if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
^1da177e4 Linus Torvalds           2005-04-16  4659  		/* Partial packet, seq < rcv_next < end_seq */
^1da177e4 Linus Torvalds           2005-04-16  4660  		SOCK_DEBUG(sk, "partial packet: rcv_next %X seq %X - %X\n",
^1da177e4 Linus Torvalds           2005-04-16  4661  			   tp->rcv_nxt, TCP_SKB_CB(skb)->seq,
^1da177e4 Linus Torvalds           2005-04-16  4662  			   TCP_SKB_CB(skb)->end_seq);
^1da177e4 Linus Torvalds           2005-04-16  4663  
1ed834655 Pavel Emelyanov          2008-07-16  4664  		tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, tp->rcv_nxt);
^1da177e4 Linus Torvalds           2005-04-16  4665  
^1da177e4 Linus Torvalds           2005-04-16  4666  		/* If window is closed, drop tail of packet. But after
^1da177e4 Linus Torvalds           2005-04-16  4667  		 * remembering D-SACK for its head made in previous line.
^1da177e4 Linus Torvalds           2005-04-16  4668  		 */
^1da177e4 Linus Torvalds           2005-04-16  4669  		if (!tcp_receive_window(tp))
^1da177e4 Linus Torvalds           2005-04-16  4670  			goto out_of_window;
^1da177e4 Linus Torvalds           2005-04-16  4671  		goto queue_and_out;
^1da177e4 Linus Torvalds           2005-04-16  4672  	}
^1da177e4 Linus Torvalds           2005-04-16  4673  
e86b29196 Eric Dumazet             2012-03-18  4674  	tcp_data_queue_ofo(sk, skb);
^1da177e4 Linus Torvalds           2005-04-16  4675  }
^1da177e4 Linus Torvalds           2005-04-16  4676  

:::::: The code at line 4656 was first introduced by commit
:::::: 463c84b97f24010a67cd871746d6a7e4c925a5f9 [NET]: Introduce inet_connection_sock

:::::: TO: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
:::::: CC: David S. Miller <davem@sunset.davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33476 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode
  2018-05-23  0:30   ` kbuild test robot
@ 2018-05-23  1:37     ` Neal Cardwell
  0 siblings, 0 replies; 11+ messages in thread
From: Neal Cardwell @ 2018-05-23  1:37 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Eric Dumazet, kbuild-all, David Miller, Netdev, Van Jacobson,
	Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet

On Tue, May 22, 2018 at 8:31 PM kbuild test robot <lkp@intel.com> wrote:

> Hi Eric,

> Thank you for the patch! Yet something to improve:

> [auto build test ERROR on net/master]
> [also build test ERROR on v4.17-rc6 next-20180517]
> [cannot apply to net-next/master]
> [if your patch is applied to the wrong git tree, please drop us a note to
help improve the system]

> url:
https://github.com/0day-ci/linux/commits/Eric-Dumazet/tcp-add-max_quickacks-param-to-tcp_incr_quickack-and-tcp_enter_quickack_mode/20180523-075103
> config: i386-randconfig-x012-201820 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=i386

> All errors (new ones prefixed by >>):

>     net//ipv4/tcp_input.c: In function 'tcp_data_queue':
> >> net//ipv4/tcp_input.c:4656:2: error: too few arguments to function
'tcp_enter_quickack_mode'
>       tcp_enter_quickack_mode(sk);
>       ^~~~~~~~~~~~~~~~~~~~~~~
>     net//ipv4/tcp_input.c:199:13: note: declared here
>      static void tcp_enter_quickack_mode(struct sock *sk, unsigned int
max_quickacks)
>                  ^~~~~~~~~~~~~~~~~~~~~~~
...

For the record, this is an error in the tool, rather than the patch. The
tool seems to be using a stale net-next tree for building this patch.

The compile error is here in line 4656:

> ^1da177e4 Linus Torvalds           2005-04-16  4652     /* Out of window.
F.e. zero window probe. */
> ^1da177e4 Linus Torvalds           2005-04-16  4653     if
(!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
> ^1da177e4 Linus Torvalds           2005-04-16  4654             goto
out_of_window;
> ^1da177e4 Linus Torvalds           2005-04-16  4655
> 463c84b97 Arnaldo Carvalho de Melo 2005-08-09 @4656
tcp_enter_quickack_mode(sk);
> ^1da177e4 Linus Torvalds           2005-04-16  4657
> ^1da177e4 Linus Torvalds           2005-04-16  4658     if
(before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
> ^1da177e4 Linus Torvalds           2005-04-16  4659             /*
Partial packet, seq < rcv_next < end_seq */
...

But that line is not in net-next any more, after Eric's recent net-next
commit:

a3893637e1eb0e ("tcp: do not force quickack when receiving out-of-order
packets")

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/net/ipv4/tcp_input.c?id=a3893637e1eb0ef5eb1bbc52b3a8d2dfa317a35d

That commit removed that line:

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0bf032839548f..f5622b2506651 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4715,8 +4715,6 @@ drop:
         if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt +
tcp_receive_window(tp)))
                 goto out_of_window;

-       tcp_enter_quickack_mode(sk);
-
         if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
                 /* Partial packet, seq < rcv_next < end_seq */
                 SOCK_DEBUG(sk, "partial packet: rcv_next %X seq %X - %X\n",

cheers,
neal

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-05-23  1:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-21 22:08 [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN Eric Dumazet
2018-05-21 22:08 ` [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode Eric Dumazet
2018-05-22 14:03   ` Soheil Hassas Yeganeh
2018-05-22 17:27   ` Neal Cardwell
2018-05-23  0:30   ` kbuild test robot
2018-05-23  1:37     ` Neal Cardwell
2018-05-21 22:08 ` [PATCH net-next 2/2] tcp: do not aggressively quick ack after ECN events Eric Dumazet
2018-05-22 14:04   ` Soheil Hassas Yeganeh
2018-05-22 16:54   ` Yuchung Cheng
2018-05-22 17:36   ` Neal Cardwell
2018-05-22 19:43 ` [PATCH net-next 0/2] tcp: reduce quickack pressure for ECN 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).