[PATCH 0/7] rdma/siw: implement non-blocking connect

[PATCH 0/7] rdma/siw: implement non-blocking connect

[Stefan Metzmacher]

Hi Bernard,

as written a few month ago, I have a patchset with a lot
of fixes for siw.ko.

As requested I'm only send isolated chunks for easier review.

This is the first chunk adressing deadlocks in siw_connect()

The RDMA application layer expects rdma_connect() to be non-blocking
as the completion is handled via RDMA_CM_EVENT_ESTABLISHED and
other async events. It's not unlikely to hold a lock during
the rdma_connect() call.

Without out this a connection attempt to a non-existing/reachable
server block until the very long tcp timeout hits.
The application layer had no chance to have its own timeout handler
as that would just deadlock with the already blocking rdma_connect().

First rdma_connect() holds id_priv->handler_mutex and deadlocks
rdma_destroy_id().

And iw_cm_connect() called from within rdma_connect() sets
IWCM_F_CONNECT_WAIT during the call to cm_id->device->ops.iw_connect(),
siw_connect() in this case. It means that iw_cm_disconnect()
and iw_destroy_cm_id() will both deadlock waiting for
IWCM_F_CONNECT_WAIT being cleared.

Patches 1-6 are preparation patches to siw_connect()
in order to do the real non-blocking split in Patch 7.

Please have a look.

Thanks!
metze

Stefan Metzmacher (7):
  rdma/siw: make use of kernel_{bind,connect,listen}()
  rdma/siw: let siw_connect() set AWAIT_MPAREP before
    siw_send_mpareqrep()
  rdma/siw: create a temporary copy of private data
  rdma/siw: use error and out logic at the end of siw_connect()
  rdma/siw: start mpa timer before calling siw_send_mpareqrep()
  rdma/siw: call the blocking kernel_bindconnect() just before
    siw_send_mpareqrep()
  rdma/siw: implement non-blocking connect.

 drivers/infiniband/sw/siw/siw_cm.c | 165 ++++++++++++++++++++++-------
 drivers/infiniband/sw/siw/siw_cm.h |   1 +
 2 files changed, 127 insertions(+), 39 deletions(-)

-- 
2.34.1

[PATCH 1/7] rdma/siw: make use of kernel_{bind,connect,listen}()

[Stefan Metzmacher]

That's nicer than dereferencing socket structures.

This prepares making rdma_connect()/siw_connect() non-blocking
in order to avoid deadlocks in the callers.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 17f34d584cd9..644c7d50e991 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1318,11 +1318,11 @@ static int kernel_bindconnect(struct socket *s, struct sockaddr *laddr,
 			return rv;
 	}
 
-	rv = s->ops->bind(s, laddr, size);
+	rv = kernel_bind(s, laddr, size);
 	if (rv < 0)
 		return rv;
 
-	rv = s->ops->connect(s, raddr, size, flags);
+	rv = kernel_connect(s, raddr, size, flags);
 
 	return rv < 0 ? rv : 0;
 }
@@ -1788,8 +1788,8 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 		if (ipv4_is_zeronet(laddr->sin_addr.s_addr))
 			s->sk->sk_bound_dev_if = sdev->netdev->ifindex;
 
-		rv = s->ops->bind(s, (struct sockaddr *)laddr,
-				  sizeof(struct sockaddr_in));
+		rv = kernel_bind(s, (struct sockaddr *)laddr,
+				 sizeof(struct sockaddr_in));
 	} else {
 		struct sockaddr_in6 *laddr = &to_sockaddr_in6(id->local_addr);
 
@@ -1806,8 +1806,8 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 		if (ipv6_addr_any(&laddr->sin6_addr))
 			s->sk->sk_bound_dev_if = sdev->netdev->ifindex;
 
-		rv = s->ops->bind(s, (struct sockaddr *)laddr,
-				  sizeof(struct sockaddr_in6));
+		rv = kernel_bind(s, (struct sockaddr *)laddr,
+				 sizeof(struct sockaddr_in6));
 	}
 	if (rv) {
 		siw_dbg(id->device, "socket bind error: %d\n", rv);
@@ -1827,7 +1827,7 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 			rv, backlog);
 		goto error;
 	}
-	rv = s->ops->listen(s, backlog);
+	rv = kernel_listen(s, backlog);
 	if (rv) {
 		siw_dbg(id->device, "listen error %d\n", rv);
 		goto error;
-- 
2.34.1

[PATCH 2/7] rdma/siw: let siw_connect() set AWAIT_MPAREP before siw_send_mpareqrep()

[Stefan Metzmacher]

There's no real change made in this commit, but it makes the follwing
commits easier to review.

The idea is that we stay in SIW_EPSTATE_CONNECTING as long as
we only deal with tcp and not started the MPA negotiation.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 644c7d50e991..8d1e7f497cf9 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1425,8 +1425,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	 */
 	siw_cep_socket_assoc(cep, s);
 
-	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
-
 	/*
 	 * Set MPA Request bits: CRC if required, no MPA Markers,
 	 * MPA Rev. according to module parameter 'mpa_version', Key 'Request'.
@@ -1469,6 +1467,8 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	}
 	memcpy(cep->mpa.hdr.key, MPA_KEY_REQ, 16);
 
+	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
+
 	rv = siw_send_mpareqrep(cep, params->private_data, pd_len);
 	/*
 	 * Reset private data.
-- 
2.34.1

[PATCH 3/7] rdma/siw: create a temporary copy of private data

[Stefan Metzmacher]

The final patch will implement a non-blocking connect,
which means that siw_connect() will be split into
siw_connect() and siw_connected().

kernel_bindconnect() will be the last action
in siw_connect(), while the MPA negotiation
is deferred to siw_connected().

We should not rely on the callers private data
pointers to be still valid when siw_connected()
is called, so we better create a copy.

Also note that __siw_cep_dealloc() already calls
kfree(cep->mpa.pdata), so we already have the required cleanup
when we'll split out siw_connected() and an error will
prevent siw_connected() being called at all.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 8d1e7f497cf9..0e53219d29de 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1467,13 +1467,27 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	}
 	memcpy(cep->mpa.hdr.key, MPA_KEY_REQ, 16);
 
+	if (pd_len > 0) {
+		cep->mpa.pdata = kmemdup(params->private_data, pd_len, GFP_KERNEL);
+		if (IS_ERR_OR_NULL(cep->mpa.pdata)) {
+			rv = -ENOMEM;
+			goto error;
+		}
+		cep->mpa.hdr.params.pd_len = pd_len;
+	}
+
 	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
-	rv = siw_send_mpareqrep(cep, params->private_data, pd_len);
+	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
+				cep->mpa.hdr.params.pd_len);
 	/*
 	 * Reset private data.
 	 */
-	cep->mpa.hdr.params.pd_len = 0;
+	if (cep->mpa.hdr.params.pd_len) {
+		cep->mpa.hdr.params.pd_len = 0;
+		kfree(cep->mpa.pdata);
+		cep->mpa.pdata = NULL;
+	}
 
 	if (rv >= 0) {
 		rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
-- 
2.34.1

[PATCH 4/7] rdma/siw: use error and out logic at the end of siw_connect()

[Stefan Metzmacher]

This will make the following changes easier.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 0e53219d29de..b19a2b777814 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1489,14 +1489,19 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		cep->mpa.pdata = NULL;
 	}
 
-	if (rv >= 0) {
-		rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
-		if (!rv) {
-			siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
-			siw_cep_set_free(cep);
-			return 0;
-		}
+	if (rv < 0) {
+		goto error;
+	}
+
+	rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
+	if (rv != 0) {
+		goto error;
 	}
+
+	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
+	siw_cep_set_free(cep);
+	return 0;
+
 error:
 	siw_dbg(id->device, "failed: %d\n", rv);
 
-- 
2.34.1

[PATCH 5/7] rdma/siw: start mpa timer before calling siw_send_mpareqrep()

[Stefan Metzmacher]

The mpa timer will also span the non-blocking connect
in the final patch.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index b19a2b777814..3fee1d4ef252 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1476,6 +1476,11 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		cep->mpa.hdr.params.pd_len = pd_len;
 	}
 
+	rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
+	if (rv != 0) {
+		goto error;
+	}
+
 	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
 	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
@@ -1493,11 +1498,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		goto error;
 	}
 
-	rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
-	if (rv != 0) {
-		goto error;
-	}
-
 	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
 	siw_cep_set_free(cep);
 	return 0;
@@ -1506,6 +1506,8 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	siw_dbg(id->device, "failed: %d\n", rv);
 
 	if (cep) {
+		siw_cancel_mpatimer(cep);
+
 		siw_socket_disassoc(s);
 		sock_release(s);
 		cep->sock = NULL;
-- 
2.34.1

Re: [PATCH 5/7] rdma/siw: start mpa timer before calling siw_send_mpareqrep()

[Cheng Xu]

On 6/15/22 4:40 PM, Stefan Metzmacher wrote:
> The mpa timer will also span the non-blocking connect
> in the final patch.
> 
> Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
> Signed-off-by: Stefan Metzmacher <metze@samba.org>
> Cc: Bernard Metzler <bmt@zurich.ibm.com>
> Cc: linux-rdma@vger.kernel.org
> ---
>   drivers/infiniband/sw/siw/siw_cm.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
> index b19a2b777814..3fee1d4ef252 100644
> --- a/drivers/infiniband/sw/siw/siw_cm.c
> +++ b/drivers/infiniband/sw/siw/siw_cm.c
> @@ -1476,6 +1476,11 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
>   		cep->mpa.hdr.params.pd_len = pd_len;
>   	}
>   
> +	rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
> +	if (rv != 0) {
> +		goto error;
> +	}
> +
>   	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
>   

Here starts the MPA timer, but the cep->state is SIW_EPSTATE_CONNECTING.

Consider the case when the connection timeout: the MPA timeout handler
will release resources if cep->state is SIW_EPSTATE_AWAIT_MPAREP and
SIW_EPSTATE_AWAIT_MPAREQ, not including SIW_EPSTATE_CONNECTING.

I think you should handle this case in the MPA timeout handler: report
a cm event and set release_cep with 1. Otherwise it will cause resource
leak.

Thanks,
Cheng Xu

Re: [PATCH 5/7] rdma/siw: start mpa timer before calling siw_send_mpareqrep()

[Stefan Metzmacher]

Hi Cheng,
> On 6/15/22 4:40 PM, Stefan Metzmacher wrote:
>> The mpa timer will also span the non-blocking connect
>> in the final patch.
>>
>> Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
>> Signed-off-by: Stefan Metzmacher <metze@samba.org>
>> Cc: Bernard Metzler <bmt@zurich.ibm.com>
>> Cc: linux-rdma@vger.kernel.org
>> ---
>>   drivers/infiniband/sw/siw/siw_cm.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
>> index b19a2b777814..3fee1d4ef252 100644
>> --- a/drivers/infiniband/sw/siw/siw_cm.c
>> +++ b/drivers/infiniband/sw/siw/siw_cm.c
>> @@ -1476,6 +1476,11 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
>>           cep->mpa.hdr.params.pd_len = pd_len;
>>       }
>> +    rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
>> +    if (rv != 0) {
>> +        goto error;
>> +    }
>> +
>>       cep->state = SIW_EPSTATE_AWAIT_MPAREP;
> 
> Here starts the MPA timer, but the cep->state is SIW_EPSTATE_CONNECTING.
> 
> Consider the case when the connection timeout: the MPA timeout handler
> will release resources if cep->state is SIW_EPSTATE_AWAIT_MPAREP and
> SIW_EPSTATE_AWAIT_MPAREQ, not including SIW_EPSTATE_CONNECTING.
> 
> I think you should handle this case in the MPA timeout handler: report
> a cm event and set release_cep with 1. Otherwise it will cause resource
> leak.

Yes, you're right.

I've actually fixed it in the rest of the patchset I have.
It seems I need to pull in a few more patches into the first chunk.

metze

[PATCH 6/7] rdma/siw: call the blocking kernel_bindconnect() just before siw_send_mpareqrep()

[Stefan Metzmacher]

We should build all state before calling kernel_bindconnect().
This will allow us to go async in the final patch.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 3fee1d4ef252..9c5276d08538 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1373,18 +1373,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	if (rv < 0)
 		goto error;
 
-	/*
-	 * NOTE: For simplification, connect() is called in blocking
-	 * mode. Might be reconsidered for async connection setup at
-	 * TCP level.
-	 */
-	rv = kernel_bindconnect(s, laddr, raddr, id->afonly);
-	if (rv != 0) {
-		siw_dbg_qp(qp, "kernel_bindconnect: error %d\n", rv);
-		goto error;
-	}
-	if (siw_tcp_nagle == false)
-		tcp_sock_set_nodelay(s->sk);
 	cep = siw_cep_alloc(sdev);
 	if (!cep) {
 		rv = -ENOMEM;
@@ -1481,6 +1469,19 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		goto error;
 	}
 
+	/*
+	 * NOTE: For simplification, connect() is called in blocking
+	 * mode. Might be reconsidered for async connection setup at
+	 * TCP level.
+	 */
+	rv = kernel_bindconnect(s, laddr, raddr, id->afonly);
+	if (rv != 0) {
+		siw_dbg_qp(qp, "kernel_bindconnect: error %d\n", rv);
+		goto error;
+	}
+	if (siw_tcp_nagle == false)
+		tcp_sock_set_nodelay(s->sk);
+
 	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
 	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
-- 
2.34.1

[PATCH 7/7] rdma/siw: implement non-blocking connect.

[Stefan Metzmacher]

This is very important in order to prevent deadlocks.

The RDMA application layer expects rdma_connect() to be non-blocking
as the completion is handled via RDMA_CM_EVENT_ESTABLISHED and
other async events. It's not unlikely to hold a lock during
the rdma_connect() call.

Without out this a connection attempt to a non-existing/reachable
server block until the very long tcp timeout hits.
The application layer had no chance to have its own timeout handler
as that would just deadlock with the already blocking rdma_connect().

First rdma_connect() holds id_priv->handler_mutex and deadlocks
rdma_destroy_id().

And iw_cm_connect() called from within rdma_connect() sets
IWCM_F_CONNECT_WAIT during the call to cm_id->device->ops.iw_connect(),
siw_connect() in this case. It means that iw_cm_disconnect()
and iw_destroy_cm_id() will both deadlock waiting for
IWCM_F_CONNECT_WAIT being cleared.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 125 ++++++++++++++++++++++-------
 drivers/infiniband/sw/siw/siw_cm.h |   1 +
 2 files changed, 96 insertions(+), 30 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 9c5276d08538..e06375018472 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -37,6 +37,7 @@ static void siw_cm_llp_write_space(struct sock *s);
 static void siw_cm_llp_error_report(struct sock *s);
 static int siw_cm_upcall(struct siw_cep *cep, enum iw_cm_event_type reason,
 			 int status);
+static void siw_connected(struct siw_cep *cep);
 
 static void siw_sk_assign_cm_upcalls(struct sock *sk)
 {
@@ -1012,6 +1013,10 @@ static void siw_cm_work_handler(struct work_struct *w)
 		siw_accept_newconn(cep);
 		break;
 
+	case SIW_CM_WORK_CONNECTED:
+		siw_connected(cep);
+		break;
+
 	case SIW_CM_WORK_READ_MPAHDR:
 		if (cep->state == SIW_EPSTATE_AWAIT_MPAREQ) {
 			if (cep->listen_cep) {
@@ -1226,6 +1231,7 @@ static void siw_cm_llp_data_ready(struct sock *sk)
 	switch (cep->state) {
 	case SIW_EPSTATE_RDMA_MODE:
 	case SIW_EPSTATE_LISTENING:
+	case SIW_EPSTATE_CONNECTING:
 		break;
 
 	case SIW_EPSTATE_AWAIT_MPAREQ:
@@ -1279,12 +1285,26 @@ static void siw_cm_llp_state_change(struct sock *sk)
 
 	switch (sk->sk_state) {
 	case TCP_ESTABLISHED:
-		/*
-		 * handle accepting socket as special case where only
-		 * new connection is possible
-		 */
-		siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
-		break;
+		if (cep->state == SIW_EPSTATE_CONNECTING) {
+			/*
+			 * handle accepting socket as special case where only
+			 * new connection is possible
+			 */
+			siw_cm_queue_work(cep, SIW_CM_WORK_CONNECTED);
+			break;
+
+		} else if (cep->state == SIW_EPSTATE_LISTENING) {
+			/*
+			 * handle accepting socket as special case where only
+			 * new connection is possible
+			 */
+			siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
+			break;
+		}
+		siw_dbg_cep(cep,
+			    "unexpected socket state %d with cep state %d\n",
+			    sk->sk_state, cep->state);
+		fallthrough;
 
 	case TCP_CLOSE:
 	case TCP_CLOSE_WAIT:
@@ -1303,7 +1323,7 @@ static void siw_cm_llp_state_change(struct sock *sk)
 static int kernel_bindconnect(struct socket *s, struct sockaddr *laddr,
 			      struct sockaddr *raddr, bool afonly)
 {
-	int rv, flags = 0;
+	int rv;
 	size_t size = laddr->sa_family == AF_INET ?
 		sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6);
 
@@ -1322,7 +1342,18 @@ static int kernel_bindconnect(struct socket *s, struct sockaddr *laddr,
 	if (rv < 0)
 		return rv;
 
-	rv = kernel_connect(s, raddr, size, flags);
+	/*
+	 * Yes, this is really O_NONBLOCK instead of
+	 * SOCK_NONBLOCK.
+	 *
+	 * __sys_connect_file() passes
+	 * sock->file->f_flags | file_flags to
+	 * sock->ops->connect().
+	 *
+	 * Also io_connect() from io_uring forces
+	 * file_flags=O_NONBLOCK to __sys_connect_file().
+	 */
+	rv = kernel_connect(s, raddr, size, O_NONBLOCK);
 
 	return rv < 0 ? rv : 0;
 }
@@ -1469,36 +1500,27 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		goto error;
 	}
 
-	/*
-	 * NOTE: For simplification, connect() is called in blocking
-	 * mode. Might be reconsidered for async connection setup at
-	 * TCP level.
-	 */
 	rv = kernel_bindconnect(s, laddr, raddr, id->afonly);
+	if (rv == -EINPROGRESS) {
+		siw_dbg_qp(qp, "kernel_bindconnect: EINPROGRESS\n");
+		rv = 0;
+	}
 	if (rv != 0) {
 		siw_dbg_qp(qp, "kernel_bindconnect: error %d\n", rv);
 		goto error;
 	}
-	if (siw_tcp_nagle == false)
-		tcp_sock_set_nodelay(s->sk);
-
-	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
-	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
-				cep->mpa.hdr.params.pd_len);
 	/*
-	 * Reset private data.
+	 * The rest will be done by siw_connected()
+	 *
+	 * siw_cm_llp_state_change() will detect
+	 * TCP_ESTABLISHED and schedules SIW_CM_WORK_CONNECTED,
+	 * which will finally call siw_connected().
+	 *
+	 * As siw_cm_llp_state_change() handles everything
+	 * siw_cm_llp_data_ready() can be a noop for
+	 * SIW_EPSTATE_CONNECTING.
 	 */
-	if (cep->mpa.hdr.params.pd_len) {
-		cep->mpa.hdr.params.pd_len = 0;
-		kfree(cep->mpa.pdata);
-		cep->mpa.pdata = NULL;
-	}
-
-	if (rv < 0) {
-		goto error;
-	}
-
 	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
 	siw_cep_set_free(cep);
 	return 0;
@@ -1537,6 +1559,49 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	return rv;
 }
 
+static void siw_connected(struct siw_cep *cep)
+{
+	struct siw_qp *qp = cep->qp;
+	struct socket *s = cep->sock;
+	int rv = -ECONNABORTED;
+
+	/*
+	 * already called with
+	 * siw_cep_set_inuse(cep);
+	 */
+
+	if (cep->state != SIW_EPSTATE_CONNECTING)
+		goto error;
+
+	if (siw_tcp_nagle == false)
+		tcp_sock_set_nodelay(s->sk);
+
+	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
+
+	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
+				cep->mpa.hdr.params.pd_len);
+	/*
+	 * Reset private data.
+	 */
+	if (cep->mpa.hdr.params.pd_len) {
+		cep->mpa.hdr.params.pd_len = 0;
+		kfree(cep->mpa.pdata);
+		cep->mpa.pdata = NULL;
+	}
+
+	if (rv < 0) {
+		goto error;
+	}
+
+	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
+	return;
+
+error:
+	siw_dbg_cep(cep, "[QP %u]: exit, error %d\n", qp_id(qp), rv);
+	siw_qp_cm_drop(qp, 1);
+	return;
+}
+
 /*
  * siw_accept - Let SoftiWARP accept an RDMA connection request
  *
diff --git a/drivers/infiniband/sw/siw/siw_cm.h b/drivers/infiniband/sw/siw/siw_cm.h
index 8c59cb3e2868..c01bdc8e64ee 100644
--- a/drivers/infiniband/sw/siw/siw_cm.h
+++ b/drivers/infiniband/sw/siw/siw_cm.h
@@ -76,6 +76,7 @@ struct siw_cep {
 
 enum siw_work_type {
 	SIW_CM_WORK_ACCEPT = 1,
+	SIW_CM_WORK_CONNECTED,
 	SIW_CM_WORK_READ_MPAHDR,
 	SIW_CM_WORK_CLOSE_LLP, /* close socket */
 	SIW_CM_WORK_PEER_CLOSE, /* socket indicated peer close */
-- 
2.34.1

Re: [PATCH 7/7] rdma/siw: implement non-blocking connect.

[Cheng Xu]

On 6/15/22 4:40 PM, Stefan Metzmacher wrote:

<...>

> @@ -1279,12 +1285,26 @@ static void siw_cm_llp_state_change(struct sock *sk)
>   
>   	switch (sk->sk_state) {
>   	case TCP_ESTABLISHED:
> -		/*
> -		 * handle accepting socket as special case where only
> -		 * new connection is possible
> -		 */
> -		siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
> -		break;
> +		if (cep->state == SIW_EPSTATE_CONNECTING) {
> +			/*
> +			 * handle accepting socket as special case where only
> +			 * new connection is possible
> +			 */
> +			siw_cm_queue_work(cep, SIW_CM_WORK_CONNECTED);
> +			break;
> +
> +		} else if (cep->state == SIW_EPSTATE_LISTENING) {
> +			/*
> +			 * handle accepting socket as special case where only
> +			 * new connection is possible
> +			 */
> +			siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
> +			break;
> +		}
> +		siw_dbg_cep(cep,
> +			    "unexpected socket state %d with cep state %d\n",
> +			    sk->sk_state, cep->state);
> +		fallthrough;
>   

Is "faillthrough" a annotation ?

Thanks,
Cheng Xu

Re: [PATCH 7/7] rdma/siw: implement non-blocking connect.

[Cheng Xu]

On 6/15/22 5:56 PM, Cheng Xu wrote:
> 
> 
> On 6/15/22 4:40 PM, Stefan Metzmacher wrote:
> 
> <...>
> 
>> @@ -1279,12 +1285,26 @@ static void siw_cm_llp_state_change(struct 
>> sock *sk)
>>       switch (sk->sk_state) {
>>       case TCP_ESTABLISHED:
>> -        /*
>> -         * handle accepting socket as special case where only
>> -         * new connection is possible
>> -         */
>> -        siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
>> -        break;
>> +        if (cep->state == SIW_EPSTATE_CONNECTING) {
>> +            /*
>> +             * handle accepting socket as special case where only
>> +             * new connection is possible
>> +             */
>> +            siw_cm_queue_work(cep, SIW_CM_WORK_CONNECTED);
>> +            break;
>> +
>> +        } else if (cep->state == SIW_EPSTATE_LISTENING) {
>> +            /*
>> +             * handle accepting socket as special case where only
>> +             * new connection is possible
>> +             */
>> +            siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
>> +            break;
>> +        }
>> +        siw_dbg_cep(cep,
>> +                "unexpected socket state %d with cep state %d\n",
>> +                sk->sk_state, cep->state);
>> +        fallthrough;
> 
> Is "faillthrough" a annotation ?
> 

Oh, please ignore this, I just learned this.

Cheng Xu

[PATCH v2 00/14] rdma/siw: implement non-blocking connect

[Stefan Metzmacher]

Hi Bernard,

as written a few month ago, I have a patchset with a lot
of fixes for siw.ko.

As requested I'm only send isolated chunks for easier review.

This is the first chunk adressing deadlocks in siw_connect()

The RDMA application layer expects rdma_connect() to be non-blocking
as the completion is handled via RDMA_CM_EVENT_ESTABLISHED and
other async events. It's not unlikely to hold a lock during
the rdma_connect() call.

Without out this a connection attempt to a non-existing/reachable
server block until the very long tcp timeout hits.
The application layer had no chance to have its own timeout handler
as that would just deadlock with the already blocking rdma_connect().

First rdma_connect() holds id_priv->handler_mutex and deadlocks
rdma_destroy_id().

And iw_cm_connect() called from within rdma_connect() sets
IWCM_F_CONNECT_WAIT during the call to cm_id->device->ops.iw_connect(),
siw_connect() in this case. It means that iw_cm_disconnect()
and iw_destroy_cm_id() will both deadlock waiting for
IWCM_F_CONNECT_WAIT being cleared.

Patch 1: Fixes a refcounting problem

Patches 2-7: Intruduces __siw_cep_terminate_upcall()
making he upcall handling much more consistent handling
more state combinations.

Patches 8-13 are preparation patches to siw_connect()
in order to do the real non-blocking split in Patch 14.

Please have a look.

Thanks!
metze

Fixed issues in v2:
- Include more preparation patches related to __siw_cep_terminate_upcall()
  bases on review from Cheng Xu <chengyou@linux.alibaba.com>

Stefan Metzmacher (14):
  rdma/siw: remove superfluous siw_cep_put() from siw_connect() error
    path
  rdma/siw: make siw_cm_upcall() a noop without valid 'id'
  rdma/siw: split out a __siw_cep_terminate_upcall() function
  rdma/siw: use __siw_cep_terminate_upcall() for indirect
    SIW_CM_WORK_CLOSE_LLP
  rdma/siw: use __siw_cep_terminate_upcall() for SIW_CM_WORK_PEER_CLOSE
  rdma/siw: use __siw_cep_terminate_upcall() for SIW_CM_WORK_MPATIMEOUT
  rdma/siw: handle SIW_EPSTATE_CONNECTING in
    __siw_cep_terminate_upcall()
  rdma/siw: make use of kernel_{bind,connect,listen}()
  rdma/siw: let siw_connect() set AWAIT_MPAREP before
    siw_send_mpareqrep()
  rdma/siw: create a temporary copy of private data
  rdma/siw: use error and out logic at the end of siw_connect()
  rdma/siw: start mpa timer before calling siw_send_mpareqrep()
  rdma/siw: call the blocking kernel_bindconnect() just before
    siw_send_mpareqrep()
  rdma/siw: implement non-blocking connect.

 drivers/infiniband/sw/siw/siw_cm.c | 347 ++++++++++++++++++-----------
 drivers/infiniband/sw/siw/siw_cm.h |   1 +
 2 files changed, 224 insertions(+), 124 deletions(-)

-- 
2.34.1

[PATCH v2 01/14] rdma/siw: remove superfluous siw_cep_put() from siw_connect() error path

[Stefan Metzmacher]

The following change demonstrate the bug:

    --- a/drivers/infiniband/sw/siw/siw_cm.c
    +++ b/drivers/infiniband/sw/siw/siw_cm.c
    @@ -1507,6 +1507,9 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
            if (rv >= 0) {
                    rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
                    if (!rv) {
    +                       rv = -ECONNRESET;
    +                       msleep_interruptible(100);
    +                       goto error;
                            siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
                            siw_cep_set_free(cep);
                            return 0;

That change triggers the WARN_ON() in siw_cep_put().

As there's no siw_cep_get() arround id->add_ref()
I removed the siw_cep_put() following id->rem_ref().

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 17f34d584cd9..a8e546670d05 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1495,7 +1495,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 
 		cep->cm_id = NULL;
 		id->rem_ref(id);
-		siw_cep_put(cep);
 
 		qp->cep = NULL;
 		siw_cep_put(cep);
-- 
2.34.1

[PATCH v2 02/14] rdma/siw: make siw_cm_upcall() a noop without valid 'id'

[Stefan Metzmacher]

This will simplify the callers.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index a8e546670d05..eeb366edba2a 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -322,6 +322,9 @@ static int siw_cm_upcall(struct siw_cep *cep, enum iw_cm_event_type reason,
 	} else {
 		id = cep->cm_id;
 	}
+	if (id == NULL)
+		return status;
+
 	/* Signal IRD and ORD */
 	if (reason == IW_CM_EVENT_ESTABLISHED ||
 	    reason == IW_CM_EVENT_CONNECT_REPLY) {
-- 
2.34.1

[PATCH v2 03/14] rdma/siw: split out a __siw_cep_terminate_upcall() function

[Stefan Metzmacher]

There are multiple places where should have the same logic.
Having one helper function to be used in all places
makes it easier to extended the logic.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 53 ++++++++++++++++++------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index eeb366edba2a..c5ef5de7e84c 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -103,6 +103,37 @@ static void siw_socket_disassoc(struct socket *s)
 	}
 }
 
+/*
+ * The caller needs to deal with siw_cep_set_inuse()
+ * and siw_cep_set_free()
+ */
+static void __siw_cep_terminate_upcall(struct siw_cep *cep,
+				       int reply_status)
+{
+	if (cep->qp && cep->qp->term_info.valid)
+		siw_send_terminate(cep->qp);
+
+	switch (cep->state) {
+	case SIW_EPSTATE_AWAIT_MPAREP:
+		siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY,
+			      reply_status);
+		break;
+
+	case SIW_EPSTATE_RDMA_MODE:
+		siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0);
+		break;
+
+	case SIW_EPSTATE_IDLE:
+	case SIW_EPSTATE_LISTENING:
+	case SIW_EPSTATE_CONNECTING:
+	case SIW_EPSTATE_AWAIT_MPAREQ:
+	case SIW_EPSTATE_RECVD_MPAREQ:
+	case SIW_EPSTATE_CLOSED:
+	default:
+		break;
+	}
+}
+
 static void siw_rtr_data_ready(struct sock *sk)
 {
 	struct siw_cep *cep;
@@ -393,29 +424,9 @@ void siw_qp_cm_drop(struct siw_qp *qp, int schedule)
 		}
 		siw_dbg_cep(cep, "immediate close, state %d\n", cep->state);
 
-		if (qp->term_info.valid)
-			siw_send_terminate(qp);
+		__siw_cep_terminate_upcall(cep, -EINVAL);
 
 		if (cep->cm_id) {
-			switch (cep->state) {
-			case SIW_EPSTATE_AWAIT_MPAREP:
-				siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY,
-					      -EINVAL);
-				break;
-
-			case SIW_EPSTATE_RDMA_MODE:
-				siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0);
-				break;
-
-			case SIW_EPSTATE_IDLE:
-			case SIW_EPSTATE_LISTENING:
-			case SIW_EPSTATE_CONNECTING:
-			case SIW_EPSTATE_AWAIT_MPAREQ:
-			case SIW_EPSTATE_RECVD_MPAREQ:
-			case SIW_EPSTATE_CLOSED:
-			default:
-				break;
-			}
 			cep->cm_id->rem_ref(cep->cm_id);
 			cep->cm_id = NULL;
 			siw_cep_put(cep);
-- 
2.34.1

[PATCH v2 04/14] rdma/siw: use __siw_cep_terminate_upcall() for indirect SIW_CM_WORK_CLOSE_LLP

[Stefan Metzmacher]

Both code paths from siw_qp_cm_drop() should use the same logic.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index c5ef5de7e84c..4387cdf99cf9 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1070,11 +1070,7 @@ static void siw_cm_work_handler(struct work_struct *w)
 		/*
 		 * QP scheduled LLP close
 		 */
-		if (cep->qp && cep->qp->term_info.valid)
-			siw_send_terminate(cep->qp);
-
-		if (cep->cm_id)
-			siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0);
+		__siw_cep_terminate_upcall(cep, -EINVAL);
 
 		release_cep = 1;
 		break;
-- 
2.34.1

[PATCH v2 05/14] rdma/siw: use __siw_cep_terminate_upcall() for SIW_CM_WORK_PEER_CLOSE

[Stefan Metzmacher]

It's easier to have generic logic in just one place.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 89 +++++++++++++++++-------------
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 4387cdf99cf9..3160f3fc4ca8 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -110,26 +110,67 @@ static void siw_socket_disassoc(struct socket *s)
 static void __siw_cep_terminate_upcall(struct siw_cep *cep,
 				       int reply_status)
 {
-	if (cep->qp && cep->qp->term_info.valid)
-		siw_send_terminate(cep->qp);
+	bool suspended = false;
+
+	if (cep->qp) {
+		struct siw_qp *qp = cep->qp;
+
+		if (qp->term_info.valid)
+			siw_send_terminate(qp);
+
+		if (qp->rx_stream.rx_suspend || qp->tx_ctx.tx_suspend)
+			suspended = true;
+	} else {
+		suspended = true;
+	}
 
 	switch (cep->state) {
 	case SIW_EPSTATE_AWAIT_MPAREP:
+		/*
+		 * MPA reply not received, but connection drop,
+		 * or timeout.
+		 */
 		siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY,
 			      reply_status);
 		break;
 
 	case SIW_EPSTATE_RDMA_MODE:
+		/*
+		 * NOTE: IW_CM_EVENT_DISCONNECT is given just
+		 *       to transition IWCM into CLOSING.
+		 */
+		WARN(!suspended, "SIW_EPSTATE_RDMA_MODE called without suspended\n");
+		siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0);
 		siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0);
 		break;
 
+	case SIW_EPSTATE_RECVD_MPAREQ:
+		/*
+		 * Wait for the ulp/CM to call accept/reject
+		 */
+		siw_dbg_cep(cep, "mpa req recvd, wait for ULP\n");
+		WARN(!suspended, "SIW_EPSTATE_RECVD_MPAREQ called without suspended\n");
+		break;
+
+	case SIW_EPSTATE_AWAIT_MPAREQ:
+		/*
+		 * Socket close before MPA request received.
+		 */
+		siw_dbg_cep(cep, "no mpareq: drop listener\n");
+		if (cep->listen_cep)
+			siw_cep_put(cep->listen_cep);
+		cep->listen_cep = NULL;
+		break;
+
 	case SIW_EPSTATE_IDLE:
 	case SIW_EPSTATE_LISTENING:
 	case SIW_EPSTATE_CONNECTING:
-	case SIW_EPSTATE_AWAIT_MPAREQ:
-	case SIW_EPSTATE_RECVD_MPAREQ:
 	case SIW_EPSTATE_CLOSED:
 	default:
+		/*
+		 * for other states there is no connection
+		 * known to the IWCM.
+		 */
 		break;
 	}
 }
@@ -1076,41 +1117,11 @@ static void siw_cm_work_handler(struct work_struct *w)
 		break;
 
 	case SIW_CM_WORK_PEER_CLOSE:
-		if (cep->cm_id) {
-			if (cep->state == SIW_EPSTATE_AWAIT_MPAREP) {
-				/*
-				 * MPA reply not received, but connection drop
-				 */
-				siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY,
-					      -ECONNRESET);
-			} else if (cep->state == SIW_EPSTATE_RDMA_MODE) {
-				/*
-				 * NOTE: IW_CM_EVENT_DISCONNECT is given just
-				 *       to transition IWCM into CLOSING.
-				 */
-				siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0);
-				siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0);
-			}
-			/*
-			 * for other states there is no connection
-			 * known to the IWCM.
-			 */
-		} else {
-			if (cep->state == SIW_EPSTATE_RECVD_MPAREQ) {
-				/*
-				 * Wait for the ulp/CM to call accept/reject
-				 */
-				siw_dbg_cep(cep,
-					    "mpa req recvd, wait for ULP\n");
-			} else if (cep->state == SIW_EPSTATE_AWAIT_MPAREQ) {
-				/*
-				 * Socket close before MPA request received.
-				 */
-				siw_dbg_cep(cep, "no mpareq: drop listener\n");
-				siw_cep_put(cep->listen_cep);
-				cep->listen_cep = NULL;
-			}
-		}
+		/*
+		 * Peer closed the connection: TCP_CLOSE*
+		 */
+		__siw_cep_terminate_upcall(cep, -ECONNRESET);
+
 		release_cep = 1;
 		break;
 
-- 
2.34.1

[PATCH v2 06/14] rdma/siw: use __siw_cep_terminate_upcall() for SIW_CM_WORK_MPATIMEOUT

[Stefan Metzmacher]

It's easier to have generic logic in just one place.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 31 ++++++++----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 3160f3fc4ca8..56c484f85160 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1126,31 +1126,16 @@ static void siw_cm_work_handler(struct work_struct *w)
 		break;
 
 	case SIW_CM_WORK_MPATIMEOUT:
+		/*
+		 * MPA request timed out:
+		 * Hide any partially received private data and signal
+		 * timeout
+		 */
 		cep->mpa_timer = NULL;
+		cep->mpa.hdr.params.pd_len = 0;
+		__siw_cep_terminate_upcall(cep, -ETIMEDOUT);
 
-		if (cep->state == SIW_EPSTATE_AWAIT_MPAREP) {
-			/*
-			 * MPA request timed out:
-			 * Hide any partially received private data and signal
-			 * timeout
-			 */
-			cep->mpa.hdr.params.pd_len = 0;
-
-			if (cep->cm_id)
-				siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY,
-					      -ETIMEDOUT);
-			release_cep = 1;
-
-		} else if (cep->state == SIW_EPSTATE_AWAIT_MPAREQ) {
-			/*
-			 * No MPA request received after peer TCP stream setup.
-			 */
-			if (cep->listen_cep) {
-				siw_cep_put(cep->listen_cep);
-				cep->listen_cep = NULL;
-			}
-			release_cep = 1;
-		}
+		release_cep = 1;
 		break;
 
 	default:
-- 
2.34.1

[PATCH v2 07/14] rdma/siw: handle SIW_EPSTATE_CONNECTING in __siw_cep_terminate_upcall()

[Stefan Metzmacher]

The final patch will implement a non-blocking connect,
which means SIW_CM_WORK_MPATIMEOUT can also happen
during SIW_EPSTATE_CONNECTING.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 56c484f85160..80e1d5b274e7 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -125,6 +125,14 @@ static void __siw_cep_terminate_upcall(struct siw_cep *cep,
 	}
 
 	switch (cep->state) {
+	case SIW_EPSTATE_CONNECTING:
+		/*
+		 * The TCP connect got rejected or timed out.
+		 */
+		siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY,
+			      reply_status);
+		break;
+
 	case SIW_EPSTATE_AWAIT_MPAREP:
 		/*
 		 * MPA reply not received, but connection drop,
@@ -164,7 +172,6 @@ static void __siw_cep_terminate_upcall(struct siw_cep *cep,
 
 	case SIW_EPSTATE_IDLE:
 	case SIW_EPSTATE_LISTENING:
-	case SIW_EPSTATE_CONNECTING:
 	case SIW_EPSTATE_CLOSED:
 	default:
 		/*
-- 
2.34.1

[PATCH v2 08/14] rdma/siw: make use of kernel_{bind,connect,listen}()

[Stefan Metzmacher]

That's nicer than dereferencing socket structures.

This prepares making rdma_connect()/siw_connect() non-blocking
in order to avoid deadlocks in the callers.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 80e1d5b274e7..e8e29ce609b4 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1331,11 +1331,11 @@ static int kernel_bindconnect(struct socket *s, struct sockaddr *laddr,
 			return rv;
 	}
 
-	rv = s->ops->bind(s, laddr, size);
+	rv = kernel_bind(s, laddr, size);
 	if (rv < 0)
 		return rv;
 
-	rv = s->ops->connect(s, raddr, size, flags);
+	rv = kernel_connect(s, raddr, size, flags);
 
 	return rv < 0 ? rv : 0;
 }
@@ -1800,8 +1800,8 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 		if (ipv4_is_zeronet(laddr->sin_addr.s_addr))
 			s->sk->sk_bound_dev_if = sdev->netdev->ifindex;
 
-		rv = s->ops->bind(s, (struct sockaddr *)laddr,
-				  sizeof(struct sockaddr_in));
+		rv = kernel_bind(s, (struct sockaddr *)laddr,
+				 sizeof(struct sockaddr_in));
 	} else {
 		struct sockaddr_in6 *laddr = &to_sockaddr_in6(id->local_addr);
 
@@ -1818,8 +1818,8 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 		if (ipv6_addr_any(&laddr->sin6_addr))
 			s->sk->sk_bound_dev_if = sdev->netdev->ifindex;
 
-		rv = s->ops->bind(s, (struct sockaddr *)laddr,
-				  sizeof(struct sockaddr_in6));
+		rv = kernel_bind(s, (struct sockaddr *)laddr,
+				 sizeof(struct sockaddr_in6));
 	}
 	if (rv) {
 		siw_dbg(id->device, "socket bind error: %d\n", rv);
@@ -1839,7 +1839,7 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 			rv, backlog);
 		goto error;
 	}
-	rv = s->ops->listen(s, backlog);
+	rv = kernel_listen(s, backlog);
 	if (rv) {
 		siw_dbg(id->device, "listen error %d\n", rv);
 		goto error;
-- 
2.34.1

[PATCH v2 09/14] rdma/siw: let siw_connect() set AWAIT_MPAREP before siw_send_mpareqrep()

[Stefan Metzmacher]

There's no real change made in this commit, but it makes the follwing
commits easier to review.

The idea is that we stay in SIW_EPSTATE_CONNECTING as long as
we only deal with tcp and not started the MPA negotiation.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index e8e29ce609b4..c980e5035552 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1438,8 +1438,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	 */
 	siw_cep_socket_assoc(cep, s);
 
-	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
-
 	/*
 	 * Set MPA Request bits: CRC if required, no MPA Markers,
 	 * MPA Rev. according to module parameter 'mpa_version', Key 'Request'.
@@ -1482,6 +1480,8 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	}
 	memcpy(cep->mpa.hdr.key, MPA_KEY_REQ, 16);
 
+	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
+
 	rv = siw_send_mpareqrep(cep, params->private_data, pd_len);
 	/*
 	 * Reset private data.
-- 
2.34.1

[PATCH v2 10/14] rdma/siw: create a temporary copy of private data

[Stefan Metzmacher]

The final patch will implement a non-blocking connect,
which means that siw_connect() will be split into
siw_connect() and siw_connected().

kernel_bindconnect() will be the last action
in siw_connect(), while the MPA negotiation
is deferred to siw_connected().

We should not rely on the callers private data
pointers to be still valid when siw_connected()
is called, so we better create a copy.

Also note that __siw_cep_dealloc() already calls
kfree(cep->mpa.pdata), so we already have the required cleanup
when we'll split out siw_connected() and an error will
prevent siw_connected() being called at all.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index c980e5035552..307494c6707a 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1480,13 +1480,27 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	}
 	memcpy(cep->mpa.hdr.key, MPA_KEY_REQ, 16);
 
+	if (pd_len > 0) {
+		cep->mpa.pdata = kmemdup(params->private_data, pd_len, GFP_KERNEL);
+		if (IS_ERR_OR_NULL(cep->mpa.pdata)) {
+			rv = -ENOMEM;
+			goto error;
+		}
+		cep->mpa.hdr.params.pd_len = pd_len;
+	}
+
 	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
-	rv = siw_send_mpareqrep(cep, params->private_data, pd_len);
+	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
+				cep->mpa.hdr.params.pd_len);
 	/*
 	 * Reset private data.
 	 */
-	cep->mpa.hdr.params.pd_len = 0;
+	if (cep->mpa.hdr.params.pd_len) {
+		cep->mpa.hdr.params.pd_len = 0;
+		kfree(cep->mpa.pdata);
+		cep->mpa.pdata = NULL;
+	}
 
 	if (rv >= 0) {
 		rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
-- 
2.34.1

[PATCH v2 11/14] rdma/siw: use error and out logic at the end of siw_connect()

[Stefan Metzmacher]

This will make the following changes easier.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 307494c6707a..66d90fc77cef 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1502,14 +1502,19 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		cep->mpa.pdata = NULL;
 	}
 
-	if (rv >= 0) {
-		rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
-		if (!rv) {
-			siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
-			siw_cep_set_free(cep);
-			return 0;
-		}
+	if (rv < 0) {
+		goto error;
+	}
+
+	rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
+	if (rv != 0) {
+		goto error;
 	}
+
+	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
+	siw_cep_set_free(cep);
+	return 0;
+
 error:
 	siw_dbg(id->device, "failed: %d\n", rv);
 
-- 
2.34.1

[PATCH v2 12/14] rdma/siw: start mpa timer before calling siw_send_mpareqrep()

[Stefan Metzmacher]

The mpa timer will also span the non-blocking connect
in the final patch.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 66d90fc77cef..279f5acf84d1 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1489,6 +1489,11 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		cep->mpa.hdr.params.pd_len = pd_len;
 	}
 
+	rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
+	if (rv != 0) {
+		goto error;
+	}
+
 	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
 	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
@@ -1506,11 +1511,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		goto error;
 	}
 
-	rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
-	if (rv != 0) {
-		goto error;
-	}
-
 	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
 	siw_cep_set_free(cep);
 	return 0;
@@ -1519,6 +1519,8 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	siw_dbg(id->device, "failed: %d\n", rv);
 
 	if (cep) {
+		siw_cancel_mpatimer(cep);
+
 		siw_socket_disassoc(s);
 		sock_release(s);
 		cep->sock = NULL;
-- 
2.34.1

[PATCH v2 13/14] rdma/siw: call the blocking kernel_bindconnect() just before siw_send_mpareqrep()

[Stefan Metzmacher]

We should build all state before calling kernel_bindconnect().
This will allow us to go async in the final patch.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 279f5acf84d1..74ed2a5a8f47 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1386,18 +1386,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	if (rv < 0)
 		goto error;
 
-	/*
-	 * NOTE: For simplification, connect() is called in blocking
-	 * mode. Might be reconsidered for async connection setup at
-	 * TCP level.
-	 */
-	rv = kernel_bindconnect(s, laddr, raddr, id->afonly);
-	if (rv != 0) {
-		siw_dbg_qp(qp, "kernel_bindconnect: error %d\n", rv);
-		goto error;
-	}
-	if (siw_tcp_nagle == false)
-		tcp_sock_set_nodelay(s->sk);
 	cep = siw_cep_alloc(sdev);
 	if (!cep) {
 		rv = -ENOMEM;
@@ -1494,6 +1482,19 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		goto error;
 	}
 
+	/*
+	 * NOTE: For simplification, connect() is called in blocking
+	 * mode. Might be reconsidered for async connection setup at
+	 * TCP level.
+	 */
+	rv = kernel_bindconnect(s, laddr, raddr, id->afonly);
+	if (rv != 0) {
+		siw_dbg_qp(qp, "kernel_bindconnect: error %d\n", rv);
+		goto error;
+	}
+	if (siw_tcp_nagle == false)
+		tcp_sock_set_nodelay(s->sk);
+
 	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
 	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
-- 
2.34.1

[PATCH v2 14/14] rdma/siw: implement non-blocking connect.

[Stefan Metzmacher]

This is very important in order to prevent deadlocks.

The RDMA application layer expects rdma_connect() to be non-blocking
as the completion is handled via RDMA_CM_EVENT_ESTABLISHED and
other async events. It's not unlikely to hold a lock during
the rdma_connect() call.

Without out this a connection attempt to a non-existing/reachable
server block until the very long tcp timeout hits.
The application layer had no chance to have its own timeout handler
as that would just deadlock with the already blocking rdma_connect().

First rdma_connect() holds id_priv->handler_mutex and deadlocks
rdma_destroy_id().

And iw_cm_connect() called from within rdma_connect() sets
IWCM_F_CONNECT_WAIT during the call to cm_id->device->ops.iw_connect(),
siw_connect() in this case. It means that iw_cm_disconnect()
and iw_destroy_cm_id() will both deadlock waiting for
IWCM_F_CONNECT_WAIT being cleared.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/sw/siw/siw_cm.c | 125 ++++++++++++++++++++++-------
 drivers/infiniband/sw/siw/siw_cm.h |   1 +
 2 files changed, 96 insertions(+), 30 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 74ed2a5a8f47..5b051aaa3fec 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -37,6 +37,7 @@ static void siw_cm_llp_write_space(struct sock *s);
 static void siw_cm_llp_error_report(struct sock *s);
 static int siw_cm_upcall(struct siw_cep *cep, enum iw_cm_event_type reason,
 			 int status);
+static void siw_connected(struct siw_cep *cep);
 
 static void siw_sk_assign_cm_upcalls(struct sock *sk)
 {
@@ -1074,6 +1075,10 @@ static void siw_cm_work_handler(struct work_struct *w)
 		siw_accept_newconn(cep);
 		break;
 
+	case SIW_CM_WORK_CONNECTED:
+		siw_connected(cep);
+		break;
+
 	case SIW_CM_WORK_READ_MPAHDR:
 		if (cep->state == SIW_EPSTATE_AWAIT_MPAREQ) {
 			if (cep->listen_cep) {
@@ -1239,6 +1244,7 @@ static void siw_cm_llp_data_ready(struct sock *sk)
 	switch (cep->state) {
 	case SIW_EPSTATE_RDMA_MODE:
 	case SIW_EPSTATE_LISTENING:
+	case SIW_EPSTATE_CONNECTING:
 		break;
 
 	case SIW_EPSTATE_AWAIT_MPAREQ:
@@ -1292,12 +1298,26 @@ static void siw_cm_llp_state_change(struct sock *sk)
 
 	switch (sk->sk_state) {
 	case TCP_ESTABLISHED:
-		/*
-		 * handle accepting socket as special case where only
-		 * new connection is possible
-		 */
-		siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
-		break;
+		if (cep->state == SIW_EPSTATE_CONNECTING) {
+			/*
+			 * handle accepting socket as special case where only
+			 * new connection is possible
+			 */
+			siw_cm_queue_work(cep, SIW_CM_WORK_CONNECTED);
+			break;
+
+		} else if (cep->state == SIW_EPSTATE_LISTENING) {
+			/*
+			 * handle accepting socket as special case where only
+			 * new connection is possible
+			 */
+			siw_cm_queue_work(cep, SIW_CM_WORK_ACCEPT);
+			break;
+		}
+		siw_dbg_cep(cep,
+			    "unexpected socket state %d with cep state %d\n",
+			    sk->sk_state, cep->state);
+		fallthrough;
 
 	case TCP_CLOSE:
 	case TCP_CLOSE_WAIT:
@@ -1316,7 +1336,7 @@ static void siw_cm_llp_state_change(struct sock *sk)
 static int kernel_bindconnect(struct socket *s, struct sockaddr *laddr,
 			      struct sockaddr *raddr, bool afonly)
 {
-	int rv, flags = 0;
+	int rv;
 	size_t size = laddr->sa_family == AF_INET ?
 		sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6);
 
@@ -1335,7 +1355,18 @@ static int kernel_bindconnect(struct socket *s, struct sockaddr *laddr,
 	if (rv < 0)
 		return rv;
 
-	rv = kernel_connect(s, raddr, size, flags);
+	/*
+	 * Yes, this is really O_NONBLOCK instead of
+	 * SOCK_NONBLOCK.
+	 *
+	 * __sys_connect_file() passes
+	 * sock->file->f_flags | file_flags to
+	 * sock->ops->connect().
+	 *
+	 * Also io_connect() from io_uring forces
+	 * file_flags=O_NONBLOCK to __sys_connect_file().
+	 */
+	rv = kernel_connect(s, raddr, size, O_NONBLOCK);
 
 	return rv < 0 ? rv : 0;
 }
@@ -1482,36 +1513,27 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 		goto error;
 	}
 
-	/*
-	 * NOTE: For simplification, connect() is called in blocking
-	 * mode. Might be reconsidered for async connection setup at
-	 * TCP level.
-	 */
 	rv = kernel_bindconnect(s, laddr, raddr, id->afonly);
+	if (rv == -EINPROGRESS) {
+		siw_dbg_qp(qp, "kernel_bindconnect: EINPROGRESS\n");
+		rv = 0;
+	}
 	if (rv != 0) {
 		siw_dbg_qp(qp, "kernel_bindconnect: error %d\n", rv);
 		goto error;
 	}
-	if (siw_tcp_nagle == false)
-		tcp_sock_set_nodelay(s->sk);
-
-	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
 
-	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
-				cep->mpa.hdr.params.pd_len);
 	/*
-	 * Reset private data.
+	 * The rest will be done by siw_connected()
+	 *
+	 * siw_cm_llp_state_change() will detect
+	 * TCP_ESTABLISHED and schedules SIW_CM_WORK_CONNECTED,
+	 * which will finally call siw_connected().
+	 *
+	 * As siw_cm_llp_state_change() handles everything
+	 * siw_cm_llp_data_ready() can be a noop for
+	 * SIW_EPSTATE_CONNECTING.
 	 */
-	if (cep->mpa.hdr.params.pd_len) {
-		cep->mpa.hdr.params.pd_len = 0;
-		kfree(cep->mpa.pdata);
-		cep->mpa.pdata = NULL;
-	}
-
-	if (rv < 0) {
-		goto error;
-	}
-
 	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
 	siw_cep_set_free(cep);
 	return 0;
@@ -1549,6 +1571,49 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
 	return rv;
 }
 
+static void siw_connected(struct siw_cep *cep)
+{
+	struct siw_qp *qp = cep->qp;
+	struct socket *s = cep->sock;
+	int rv = -ECONNABORTED;
+
+	/*
+	 * already called with
+	 * siw_cep_set_inuse(cep);
+	 */
+
+	if (cep->state != SIW_EPSTATE_CONNECTING)
+		goto error;
+
+	if (siw_tcp_nagle == false)
+		tcp_sock_set_nodelay(s->sk);
+
+	cep->state = SIW_EPSTATE_AWAIT_MPAREP;
+
+	rv = siw_send_mpareqrep(cep, cep->mpa.pdata,
+				cep->mpa.hdr.params.pd_len);
+	/*
+	 * Reset private data.
+	 */
+	if (cep->mpa.hdr.params.pd_len) {
+		cep->mpa.hdr.params.pd_len = 0;
+		kfree(cep->mpa.pdata);
+		cep->mpa.pdata = NULL;
+	}
+
+	if (rv < 0) {
+		goto error;
+	}
+
+	siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
+	return;
+
+error:
+	siw_dbg_cep(cep, "[QP %u]: exit, error %d\n", qp_id(qp), rv);
+	siw_qp_cm_drop(qp, 1);
+	return;
+}
+
 /*
  * siw_accept - Let SoftiWARP accept an RDMA connection request
  *
diff --git a/drivers/infiniband/sw/siw/siw_cm.h b/drivers/infiniband/sw/siw/siw_cm.h
index 8c59cb3e2868..c01bdc8e64ee 100644
--- a/drivers/infiniband/sw/siw/siw_cm.h
+++ b/drivers/infiniband/sw/siw/siw_cm.h
@@ -76,6 +76,7 @@ struct siw_cep {
 
 enum siw_work_type {
 	SIW_CM_WORK_ACCEPT = 1,
+	SIW_CM_WORK_CONNECTED,
 	SIW_CM_WORK_READ_MPAHDR,
 	SIW_CM_WORK_CLOSE_LLP, /* close socket */
 	SIW_CM_WORK_PEER_CLOSE, /* socket indicated peer close */
-- 
2.34.1