[PATCH] tcp: Flush socket before checking for more data in active close state
Otherwise, if all the pending data is acknowledged:
- tcp_update_seqack_from_tap() updates the current tap-side ACK
sequence (conn->seq_ack_from_tap)
- next, we compare the sequence we sent (conn->seq_to_tap) to the
ACK sequence (conn->seq_ack_from_tap) in tcp_data_from_sock() to
understand if there's more data we can send.
If they match, we conclude that we haven't sent any of that data,
and keep re-sending it.
We need, instead, to flush the socket (drop acknowledged data) before
calling tcp_update_seqack_from_tap(), so that once we update
conn->seq_ack_from_tap, we can be sure that all data until there is
gone from the socket.
Link: https://bugs.passt.top/show_bug.cgi?id=114
Reported-by: Marek Marczykowski-Górecki
On Wed, Mar 19, 2025 at 08:05:19PM +0100, Stefano Brivio wrote:
Otherwise, if all the pending data is acknowledged:
- tcp_update_seqack_from_tap() updates the current tap-side ACK sequence (conn->seq_ack_from_tap)
- next, we compare the sequence we sent (conn->seq_to_tap) to the ACK sequence (conn->seq_ack_from_tap) in tcp_data_from_sock() to understand if there's more data we can send.
If they match, we conclude that we haven't sent any of that data, and keep re-sending it.
We need, instead, to flush the socket (drop acknowledged data) before calling tcp_update_seqack_from_tap(), so that once we update conn->seq_ack_from_tap, we can be sure that all data until there is gone from the socket.
IIUC, this is, roughly speaking, a manifestation of the difference between "seq ack from tap" and "seq ack to sock". We don't track the latter, because it's kind of implied by the former. However, with the incorrect ordering here the brief window in which they're not the same is biting us.
Link: https://bugs.passt.top/show_bug.cgi?id=114 Reported-by: Marek Marczykowski-Górecki
Fixes: 30f1e082c3c0 ("tcp: Keep updating window and checking for socket data after FIN from guest") Signed-off-by: Stefano Brivio
Reviewed-by: David Gibson
--- tcp.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tcp.c b/tcp.c index 68af43d..fa1d885 100644 --- a/tcp.c +++ b/tcp.c @@ -2049,6 +2049,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
/* Established connections not accepting data from tap */ 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_data_from_sock(c, conn);
-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
participants (2)
-
David Gibson
-
Stefano Brivio