public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: jmaloy@redhat.com
To: netdev@vger.kernel.org, davem@davemloft.net
Cc: kuba@kernel.org, passt-dev@passt.top, jmaloy@redhat.com,
	sbrivio@redhat.com, lvivier@redhat.com, dgibson@redhat.com,
	eric.dumazet@gmail.com, edumazet@google.com
Subject: [net-next 2/2] tcp: correct handling of extreme menory squeeze
Date: Sat,  6 Apr 2024 14:21:07 -0400	[thread overview]
Message-ID: <20240406182107.261472-3-jmaloy@redhat.com> (raw)
In-Reply-To: <20240406182107.261472-1-jmaloy@redhat.com>

From: Jon Maloy <jmaloy@redhat.com>

Testing of the previous commit ("tcp: add support for SO_PEEK_OFF")
in this series along with the pasta protocol splicer revealed a bug in
the way tcp handles window advertising during extreme memory squeeze
situations.

The excerpt of the below logging session shows what is happeing:

[5201<->54494]:     ==== Activating log @ tcp_select_window()/268 ====
[5201<->54494]:     (inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM) --> TRUE
[5201<->54494]:   tcp_select_window(<-) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354, returning 0
[5201<->54494]:   ADVERTISING WINDOW SIZE 0
[5201<->54494]: __tcp_transmit_skb(<-) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354

[5201<->54494]: tcp_recvmsg_locked(->)
[5201<->54494]:   __tcp_cleanup_rbuf(->) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]:     (win_now: 250164, new_win: 262144 >= (2 * win_now): 500328))? --> time_to_ack: 0
[5201<->54494]:     NOT calling tcp_send_ack()
[5201<->54494]:   __tcp_cleanup_rbuf(<-) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]: tcp_recvmsg_locked(<-) returning 131072 bytes, window now: 250164, qlen: 83

[...]

[5201<->54494]: tcp_recvmsg_locked(->)
[5201<->54494]:   __tcp_cleanup_rbuf(->) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]:     (win_now: 250164, new_win: 262144 >= (2 * win_now): 500328))? --> time_to_ack: 0
[5201<->54494]:     NOT calling tcp_send_ack()
[5201<->54494]:   __tcp_cleanup_rbuf(<-) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]: tcp_recvmsg_locked(<-) returning 131072 bytes, window now: 250164, qlen: 1

[5201<->54494]: tcp_recvmsg_locked(->)
[5201<->54494]:   __tcp_cleanup_rbuf(->) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]:     (win_now: 250164, new_win: 262144 >= (2 * win_now): 500328))? --> time_to_ack: 0
[5201<->54494]:     NOT calling tcp_send_ack()
[5201<->54494]:   __tcp_cleanup_rbuf(<-) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]: tcp_recvmsg_locked(<-) returning 57036 bytes, window now: 250164, qlen: 0

[5201<->54494]: tcp_recvmsg_locked(->)
[5201<->54494]:   __tcp_cleanup_rbuf(->) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]:     NOT calling tcp_send_ack()
[5201<->54494]:   __tcp_cleanup_rbuf(<-) tp->rcv_wup: 2812454294, tp->rcv_wnd: 5812224, tp->rcv_nxt 2818016354
[5201<->54494]: tcp_recvmsg_locked(<-) returning -11 bytes, window now: 250164, qlen: 0

We can see that although we are adverising a window size of zero,
tp->rcv_wnd is not updated accordingly. This leads to a discrepancy
between this side's and the peer's view of the current window size.
- The peer thinks the window is zero, and stops sending.
- This side ends up in a cycle where it repeatedly caclulates a new
  window size it finds too small to advertise.

Hence no messages are received, and no acknowledges are sent, and
the situation remains locked even after the last queued receive buffer
has been consumed.

We fix this by setting tp->rcv_wnd to 0 before we return from the
function tcp_select_window() in this particular case.
Further testing shows that the connection recovers neatly from the
squeeze situation, and traffic can continue indefinitely.

Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 net/ipv4/tcp_output.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 9282fafc0e61..57ead8f3c334 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -263,11 +263,15 @@ static u16 tcp_select_window(struct sock *sk)
 	u32 cur_win, new_win;
 
 	/* Make the window 0 if we failed to queue the data because we
-	 * are out of memory. The window is temporary, so we don't store
-	 * it on the socket.
+	 * are out of memory. The window needs to be stored in the socket
+	 * for the connection to recover.
 	 */
-	if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM))
-		return 0;
+	if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM)) {
+		new_win = 0;
+		tp->rcv_wnd = 0;
+		tp->rcv_wup = tp->rcv_nxt;
+		goto out;
+	}
 
 	cur_win = tcp_receive_window(tp);
 	new_win = __tcp_select_window(sk);
@@ -301,7 +305,7 @@ static u16 tcp_select_window(struct sock *sk)
 
 	/* RFC1323 scaling applied */
 	new_win >>= tp->rx_opt.rcv_wscale;
-
+out:
 	/* If we advertise zero window, disable fast path. */
 	if (new_win == 0) {
 		tp->pred_flags = 0;
-- 
@@ -263,11 +263,15 @@ static u16 tcp_select_window(struct sock *sk)
 	u32 cur_win, new_win;
 
 	/* Make the window 0 if we failed to queue the data because we
-	 * are out of memory. The window is temporary, so we don't store
-	 * it on the socket.
+	 * are out of memory. The window needs to be stored in the socket
+	 * for the connection to recover.
 	 */
-	if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM))
-		return 0;
+	if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM)) {
+		new_win = 0;
+		tp->rcv_wnd = 0;
+		tp->rcv_wup = tp->rcv_nxt;
+		goto out;
+	}
 
 	cur_win = tcp_receive_window(tp);
 	new_win = __tcp_select_window(sk);
@@ -301,7 +305,7 @@ static u16 tcp_select_window(struct sock *sk)
 
 	/* RFC1323 scaling applied */
 	new_win >>= tp->rx_opt.rcv_wscale;
-
+out:
 	/* If we advertise zero window, disable fast path. */
 	if (new_win == 0) {
 		tp->pred_flags = 0;
-- 
2.42.0


  parent reply	other threads:[~2024-04-06 18:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-06 18:21 [net-next 0/2] tcp: add support for SO_PEEK_OFF socket option jmaloy
2024-04-06 18:21 ` [net-next 1/2] " jmaloy
2024-04-08  9:46   ` Eric Dumazet
2024-04-06 18:21 ` jmaloy [this message]
2024-04-06 16:37   ` [net-next 2/2] tcp: correct handling of extreme menory squeeze Eric Dumazet
2024-04-07  4:52     ` Jason Xing
2024-04-07  5:51       ` Menglong Dong
2024-04-08 11:01         ` Jon Maloy
2024-04-08  8:03     ` Eric Dumazet
2024-04-08 11:13       ` Jon Maloy
  -- strict thread matches above, loose matches on Subject: below --
2024-04-03 22:58 [net-next 0/2] tcp: add support for SO_PEEK_OFF socket option Jon Maloy
2024-04-03 22:58 ` [net-next 2/2] tcp: correct handling of extreme menory squeeze Jon Maloy
2024-04-05 17:55   ` Stefano Brivio
2024-04-05 19:37     ` Jon Maloy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240406182107.261472-3-jmaloy@redhat.com \
    --to=jmaloy@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dgibson@redhat.com \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=kuba@kernel.org \
    --cc=lvivier@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

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 IMAP folder(s).