Re: [PATCH 6/6] tcp_splice: Simplify shutdown(2) handling

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 20 May 2026 23:08:51 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> At the end of tcp_splice_forward(), we check for half-closed connections
> and propagate the FIN to the other side with a shutdown(2).  Currently we
> check for a half closed connection in either direction.  That's unnecessary
> here, because tcp_splice_forward() will already be called for each
> direction if there are any relevant events.

True, but do we have the guarantee that tcp_splice_forward() will also
be called once all relevant FIN_RCVD / FIN_SENT flags have been sent?

The reason why we check both sides here is that we might have updated
flags for one side, and now we need to double check if it's time to
call shutdown() as a consequence.

Maybe we never have to, but I think it's not really obvious to prove.

> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  tcp_splice.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/tcp_splice.c b/tcp_splice.c
> index b45f0060..e5018f2e 100644
> --- a/tcp_splice.c
> +++ b/tcp_splice.c
> @@ -582,21 +582,15 @@ retry:
>  			break;
>  	}
>  
> -	if (!conn->pending[fromsidei] &&
> -	    conn->events & FIN_RCVD(fromsidei)) {
> -		unsigned sidei;
> -
> -		flow_foreach_sidei(sidei) {
> -			if ((conn->events & FIN_RCVD(sidei)) &&
> -			    !(conn->events & FIN_SENT(!sidei))) {
> -				if (shutdown(conn->s[!sidei], SHUT_WR) < 0) {
> -					flow_perror(conn, "shutdown() on %s",
> -						    pif_name(conn->f.pif[!sidei]));
> -					return -1;
> -				}
> -				conn_event(conn, FIN_SENT(!sidei));
> -			}
> +	if ((conn->events & FIN_RCVD(fromsidei)) &&
> +	    !(conn->events & FIN_SENT(!fromsidei)) &&
> +	    !conn->pending[fromsidei]) {
> +		if (shutdown(conn->s[!fromsidei], SHUT_WR) < 0) {
> +			flow_perror(conn, "shutdown() on %s",
> +				    pif_name(conn->f.pif[!fromsidei]));
> +			return -1;
>  		}
> +		conn_event(conn, FIN_SENT(!fromsidei));
>  	}
>  
>  	return 0;

-- 
Stefano