Re: [PATCH 2/3] tcp: Properly propagate tap-side RST to socket side

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

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

On Tue, Jan 27, 2026 at 12:32:32PM +0100, Stefano Brivio wrote:
> On Tue, 27 Jan 2026 19:39:52 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > When the guest sends a TCP RST, or on certain error conditions, we want to
> > signal the abnormal termination of a TCP connection to the peer with an
> > RST as well.  We attempt to do that by close()ing the socket.
> > 
> > That doesn't work: a close() will usually send a FIN, rather than an RST.
> > The standard method of forcing an RST on a socket is to set the SO_LINGER
> > socket option with a 0 timeout, then close().
> > 
> > Update the tcp_rst() path to do this, so it forces a socket side RST.
> > Update the handling of a guest side RST to use the same path (minus
> > sending a tap side RST) so that we properly propagate guest RSTs to the
> > peer.
> > 
> > Link: https://bugs.passt.top/show_bug.cgi?id=191
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  tcp.c | 37 +++++++++++++++++++++++++++++++++----
> >  1 file changed, 33 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tcp.c b/tcp.c
> > index 45dde5a0..9da37c2f 100644
> > --- a/tcp.c
> > +++ b/tcp.c
> > @@ -1403,7 +1403,34 @@ static int tcp_send_flag(const struct ctx *c, struct tcp_tap_conn *conn,
> >  }
> >  
> >  /**
> > - * tcp_rst_do() - Reset a tap connection: send RST segment to tap, close socket
> > + * tcp_sock_rst() - Close TCP connection forcing RST on socket side
> > + * @c:		Execution context
> > + * @conn:	Connection pointer
> > + */
> > +static void tcp_sock_rst(const struct ctx *c, struct tcp_tap_conn *conn)
> > +{
> > +	const struct linger linger0 = {
> > +		.l_onoff = 1,
> > +		.l_linger = 0,
> > +	};
> > +
> > +	/* Force RST on socket to inform the peer
> > +	 *
> > +	 * We do this by setting SO_LINGER with 0 timeout, which means that
> > +	 * close() will send an RST (unless the connection is already closed in
> > +	 * both directions).
> > +	 */
> > +	if (setsockopt(conn->sock, SOL_SOCKET,
> > +		       SO_LINGER, &linger0, sizeof(linger0)) < 0) {
> > +		flow_dbg_perror(conn,
> > +				"SO_LINGER failed, may not send RST to peer");
> > +	}
> > +
> > +	conn_event(c, conn, CLOSED);
> > +}
> > +
> > +/**
> > + * tcp_rst_do() - Reset a tap connection: send RST segment on both sides, close
> >   * @c:		Execution context
> >   * @conn:	Connection pointer
> >   */
> > @@ -1412,8 +1439,10 @@ void tcp_rst_do(const struct ctx *c, struct tcp_tap_conn *conn)
> >  	if (conn->events == CLOSED)
> >  		return;
> >  
> > +	/* Send RST on tap */
> >  	tcp_send_flag(c, conn, RST);
> > -	conn_event(c, conn, CLOSED);
> > +
> > +	tcp_sock_rst(c, conn);
> >  }
> >  
> >  /**
> > @@ -1884,7 +1913,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
> >  			return -1;
> >  
> >  		if (th->rst) {
> > -			conn_event(c, conn, CLOSED);
> > +			tcp_sock_rst(c, conn);
> 
> The whole series looks good to me, except for one exceedingly minor
> aspect: should we do this also in the getsockopt() error handling path
> of tcp_prepare_flags()?

Yes, I think we should.  Or, perhaps more to the point, we should
actually handle the error code that tcp_prepare_flags() returns via
tcp_send_flag(), and we currently ignore in all callers.

> I would be inclined to apply it regardless of that, the fix is critical
> enough. I'll start the usual test run in a few hours.
> 
> -- 
> 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

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