On Wed, May 20, 2026 at 10:30:23PM +0200, Stefano Brivio wrote:
On Wed, 20 May 2026 23:08:51 +1000 David Gibson
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?
Yes, because tcp_splice_forward() is (now) the only place that *sets* FIN_RCVD (or FIN_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.
tcp_splice_forward() only touches FIN_RCVD(fromsidei). So, we only need to examine FIN_RCVD(fromsidei). If FIN_RCVD is set for the other side, it must be in another call to tcp_splice_forward() which will also examine that other flag and shutdown() as necessary.
Signed-off-by: David Gibson
--- 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
-- 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