Skip to contentSkip to conversation

RDS-TCP: Reset tcp callbacks if re-using an outgoing socket in rds_tcp_accept_one()

PATCH Applied Sowmini Varadhan 3 participants 3 messages Oct 11, 2015 – Oct 13, 2015

Subsystems (3)
Networking Network drivers RDS - reliable datagram sockets
lore
Share & tools
lore

Sowmini Varadhan

author
#0 lore raw

PATCH 1/1 RDS-TCP: Reset tcp callbacks if re-using an outgoing socket in rds_tcp_accept_one()

Consider the following "duelling syn" sequence between two peers A and B:

        	A		B
        	SYN1     -->
        	    	<--	SYN2
        	SYN2ACK  -->

Note that the SYN/ACK has already been sent out by TCP before rds_tcp_accept_one() gets invoked as part of callbacks.

If the inet_addr(A) is numerically less than inet_addr(B), the arbitration scheme in rds_tcp_accept_one() will prefer the TCP connection triggered by SYN1, and will send a CLOSE for the SYN2 (just after the SYN2ACK was sent).

Since B also follows the same arbitration scheme, it will send the SYN-ACK for SYN1 that will set up a healthy ESTABLISHED connection on both sides. B will also get a CLOSE for SYN2, which should result in the cleanup of the TCP state machine for SYN2, but it should not trigger any stale RDS-TCP callbacks (such as ->writespace, ->state_change etc), that would disrupt the progress of the SYN2 based RDS-TCP connection.

Thus the arbitration scheme in rds_tcp_accept_one() should restore rds_tcp callbacks for the winner before setting them up for the new accept socket, and also make sure that conn->c_outgoing is set to 0 so that we do not trigger any reconnect attempts on the passive side of the tcp socket in the future, in conformance with commit c82ac7e69efe ("net/rds: RDS-TCP: only initiate reconnect attempt on outgoing TCP socket.")

Signed-off-by Sowmini Varadhan <[email protected]>
---
 net/rds/tcp_listen.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
net/rds/tcp_listen.c +3 -0
index 1d90240..0936a4a 100644
@@ -125,6 +125,9 @@ int rds_tcp_accept_one(struct socket *sock)
 		new_sock = NULL;
 		ret = 0;
 		goto out;
+	} else if (rs_tcp->t_sock) {
+		rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
+		conn->c_outgoing = 0;
 	}
 
 	rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
Signature · 1 line
1.7.1
Acked-by on v1 1/1
#1 lore raw
Quoted 30 lines · Sowmini Varadhan

Consider the following "duelling syn" sequence between two peers A and B:

         	A		B
         	SYN1     -->
         	    	<--	SYN2
         	SYN2ACK  -->

Note that the SYN/ACK has already been sent out by TCP before rds_tcp_accept_one() gets invoked as part of callbacks.

If the inet_addr(A) is numerically less than inet_addr(B), the arbitration scheme in rds_tcp_accept_one() will prefer the TCP connection triggered by SYN1, and will send a CLOSE for the SYN2 (just after the SYN2ACK was sent).

Since B also follows the same arbitration scheme, it will send the SYN-ACK for SYN1 that will set up a healthy ESTABLISHED connection on both sides. B will also get a CLOSE for SYN2, which should result in the cleanup of the TCP state machine for SYN2, but it should not trigger any stale RDS-TCP callbacks (such as ->writespace, ->state_change etc), that would disrupt the progress of the SYN2 based RDS-TCP connection.

Thus the arbitration scheme in rds_tcp_accept_one() should restore rds_tcp callbacks for the winner before setting them up for the new accept socket, and also make sure that conn->c_outgoing is set to 0 so that we do not trigger any reconnect attempts on the passive side of the tcp socket in the future, in conformance with commit c82ac7e69efe ("net/rds: RDS-TCP: only initiate reconnect attempt on outgoing TCP socket.")

Quoted 2 lines
Signed-off-by Sowmini Varadhan <[email protected]>

---

Looks fine to me.

Acked-by Santosh Shilimkar <[email protected]>
maintainer Applied
#2 lore raw
In reply to Sowmini Varadhan

From: Sowmini Varadhan <[email protected]>
Date: Sun, 11 Oct 2015 16:49:44 -0400

Quoted 30 lines

Consider the following "duelling syn" sequence between two peers A and B:

        	A		B
        	SYN1     -->
        	    	<--	SYN2
        	SYN2ACK  -->

Note that the SYN/ACK has already been sent out by TCP before rds_tcp_accept_one() gets invoked as part of callbacks.

If the inet_addr(A) is numerically less than inet_addr(B), the arbitration scheme in rds_tcp_accept_one() will prefer the TCP connection triggered by SYN1, and will send a CLOSE for the SYN2 (just after the SYN2ACK was sent).

Since B also follows the same arbitration scheme, it will send the SYN-ACK for SYN1 that will set up a healthy ESTABLISHED connection on both sides. B will also get a CLOSE for SYN2, which should result in the cleanup of the TCP state machine for SYN2, but it should not trigger any stale RDS-TCP callbacks (such as ->writespace, ->state_change etc), that would disrupt the progress of the SYN2 based RDS-TCP connection.

Thus the arbitration scheme in rds_tcp_accept_one() should restore rds_tcp callbacks for the winner before setting them up for the new accept socket, and also make sure that conn->c_outgoing is set to 0 so that we do not trigger any reconnect attempts on the passive side of the tcp socket in the future, in conformance with commit c82ac7e69efe ("net/rds: RDS-TCP: only initiate reconnect attempt on outgoing TCP socket.")

Quoted 1 line
Signed-off-by Sowmini Varadhan <[email protected]>

Applied.