net/rds: RDS-TCP robustness fixes

This patch-set contains bug fixes for state-recovery at the RDS layer when the underlying transport is TCP and the TCP state at one of the endpoints is reset

V2 changes: DaveM comments to reduce memory footprint, follow
NFS/RPC model where possible. Added test-case #3

Without the changes in this set, when one of the endpoints is reset, the existing code does not correctly clean up RDS socket state for stale connections, resulting in some unstable, timing-dependant behavior on the wire, including an infinite exchange of 3WHs back-and-forth, and a resulting potential to never converge RDS state.

Test cases used to verify the changes in this set are:

1. Start rds client/server applications on two participating nodes, node1 and node2. After at least one packet has been sent (to establish the TCP connection), restart the rds_tcp module on the client, and now resend packets. Tcpdump should show server sending a FIN for the "old" client port, and clean connection establishment/exchange for the new client port.

2. At the end of step 1, restart rds srever on node2, and start client on node1, make sure using tcpdump, 'netstat -an|grep 16385' that packets flow correctly.

3. start RDS client/server application on two participating nodes, and repeat steps 1 and 2, but this time, simulate node failure by doing "ifconfig <intf> down", so no FIN is sent.

Sowmini Varadhan (2):
RDS-TCP: Always create a new rds_sock for an incoming connection.
RDS-TCP: only initiate reconnect attempt on outgoing TCP socket.

 net/rds/connection.c  |   17 +++++++++++++++--
 net/rds/tcp_connect.c |    1 +
 net/rds/tcp_listen.c  |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

When running RDS over TCP, the active (client) side connects to the listening ("passive") side at the RDS_TCP_PORT. After the connection is established, if the client side reboots (potentially without even sending a FIN) the server still has a TCP socket in the esablished state. If the server-side now gets a new SYN comes from the client with a different client port, TCP will create a new socket-pair, but the RDS layer will incorrectly pull up the old rds_connection (which is still associated with the stale t_sock and RDS socket state).

This patch corrects this behavior by having rds_tcp_accept_one() always create a new connection for an incoming TCP SYN.
The rds and tcp state associated with the old socket-pair is cleaned up via the rds_tcp_state_change() callback which would typically be invoked in most cases when the client-TCP sends a FIN on TCP restart, triggering a transition to CLOSE_WAIT state. In the rarer event of client death without a FIN, TCP_KEEPALIVE probes on the socket will detect the stale socket, and the TCP transition to CLOSE state will trigger the RDS state cleanup.

---
v2: DaveM comments- make the TCP server side recovery follow patterns in
NFS/RPC, and keep things neat without needless memory bloat.

 net/rds/connection.c  |    4 ++++
 net/rds/tcp_connect.c |    1 +
 net/rds/tcp_listen.c  |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 0 deletions(-)

index 14f0413..60f0cd6 100644
@@ -126,7 +126,10 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
 	struct rds_transport *loop_trans;
 	unsigned long flags;
 	int ret;
+	struct rds_transport *otrans = trans;
 
+	if (!is_outgoing && otrans->t_type == RDS_TRANS_TCP)
+		goto new_conn;
 	rcu_read_lock();
 	conn = rds_conn_lookup(head, laddr, faddr, trans);
 	if (conn && conn->c_loopback && conn->c_trans != &rds_loop_transport &&

@@ -142,6 +145,7 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
 	if (conn)
 		goto out;
 
+new_conn:
 	conn = kmem_cache_zalloc(rds_conn_slab, gfp);
 	if (!conn) {
 		conn = ERR_PTR(-ENOMEM);

index f9f564a..973109c 100644
@@ -62,6 +62,7 @@ void rds_tcp_state_change(struct sock *sk)
 		case TCP_ESTABLISHED:
 			rds_connect_complete(conn);
 			break;
+		case TCP_CLOSE_WAIT:
 		case TCP_CLOSE:
 			rds_conn_drop(conn);
 		default:

1.7.1

When the peer of an RDS-TCP connection restarts, a reconnect attempt should only be made from the active side of the TCP connection, i.e. the side that has a transient TCP port number. Do not add the passive side of the TCP connection to the c_hash_node and thus avoid triggering rds_queue_reconnect() for passive rds connections.

---
 net/rds/connection.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

index 60f0cd6..da6da57 100644
@@ -234,13 +234,22 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
 		/* Creating normal conn */
 		struct rds_connection *found;
 
-		found = rds_conn_lookup(head, laddr, faddr, trans);
+		if (!is_outgoing && otrans->t_type == RDS_TRANS_TCP)
+			found = NULL;
+		else
+			found = rds_conn_lookup(head, laddr, faddr, trans);
 		if (found) {
 			trans->conn_free(conn->c_transport_data);
 			kmem_cache_free(rds_conn_slab, conn);
 			conn = found;
 		} else {
-			hlist_add_head_rcu(&conn->c_hash_node, head);
+			if ((is_outgoing && otrans->t_type == RDS_TRANS_TCP) ||
+			    (otrans->t_type != RDS_TRANS_TCP)) {
+				/* Only the active side should be added to
+				 * reconnect list for TCP.
+				 */
+				hlist_add_head_rcu(&conn->c_hash_node, head);
+			}
 			rds_cong_add_conn(conn);
 			rds_conn_count++;
 		}

1.7.1

From: Sowmini Varadhan <[email protected]>
Date: Tue, 5 May 2015 15:20:50 -0400

...

Yeah this looks a lot better, series applied, thanks.