[PATCH v2 3/6] tcp: Rewind sequence when guest shrinks window to zero

[Stefano Brivio <sbrivio@redhat.com>]

A window shrunk to zero means by definition that anything else that
might be in flight is now out of window. Restart from the currently
acknowledged sequence.

We need to do that both in tcp_tap_window_update(), where we already
check for zero-window updates, as well as in tcp_data_from_tap(),
because we might get one of those updates in a batch of packets that
also contains a non-zero window update.

Suggested-by: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 tcp.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/tcp.c b/tcp.c
index 1402ca2..11c9c84 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1257,19 +1257,25 @@ static void tcp_get_tap_ws(struct tcp_tap_conn *conn,
 
 /**
  * tcp_tap_window_update() - Process an updated window from tap side
+ * @c:		Execution context
  * @conn:	Connection pointer
  * @wnd:	Window value, host order, unscaled
  */
-static void tcp_tap_window_update(struct tcp_tap_conn *conn, unsigned wnd)
+static void tcp_tap_window_update(const struct ctx *c,
+				  struct tcp_tap_conn *conn, unsigned wnd)
 {
 	wnd = MIN(MAX_WINDOW, wnd << conn->ws_from_tap);
 
 	/* Work-around for bug introduced in peer kernel code, commit
-	 * e2142825c120 ("net: tcp: send zero-window ACK when no memory").
-	 * We don't update if window shrank to zero.
+	 * e2142825c120 ("net: tcp: send zero-window ACK when no memory"): don't
+	 * update the window if it shrank to zero, so that we'll eventually
+	 * retry to send data, but rewind the sequence as that obviously implies
+	 * that no data beyond the updated window will ever be acknowledged.
 	 */
-	if (!wnd && SEQ_LT(conn->seq_ack_from_tap, conn->seq_to_tap))
+	if (!wnd && SEQ_LT(conn->seq_ack_from_tap, conn->seq_to_tap)) {
+		tcp_rewind_seq(c, conn);
 		return;
+	}
 
 	conn->wnd_from_tap = MIN(wnd >> conn->ws_from_tap, USHRT_MAX);
 
@@ -1694,7 +1700,8 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
 			tcp_timer_ctl(c, conn);
 
 			if (p->count == 1) {
-				tcp_tap_window_update(conn, ntohs(th->window));
+				tcp_tap_window_update(c, conn,
+						      ntohs(th->window));
 				return 1;
 			}
 
@@ -1713,6 +1720,15 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
 				       ack_seq == max_ack_seq &&
 				       ntohs(th->window) == max_ack_seq_wnd;
 
+				/* See tcp_tap_window_update() for details. On
+				 * top of that, we also need to check here if a
+				 * zero-window update is contained in a batch of
+				 * packets that includes a non-zero window as
+				 * well.
+				 */
+				if (!ntohs(th->window))
+					tcp_rewind_seq(c, conn);
+
 				max_ack_seq_wnd = ntohs(th->window);
 				max_ack_seq = ack_seq;
 			}
@@ -1772,7 +1788,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
 	if (ack && !tcp_sock_consume(conn, max_ack_seq))
 		tcp_update_seqack_from_tap(c, conn, max_ack_seq);
 
-	tcp_tap_window_update(conn, max_ack_seq_wnd);
+	tcp_tap_window_update(c, conn, max_ack_seq_wnd);
 
 	if (retr) {
 		flow_trace(conn,
@@ -1861,7 +1877,7 @@ static void tcp_conn_from_sock_finish(const struct ctx *c,
 				      const struct tcphdr *th,
 				      const char *opts, size_t optlen)
 {
-	tcp_tap_window_update(conn, ntohs(th->window));
+	tcp_tap_window_update(c, conn, ntohs(th->window));
 	tcp_get_tap_ws(conn, opts, optlen);
 
 	/* First value is not scaled */
@@ -2059,7 +2075,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 		if (!th->ack)
 			goto reset;
 
-		tcp_tap_window_update(conn, ntohs(th->window));
+		tcp_tap_window_update(c, conn, ntohs(th->window));
 
 		tcp_data_from_sock(c, conn);
 
@@ -2071,7 +2087,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 	if (conn->events & TAP_FIN_RCVD) {
 		tcp_sock_consume(conn, ntohl(th->ack_seq));
 		tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq));
-		tcp_tap_window_update(conn, ntohs(th->window));
+		tcp_tap_window_update(c, conn, ntohs(th->window));
 		tcp_data_from_sock(c, conn);
 
 		if (conn->events & SOCK_FIN_RCVD &&
-- 
@@ -1257,19 +1257,25 @@ static void tcp_get_tap_ws(struct tcp_tap_conn *conn,
 
 /**
  * tcp_tap_window_update() - Process an updated window from tap side
+ * @c:		Execution context
  * @conn:	Connection pointer
  * @wnd:	Window value, host order, unscaled
  */
-static void tcp_tap_window_update(struct tcp_tap_conn *conn, unsigned wnd)
+static void tcp_tap_window_update(const struct ctx *c,
+				  struct tcp_tap_conn *conn, unsigned wnd)
 {
 	wnd = MIN(MAX_WINDOW, wnd << conn->ws_from_tap);
 
 	/* Work-around for bug introduced in peer kernel code, commit
-	 * e2142825c120 ("net: tcp: send zero-window ACK when no memory").
-	 * We don't update if window shrank to zero.
+	 * e2142825c120 ("net: tcp: send zero-window ACK when no memory"): don't
+	 * update the window if it shrank to zero, so that we'll eventually
+	 * retry to send data, but rewind the sequence as that obviously implies
+	 * that no data beyond the updated window will ever be acknowledged.
 	 */
-	if (!wnd && SEQ_LT(conn->seq_ack_from_tap, conn->seq_to_tap))
+	if (!wnd && SEQ_LT(conn->seq_ack_from_tap, conn->seq_to_tap)) {
+		tcp_rewind_seq(c, conn);
 		return;
+	}
 
 	conn->wnd_from_tap = MIN(wnd >> conn->ws_from_tap, USHRT_MAX);
 
@@ -1694,7 +1700,8 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
 			tcp_timer_ctl(c, conn);
 
 			if (p->count == 1) {
-				tcp_tap_window_update(conn, ntohs(th->window));
+				tcp_tap_window_update(c, conn,
+						      ntohs(th->window));
 				return 1;
 			}
 
@@ -1713,6 +1720,15 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
 				       ack_seq == max_ack_seq &&
 				       ntohs(th->window) == max_ack_seq_wnd;
 
+				/* See tcp_tap_window_update() for details. On
+				 * top of that, we also need to check here if a
+				 * zero-window update is contained in a batch of
+				 * packets that includes a non-zero window as
+				 * well.
+				 */
+				if (!ntohs(th->window))
+					tcp_rewind_seq(c, conn);
+
 				max_ack_seq_wnd = ntohs(th->window);
 				max_ack_seq = ack_seq;
 			}
@@ -1772,7 +1788,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
 	if (ack && !tcp_sock_consume(conn, max_ack_seq))
 		tcp_update_seqack_from_tap(c, conn, max_ack_seq);
 
-	tcp_tap_window_update(conn, max_ack_seq_wnd);
+	tcp_tap_window_update(c, conn, max_ack_seq_wnd);
 
 	if (retr) {
 		flow_trace(conn,
@@ -1861,7 +1877,7 @@ static void tcp_conn_from_sock_finish(const struct ctx *c,
 				      const struct tcphdr *th,
 				      const char *opts, size_t optlen)
 {
-	tcp_tap_window_update(conn, ntohs(th->window));
+	tcp_tap_window_update(c, conn, ntohs(th->window));
 	tcp_get_tap_ws(conn, opts, optlen);
 
 	/* First value is not scaled */
@@ -2059,7 +2075,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 		if (!th->ack)
 			goto reset;
 
-		tcp_tap_window_update(conn, ntohs(th->window));
+		tcp_tap_window_update(c, conn, ntohs(th->window));
 
 		tcp_data_from_sock(c, conn);
 
@@ -2071,7 +2087,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 	if (conn->events & TAP_FIN_RCVD) {
 		tcp_sock_consume(conn, ntohl(th->ack_seq));
 		tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq));
-		tcp_tap_window_update(conn, ntohs(th->window));
+		tcp_tap_window_update(c, conn, ntohs(th->window));
 		tcp_data_from_sock(c, conn);
 
 		if (conn->events & SOCK_FIN_RCVD &&
-- 
2.43.0