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

[Jon Maloy <jmaloy@redhat.com>]

On 2025-09-09 14:16, 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>

Reviewed-by: Jon Maloy <jmaloy@redhat.com>

> ---
>   tcp.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 12d42e0..b83510b 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2116,9 +2116,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;
>   	}
> @@ -2398,7 +2403,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;
>   	}