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 <marmarek(a)invisiblethingslab.com> Fixes: 30f1e082c3c0 ("tcp: Keep updating window and checking for socket data after FIN from guest") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>--- 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