Skip to contentSkip to conversation

FAILED: patch "[PATCH] Revert "tcp: change pingpong threshold to 3"" failed to apply to 5.4-stable tree

PATCH In review [email protected] 4 participants 5 messages Jul 30, 2022 – Aug 17, 2022

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 4d8f24eeedc58d5f87b650ddda73c16e8ba56559 Mon Sep 17 00:00:00 2001
From: Wei Wang <[email protected]>
Date: Thu, 21 Jul 2022 20:44:04 +0000
Subject: [PATCH] Revert "tcp: change pingpong threshold to 3"

This reverts commit 4a41f453bedfd5e9cd040bad509d9da49feb3e2c.

This to-be-reverted commit was meant to apply a stricter rule for the stack to enter pingpong mode. However, the condition used to check for interactive session "before(tp->lsndtime, icsk->icsk_ack.lrcvtime)" is jiffy based and might be too coarse, which delays the stack entering pingpong mode.
We revert this patch so that we no longer use the above condition to determine interactive session, and also reduce pingpong threshold to 1.

include/net/inet_connection_sock.h +1 -9
index 85cd695e7fd1..ee88f0f1350f 100644
@@ -321,7 +321,7 @@ void inet_csk_update_fastreuse(struct inet_bind_bucket *tb,
 
 struct dst_entry *inet_csk_update_pmtu(struct sock *sk, u32 mtu);
 
-#define TCP_PINGPONG_THRESH	3
+#define TCP_PINGPONG_THRESH	1
 
 static inline void inet_csk_enter_pingpong_mode(struct sock *sk)
 {
@@ -338,14 +338,6 @@ static inline bool inet_csk_in_pingpong_mode(struct sock *sk)
 	return inet_csk(sk)->icsk_ack.pingpong >= TCP_PINGPONG_THRESH;
 }
 
-static inline void inet_csk_inc_pingpong_cnt(struct sock *sk)
-{
-	struct inet_connection_sock *icsk = inet_csk(sk);
-
-	if (icsk->icsk_ack.pingpong < U8_MAX)
-		icsk->icsk_ack.pingpong++;
-}
-
 static inline bool inet_csk_has_ulp(struct sock *sk)
 {
 	return inet_sk(sk)->is_icsk && !!inet_csk(sk)->icsk_ulp_ops;
net/ipv4/tcp_output.c +6 -9
index cf6713c9567e..2efe41c84ee8 100644
@@ -167,16 +167,13 @@ static void tcp_event_data_sent(struct tcp_sock *tp,
 	if (tcp_packets_in_flight(tp) == 0)
 		tcp_ca_event(sk, CA_EVENT_TX_START);
 
-	/* If this is the first data packet sent in response to the
-	 * previous received data,
-	 * and it is a reply for ato after last received packet,
-	 * increase pingpong count.
-	 */
-	if (before(tp->lsndtime, icsk->icsk_ack.lrcvtime) &&
-	    (u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
-		inet_csk_inc_pingpong_cnt(sk);
-
 	tp->lsndtime = now;
+
+	/* If it is a reply for ato after last received
+	 * packet, enter pingpong mode.
+	 */
+	if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
+		inet_csk_enter_pingpong_mode(sk);
 }
 
 /* Account for an ACK we sent. */
#1 lore raw

PATCH 1/1 Revert "tcp: change pingpong threshold to 3"

From: Wei Wang <[email protected]>

commit 4d8f24eeedc58d5f87b650ddda73c16e8ba56559 upstream.

This reverts commit 4a41f453bedfd5e9cd040bad509d9da49feb3e2c.

This to-be-reverted commit was meant to apply a stricter rule for the stack to enter pingpong mode. However, the condition used to check for interactive session "before(tp->lsndtime, icsk->icsk_ack.lrcvtime)" is jiffy based and might be too coarse, which delays the stack entering pingpong mode.
We revert this patch so that we no longer use the above condition to determine interactive session, and also reduce pingpong threshold to 1.

---
 include/net/inet_connection_sock.h | 10 +---------
 net/ipv4/tcp_output.c              | 15 ++++++---------
 2 files changed, 7 insertions(+), 18 deletions(-)
include/net/inet_connection_sock.h +1 -9
index 13792c0ef46e..2e5f2c8b0663 100644
@@ -318,7 +318,7 @@ void inet_csk_update_fastreuse(struct inet_bind_bucket *tb,
 
 struct dst_entry *inet_csk_update_pmtu(struct sock *sk, u32 mtu);
 
-#define TCP_PINGPONG_THRESH	3
+#define TCP_PINGPONG_THRESH	1
 
 static inline void inet_csk_enter_pingpong_mode(struct sock *sk)
 {
@@ -334,12 +334,4 @@ static inline bool inet_csk_in_pingpong_mode(struct sock *sk)
 {
 	return inet_csk(sk)->icsk_ack.pingpong >= TCP_PINGPONG_THRESH;
 }
-
-static inline void inet_csk_inc_pingpong_cnt(struct sock *sk)
-{
-	struct inet_connection_sock *icsk = inet_csk(sk);
-
-	if (icsk->icsk_ack.pingpong < U8_MAX)
-		icsk->icsk_ack.pingpong++;
-}
 #endif /* _INET_CONNECTION_SOCK_H */
net/ipv4/tcp_output.c +6 -9
index ef749a47768a..7f9004ae44a4 100644
@@ -166,16 +166,13 @@ static void tcp_event_data_sent(struct tcp_sock *tp,
 	if (tcp_packets_in_flight(tp) == 0)
 		tcp_ca_event(sk, CA_EVENT_TX_START);
 
-	/* If this is the first data packet sent in response to the
-	 * previous received data,
-	 * and it is a reply for ato after last received packet,
-	 * increase pingpong count.
-	 */
-	if (before(tp->lsndtime, icsk->icsk_ack.lrcvtime) &&
-	    (u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
-		inet_csk_inc_pingpong_cnt(sk);
-
 	tp->lsndtime = now;
+
+	/* If it is a reply for ato after last received
+	 * packet, enter pingpong mode.
+	 */
+	if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
+		inet_csk_enter_pingpong_mode(sk);
 }
 
 /* Account for an ACK we sent. */
Signature · 1 line
2.37.1
maintainer
#2 lore raw

Re: [PATCH stable 5.4] Revert "tcp: change pingpong threshold to 3"

Quoted 14 lines · On Mon, 15 Aug 2022 20:51:29 +0000 Kuniyuki Iwashima wrote:

From: Wei Wang <[email protected]>

commit 4d8f24eeedc58d5f87b650ddda73c16e8ba56559 upstream.

This reverts commit 4a41f453bedfd5e9cd040bad509d9da49feb3e2c.

This to-be-reverted commit was meant to apply a stricter rule for the stack to enter pingpong mode. However, the condition used to check for interactive session "before(tp->lsndtime, icsk->icsk_ack.lrcvtime)" is jiffy based and might be too coarse, which delays the stack entering pingpong mode.
We revert this patch so that we no longer use the above condition to determine interactive session, and also reduce pingpong threshold to 1.

Quoted 8 lines

Should we wait for the resolution in the original thread first?

https://lore.kernel.org/all/[email protected]/

#3 lore raw

Re: [PATCH stable 5.4] Revert "tcp: change pingpong threshold to 3"

In reply to Kuniyuki Iwashima
Quoted 6 lines · Kuniyuki Iwashima

From: Wei Wang <[email protected]>

commit 4d8f24eeedc58d5f87b650ddda73c16e8ba56559 upstream.

This reverts commit 4a41f453bedfd5e9cd040bad509d9da49feb3e2c.

Quoted 7 lines

This to-be-reverted commit was meant to apply a stricter rule for the stack to enter pingpong mode. However, the condition used to check for interactive session "before(tp->lsndtime, icsk->icsk_ack.lrcvtime)" is jiffy based and might be too coarse, which delays the stack entering pingpong mode.
We revert this patch so that we no longer use the above condition to determine interactive session, and also reduce pingpong threshold to 1.

I would wait a bit before backporting this to stable:
https://lore.kernel.org/all/[email protected]/

Quoted 66 lines
---
  include/net/inet_connection_sock.h | 10 +---------
  net/ipv4/tcp_output.c              | 15 ++++++---------
  2 files changed, 7 insertions(+), 18 deletions(-)
include/net/inet_connection_sock.h +1 -9
index 13792c0ef46e..2e5f2c8b0663 100644
@@ -318,7 +318,7 @@ void inet_csk_update_fastreuse(struct inet_bind_bucket *tb,
 
  struct dst_entry *inet_csk_update_pmtu(struct sock *sk, u32 mtu);
 
-#define TCP_PINGPONG_THRESH	3
+#define TCP_PINGPONG_THRESH	1
 
  static inline void inet_csk_enter_pingpong_mode(struct sock *sk)
  {
@@ -334,12 +334,4 @@ static inline bool inet_csk_in_pingpong_mode(struct sock *sk)
  {
  	return inet_csk(sk)->icsk_ack.pingpong >= TCP_PINGPONG_THRESH;
  }
-
-static inline void inet_csk_inc_pingpong_cnt(struct sock *sk)
-{
-	struct inet_connection_sock *icsk = inet_csk(sk);
-
-	if (icsk->icsk_ack.pingpong < U8_MAX)
-		icsk->icsk_ack.pingpong++;
-}
  #endif /* _INET_CONNECTION_SOCK_H */
net/ipv4/tcp_output.c +6 -9
index ef749a47768a..7f9004ae44a4 100644
@@ -166,16 +166,13 @@ static void tcp_event_data_sent(struct tcp_sock *tp,
  	if (tcp_packets_in_flight(tp) == 0)
  		tcp_ca_event(sk, CA_EVENT_TX_START);
 
-	/* If this is the first data packet sent in response to the
-	 * previous received data,
-	 * and it is a reply for ato after last received packet,
-	 * increase pingpong count.
-	 */
-	if (before(tp->lsndtime, icsk->icsk_ack.lrcvtime) &&
-	    (u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
-		inet_csk_inc_pingpong_cnt(sk);
-
  	tp->lsndtime = now;
+
+	/* If it is a reply for ato after last received
+	 * packet, enter pingpong mode.
+	 */
+	if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
+		inet_csk_enter_pingpong_mode(sk);
  }
 
  /* Account for an ACK we sent. */
Signature · 2 lines
js
suse labs
author maintainer
#4 lore raw

Re: [PATCH stable 5.4] Revert "tcp: change pingpong threshold to 3"

Quoted 15 lines · Jiri Slaby
Quoted 6 lines · Kuniyuki Iwashima

From: Wei Wang <[email protected]>

commit 4d8f24eeedc58d5f87b650ddda73c16e8ba56559 upstream.

This reverts commit 4a41f453bedfd5e9cd040bad509d9da49feb3e2c.

Quoted 7 lines

This to-be-reverted commit was meant to apply a stricter rule for the stack to enter pingpong mode. However, the condition used to check for interactive session "before(tp->lsndtime, icsk->icsk_ack.lrcvtime)" is jiffy based and might be too coarse, which delays the stack entering pingpong mode.
We revert this patch so that we no longer use the above condition to determine interactive session, and also reduce pingpong threshold to 1.

Quoted 2 lines

I would wait a bit before backporting this to stable:
https://lore.kernel.org/all/[email protected]/

Looks like all is good, so I'll leave this be for now.

thanks,

greg k-h