Re: [PATCH 4/6] tcp: Fix closing logic for half-closed connections

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

On Fri, Aug 15, 2025 at 06:10:40PM +0200, Stefano Brivio wrote:
> First off, don't close connections half-closed by the guest before
> our own FIN is acknowledged by the guest itself.
> 
> That is, after we receive a FIN from the guest (TAP_FIN_RCVD), if we
> don't have any data left to send from the socket (SOCK_FIN_RCVD, or
> EPOLLHUP), we send a FIN segment to the guest (TAP_FIN_SENT), but we
> need to actually have it acknowledged (and have no pending
> retransmissions) before we can close the connection: check for
> TAP_FIN_ACKED, first.
> 
> Then, if we set TAP_FIN_SENT, and we receive an ACK segment from the
> guest, set TAP_FIN_ACKED. This was entirely missing for the
> TAP_FIN_RCVD case, and as we fix the problem described above, this
> becomes relevant as well.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tcp.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index dda0a2e..aed25a9 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2081,9 +2081,14 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  		tcp_tap_window_update(c, conn, ntohs(th->window));
>  		tcp_data_from_sock(c, conn);
>  
> -		if (conn->events & SOCK_FIN_RCVD &&
> -		    conn->seq_ack_from_tap == conn->seq_to_tap)
> -			conn_event(c, conn, CLOSED);
> +		if (conn->seq_ack_from_tap == conn->seq_to_tap) {
> +			if (th->ack && conn->events & TAP_FIN_SENT)
> +				conn_event(c, conn, TAP_FIN_ACKED);
> +
> +			if (conn->events & SOCK_FIN_RCVD &&
> +			    conn->events & TAP_FIN_ACKED)
> +				conn_event(c, conn, CLOSED);
> +		}
>  
>  		return 1;
>  	}
> @@ -2363,7 +2368,7 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref,
>  		return;
>  	}
>  
> -	if ((conn->events & TAP_FIN_SENT) && (events & EPOLLHUP)) {
> +	if ((conn->events & TAP_FIN_ACKED) && (events & EPOLLHUP)) {
>  		conn_event(c, conn, CLOSED);
>  		return;
>  	}

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]