linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY
@ 2026-08-05  0:02 Yunseong Kim
  2026-09-01 12:41 ` Leon Romanovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Yunseong Kim @ 2026-08-05  0:02 UTC (permalink / raw)
  To: Jason Gunthorpe, Leon Romanovsky, Jacob Moroni, Bart Van Assche
  Cc: Stefan Metzmacher, Steve French, Namjae Jeon, Tom Talpey,
	Yunseong Kim, linux-cifs, samba-technical, linux-kernel,
	linux-rdma, Yunseong Kim

After a successful connect downcall, iw_cm_connect() returns with
IWCM_F_CONNECT_WAIT still set and the cm_id in IW_CM_STATE_CONN_SENT.
The only thing that clears the flag is the provider delivering
IW_CM_EVENT_CONNECT_REPLY (cm_conn_rep_handler()).  Until that event
arrives, iw_cm_disconnect() and destroy_cm_id() sleep uninterruptibly
in

	wait_event(cm_id_priv->connect_wait,
		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags));

and both state machines treat IW_CM_STATE_CONN_SENT as BUG(), so the
API has no way to cancel a pending active connect.  If the provider
never generates the reply, because the peer died in the middle of
connection setup or because of a provider bug, every teardown path
(rdma_disconnect(), rdma_destroy_id()) blocks in D state forever and
the ULP cannot recover: there is no way to disconnect after
rdma_connect() was called without risking an unbounded hang.

This class of problem is not theoretical.  The pending smbdirect
change "smb: smbdirect: bound the disconnect wait in destroy_sync" [1]
had to work around it on the ULP side:
smbdirect_socket_destroy_sync() waited unbounded for the socket to
reach SMBDIRECT_SOCKET_DISCONNECTED, a transition that depends on an
asynchronous RDMA CM disconnect event, and when the peer died abruptly
(a killed client, or Soft-RoCE/RXE where no graceful disconnect
completes) that event never arrived.  The destroy ran on the single
ksmbd-conn-release workqueue, every later connection release queued
behind it in D state, and the whole server wedged until hung_task
fired.  That change bounded the wait and drove the socket state
machine to DISCONNECTED locally on timeout; ULPs should not have to
resort to that, the CM should offer a teardown they can rely on.

IWCM_F_CONNECT_WAIT currently guards two different windows:

 * the connect/accept downcall into the provider being in progress;
   teardown must keep waiting for that, it is short and bounded;

 * an issued active connect waiting for CONNECT_REPLY, which is
   potentially unbounded.

Mark the second window with a new flag, IWCM_F_CONNECT_SENT: set by
iw_cm_connect() once the downcall has returned successfully, cleared
by cm_conn_rep_handler().  The teardown waits now complete when either
the downcall has finished (!IWCM_F_CONNECT_WAIT, as before) or the
pending-reply window has been entered (IWCM_F_CONNECT_SENT), and the
previously BUG() CONN_SENT cases become:

 * iw_cm_disconnect(): return -ENOTCONN; there is no established
   connection to disconnect, aborting is the destroy path's job;

 * destroy_cm_id(): abort the pending connect locally by moving to
   DESTROYING and putting the QP into error so the provider tears the
   connection attempt down.

A CONNECT_REPLY that arrives after the abort is dropped: either
cm_work_handler() sees IWCM_F_DROP_EVENTS, or cm_conn_rep_handler()
now recognizes IW_CM_STATE_DESTROYING (the abort and the reply
serialize on cm_id_priv->lock) and frees the event without touching
the QP that destroy_cm_id() already released.  The cm_id memory stays
valid for such a late reply because the provider holds its own
reference (cm_id->add_ref) for as long as it can deliver events.

The passive side has a sibling gap, where after a successful accept
downcall the flag stays set until the provider's ESTABLISHED event
arrives, which this patch deliberately does not change.

[1] https://github.com/smfrench/smb3-kernel/commit/26d0f82a02c8a9c9c8cdfc138acb7ed0bf8e01a9

Suggested-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
---
 drivers/infiniband/core/iwcm.c | 76 +++++++++++++++++++++++++++++-----
 drivers/infiniband/core/iwcm.h |  1 +
 2 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index 0b7246ec559e..4fb36ac92cc5 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -308,9 +308,18 @@ int iw_cm_disconnect(struct iw_cm_id *cm_id, int abrupt)
 	struct ib_qp *qp = NULL;
 
 	cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);
-	/* Wait if we're currently in a connect or accept downcall */
+	/*
+	 * Wait if we're currently in a connect or accept downcall.  A
+	 * pending active connect whose downcall already returned
+	 * (IWCM_F_CONNECT_SENT) is not waited for: the CONNECT_REPLY that
+	 * would end such a wait comes from the provider and may never
+	 * arrive if the peer died during connection setup, so the
+	 * CONN_SENT state is handled below instead of sleeping without
+	 * bound here.
+	 */
 	wait_event(cm_id_priv->connect_wait,
-		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags));
+		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags) ||
+		   test_bit(IWCM_F_CONNECT_SENT, &cm_id_priv->flags));
 
 	spin_lock_irqsave(&cm_id_priv->lock, flags);
 	switch (cm_id_priv->state) {
@@ -338,7 +347,14 @@ int iw_cm_disconnect(struct iw_cm_id *cm_id, int abrupt)
 		 */
 		break;
 	case IW_CM_STATE_CONN_SENT:
-		/* Can only get here if wait above fails */
+		/*
+		 * Active connect still waiting for the provider's
+		 * CONNECT_REPLY: there is no established connection to
+		 * disconnect.  Tell the caller; aborting the pending
+		 * connect is iw_destroy_cm_id()'s job.
+		 */
+		ret = -ENOTCONN;
+		break;
 	default:
 		BUG();
 	}
@@ -375,10 +391,15 @@ static void destroy_cm_id(struct iw_cm_id *cm_id)
 	cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);
 	/*
 	 * Wait if we're currently in a connect or accept downcall. A
-	 * listening endpoint should never block here.
+	 * listening endpoint should never block here.  A pending active
+	 * connect whose downcall already returned (IWCM_F_CONNECT_SENT)
+	 * is not waited for, since its CONNECT_REPLY may never arrive if
+	 * the peer died during connection setup; it is aborted locally
+	 * in the CONN_SENT case below.
 	 */
 	wait_event(cm_id_priv->connect_wait,
-		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags));
+		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags) ||
+		   test_bit(IWCM_F_CONNECT_SENT, &cm_id_priv->flags));
 
 	/*
 	 * Since we're deleting the cm_id, drop any events that
@@ -422,6 +443,21 @@ static void destroy_cm_id(struct iw_cm_id *cm_id)
 		spin_lock_irqsave(&cm_id_priv->lock, flags);
 		break;
 	case IW_CM_STATE_CONN_SENT:
+		/*
+		 * Abort a pending active connect: the connect downcall has
+		 * returned (IWCM_F_CONNECT_SENT) but the provider has not
+		 * delivered CONNECT_REPLY.  Move the QP to error so the
+		 * provider tears the connection attempt down.  A late
+		 * CONNECT_REPLY is dropped via IWCM_F_DROP_EVENTS or the
+		 * DESTROYING check in cm_conn_rep_handler(), and the
+		 * provider's own reference (cm_id->add_ref) keeps this
+		 * cm_id alive until that reply has been delivered.
+		 */
+		cm_id_priv->state = IW_CM_STATE_DESTROYING;
+		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
+		(void)iwcm_modify_qp_err(qp);
+		spin_lock_irqsave(&cm_id_priv->lock, flags);
+		break;
 	case IW_CM_STATE_DESTROYING:
 	default:
 		BUG();
@@ -689,9 +725,11 @@ EXPORT_SYMBOL(iw_cm_accept);
 /*
  * Active Side: CM_ID <-- CONN_SENT
  *
- * If successful, results in the generation of a CONNECT_REPLY
- * event. iw_cm_disconnect and iw_cm_destroy will block until the
- * CONNECT_REPLY event is received from the provider.
+ * If successful, results in the generation of a CONNECT_REPLY event.
+ * IWCM_F_CONNECT_SENT marks the window between the connect downcall
+ * returning and that CONNECT_REPLY arriving; during it
+ * iw_cm_disconnect() returns -ENOTCONN and iw_destroy_cm_id() aborts
+ * the pending connect locally instead of blocking on the provider.
  */
 int iw_cm_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *iw_param)
 {
@@ -728,8 +766,16 @@ int iw_cm_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *iw_param)
 	ret = iw_cm_map(cm_id, true);
 	if (!ret)
 		ret = cm_id->device->ops.iw_connect(cm_id, iw_param);
-	if (!ret)
+	if (!ret) {
+		/*
+		 * The downcall is done; only the provider's CONNECT_REPLY
+		 * is outstanding.  Let teardown waiters proceed so they
+		 * can abort instead of depending on that reply.
+		 */
+		set_bit(IWCM_F_CONNECT_SENT, &cm_id_priv->flags);
+		wake_up_all(&cm_id_priv->connect_wait);
 		return 0;	/* success */
+	}
 
 	spin_lock_irqsave(&cm_id_priv->lock, flags);
 	qp = cm_id_priv->qp;
@@ -882,7 +928,7 @@ static int cm_conn_rep_handler(struct iwcm_id_private *cm_id_priv,
 {
 	struct ib_qp *qp = NULL;
 	unsigned long flags;
-	int ret;
+	int ret = 0;
 
 	spin_lock_irqsave(&cm_id_priv->lock, flags);
 	/*
@@ -890,6 +936,15 @@ static int cm_conn_rep_handler(struct iwcm_id_private *cm_id_priv,
 	 * iw_cm_disconnect will not wait and deadlock this thread
 	 */
 	clear_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags);
+	clear_bit(IWCM_F_CONNECT_SENT, &cm_id_priv->flags);
+	if (cm_id_priv->state == IW_CM_STATE_DESTROYING) {
+		/*
+		 * destroy_cm_id() aborted the pending connect and already
+		 * released the QP; drop the late reply.
+		 */
+		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
+		goto out;
+	}
 	BUG_ON(cm_id_priv->state != IW_CM_STATE_CONN_SENT);
 	if (iw_event->status == 0) {
 		cm_id_priv->id.m_local_addr = iw_event->local_addr;
@@ -908,6 +963,7 @@ static int cm_conn_rep_handler(struct iwcm_id_private *cm_id_priv,
 		cm_id_priv->id.device->ops.iw_rem_ref(qp);
 	ret = cm_id_priv->id.cm_handler(&cm_id_priv->id, iw_event);
 
+out:
 	if (iw_event->private_data_len)
 		kfree(iw_event->private_data);
 
diff --git a/drivers/infiniband/core/iwcm.h b/drivers/infiniband/core/iwcm.h
index b56fb12edece..74ce33616863 100644
--- a/drivers/infiniband/core/iwcm.h
+++ b/drivers/infiniband/core/iwcm.h
@@ -57,5 +57,6 @@ struct iwcm_id_private {
 
 #define IWCM_F_DROP_EVENTS	  1
 #define IWCM_F_CONNECT_WAIT       2
+#define IWCM_F_CONNECT_SENT       3
 
 #endif /* IWCM_H */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY
  2026-08-05  0:02 [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY Yunseong Kim
@ 2026-09-01 12:41 ` Leon Romanovsky
  2026-09-01 16:50   ` Stefan Metzmacher
  0 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2026-09-01 12:41 UTC (permalink / raw)
  To: Yunseong Kim, Stefan Metzmacher
  Cc: Jason Gunthorpe, Jacob Moroni, Bart Van Assche, Steve French,
	Namjae Jeon, Tom Talpey, Yunseong Kim, linux-cifs,
	samba-technical, linux-kernel, linux-rdma

On Wed, Aug 05, 2026 at 02:02:00AM +0200, Yunseong Kim wrote:
> After a successful connect downcall, iw_cm_connect() returns with
> IWCM_F_CONNECT_WAIT still set and the cm_id in IW_CM_STATE_CONN_SENT.
> The only thing that clears the flag is the provider delivering
> IW_CM_EVENT_CONNECT_REPLY (cm_conn_rep_handler()).  Until that event
> arrives, iw_cm_disconnect() and destroy_cm_id() sleep uninterruptibly
> in
> 
> 	wait_event(cm_id_priv->connect_wait,
> 		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags));
> 
> and both state machines treat IW_CM_STATE_CONN_SENT as BUG(), so the
> API has no way to cancel a pending active connect.  If the provider
> never generates the reply, because the peer died in the middle of
> connection setup or because of a provider bug, every teardown path
> (rdma_disconnect(), rdma_destroy_id()) blocks in D state forever and
> the ULP cannot recover: there is no way to disconnect after
> rdma_connect() was called without risking an unbounded hang.
> 
> This class of problem is not theoretical.  The pending smbdirect
> change "smb: smbdirect: bound the disconnect wait in destroy_sync" [1]
> had to work around it on the ULP side:
> smbdirect_socket_destroy_sync() waited unbounded for the socket to
> reach SMBDIRECT_SOCKET_DISCONNECTED, a transition that depends on an
> asynchronous RDMA CM disconnect event, and when the peer died abruptly
> (a killed client, or Soft-RoCE/RXE where no graceful disconnect
> completes) that event never arrived.  The destroy ran on the single
> ksmbd-conn-release workqueue, every later connection release queued
> behind it in D state, and the whole server wedged until hung_task
> fired.  That change bounded the wait and drove the socket state
> machine to DISCONNECTED locally on timeout; ULPs should not have to
> resort to that, the CM should offer a teardown they can rely on.
> 
> IWCM_F_CONNECT_WAIT currently guards two different windows:
> 
>  * the connect/accept downcall into the provider being in progress;
>    teardown must keep waiting for that, it is short and bounded;
> 
>  * an issued active connect waiting for CONNECT_REPLY, which is
>    potentially unbounded.
> 
> Mark the second window with a new flag, IWCM_F_CONNECT_SENT: set by
> iw_cm_connect() once the downcall has returned successfully, cleared
> by cm_conn_rep_handler().  The teardown waits now complete when either
> the downcall has finished (!IWCM_F_CONNECT_WAIT, as before) or the
> pending-reply window has been entered (IWCM_F_CONNECT_SENT), and the
> previously BUG() CONN_SENT cases become:
> 
>  * iw_cm_disconnect(): return -ENOTCONN; there is no established
>    connection to disconnect, aborting is the destroy path's job;
> 
>  * destroy_cm_id(): abort the pending connect locally by moving to
>    DESTROYING and putting the QP into error so the provider tears the
>    connection attempt down.
> 
> A CONNECT_REPLY that arrives after the abort is dropped: either
> cm_work_handler() sees IWCM_F_DROP_EVENTS, or cm_conn_rep_handler()
> now recognizes IW_CM_STATE_DESTROYING (the abort and the reply
> serialize on cm_id_priv->lock) and frees the event without touching
> the QP that destroy_cm_id() already released.  The cm_id memory stays
> valid for such a late reply because the provider holds its own
> reference (cm_id->add_ref) for as long as it can deliver events.
> 
> The passive side has a sibling gap, where after a successful accept
> downcall the flag stays set until the provider's ESTABLISHED event
> arrives, which this patch deliberately does not change.
> 
> [1] https://github.com/smfrench/smb3-kernel/commit/26d0f82a02c8a9c9c8cdfc138acb7ed0bf8e01a9
> 
> Suggested-by: Stefan Metzmacher <metze@samba.org>
> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
> ---
>  drivers/infiniband/core/iwcm.c | 76 +++++++++++++++++++++++++++++-----
>  drivers/infiniband/core/iwcm.h |  1 +
>  2 files changed, 67 insertions(+), 10 deletions(-)

I'm not sure what to do with this patch, as the iWARP folks have
remained silent.

Stefan,

Do you still need this patch? Have you tested it?

Thanks,

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY
  2026-09-01 12:41 ` Leon Romanovsky
@ 2026-09-01 16:50   ` Stefan Metzmacher
  2026-09-02  7:15     ` Leon Romanovsky
  2026-09-03 10:42     ` Bernard Metzler
  0 siblings, 2 replies; 5+ messages in thread
From: Stefan Metzmacher @ 2026-09-01 16:50 UTC (permalink / raw)
  To: Leon Romanovsky, Yunseong Kim
  Cc: Jason Gunthorpe, Jacob Moroni, Bart Van Assche, Namjae Jeon,
	Tom Talpey, Yunseong Kim, linux-cifs, samba-technical,
	linux-kernel, linux-rdma, Bernard Metzler

Hi Leon,

> On Wed, Aug 05, 2026 at 02:02:00AM +0200, Yunseong Kim wrote:
>> After a successful connect downcall, iw_cm_connect() returns with
>> IWCM_F_CONNECT_WAIT still set and the cm_id in IW_CM_STATE_CONN_SENT.
>> The only thing that clears the flag is the provider delivering
>> IW_CM_EVENT_CONNECT_REPLY (cm_conn_rep_handler()).  Until that event
>> arrives, iw_cm_disconnect() and destroy_cm_id() sleep uninterruptibly
>> in
>>
>> 	wait_event(cm_id_priv->connect_wait,
>> 		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags));
>>
>> and both state machines treat IW_CM_STATE_CONN_SENT as BUG(), so the
>> API has no way to cancel a pending active connect.  If the provider
>> never generates the reply, because the peer died in the middle of
>> connection setup or because of a provider bug, every teardown path
>> (rdma_disconnect(), rdma_destroy_id()) blocks in D state forever and
>> the ULP cannot recover: there is no way to disconnect after
>> rdma_connect() was called without risking an unbounded hang.
>>
>> This class of problem is not theoretical.  The pending smbdirect
>> change "smb: smbdirect: bound the disconnect wait in destroy_sync" [1]
>> had to work around it on the ULP side:
>> smbdirect_socket_destroy_sync() waited unbounded for the socket to
>> reach SMBDIRECT_SOCKET_DISCONNECTED, a transition that depends on an
>> asynchronous RDMA CM disconnect event, and when the peer died abruptly
>> (a killed client, or Soft-RoCE/RXE where no graceful disconnect
>> completes) that event never arrived.  The destroy ran on the single
>> ksmbd-conn-release workqueue, every later connection release queued
>> behind it in D state, and the whole server wedged until hung_task
>> fired.  That change bounded the wait and drove the socket state
>> machine to DISCONNECTED locally on timeout; ULPs should not have to
>> resort to that, the CM should offer a teardown they can rely on.
>>
>> IWCM_F_CONNECT_WAIT currently guards two different windows:
>>
>>   * the connect/accept downcall into the provider being in progress;
>>     teardown must keep waiting for that, it is short and bounded;
>>
>>   * an issued active connect waiting for CONNECT_REPLY, which is
>>     potentially unbounded.
>>
>> Mark the second window with a new flag, IWCM_F_CONNECT_SENT: set by
>> iw_cm_connect() once the downcall has returned successfully, cleared
>> by cm_conn_rep_handler().  The teardown waits now complete when either
>> the downcall has finished (!IWCM_F_CONNECT_WAIT, as before) or the
>> pending-reply window has been entered (IWCM_F_CONNECT_SENT), and the
>> previously BUG() CONN_SENT cases become:
>>
>>   * iw_cm_disconnect(): return -ENOTCONN; there is no established
>>     connection to disconnect, aborting is the destroy path's job;
>>
>>   * destroy_cm_id(): abort the pending connect locally by moving to
>>     DESTROYING and putting the QP into error so the provider tears the
>>     connection attempt down.
>>
>> A CONNECT_REPLY that arrives after the abort is dropped: either
>> cm_work_handler() sees IWCM_F_DROP_EVENTS, or cm_conn_rep_handler()
>> now recognizes IW_CM_STATE_DESTROYING (the abort and the reply
>> serialize on cm_id_priv->lock) and frees the event without touching
>> the QP that destroy_cm_id() already released.  The cm_id memory stays
>> valid for such a late reply because the provider holds its own
>> reference (cm_id->add_ref) for as long as it can deliver events.
>>
>> The passive side has a sibling gap, where after a successful accept
>> downcall the flag stays set until the provider's ESTABLISHED event
>> arrives, which this patch deliberately does not change.
>>
>> [1] https://github.com/smfrench/smb3-kernel/commit/26d0f82a02c8a9c9c8cdfc138acb7ed0bf8e01a9
>>
>> Suggested-by: Stefan Metzmacher <metze@samba.org>
>> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
>> ---
>>   drivers/infiniband/core/iwcm.c | 76 +++++++++++++++++++++++++++++-----
>>   drivers/infiniband/core/iwcm.h |  1 +
>>   2 files changed, 67 insertions(+), 10 deletions(-)
> 
> I'm not sure what to do with this patch, as the iWARP folks have
> remained silent.
> 
> Stefan,
> 
> Do you still need this patch? Have you tested it?

I'll test it soon.

But the problem is real and I hit it very often in the past and the only option
was a reboot.

I added Bernard explicitly...

metze


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY
  2026-09-01 16:50   ` Stefan Metzmacher
@ 2026-09-02  7:15     ` Leon Romanovsky
  2026-09-03 10:42     ` Bernard Metzler
  1 sibling, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2026-09-02  7:15 UTC (permalink / raw)
  To: Stefan Metzmacher
  Cc: Yunseong Kim, Jason Gunthorpe, Jacob Moroni, Bart Van Assche,
	Namjae Jeon, Tom Talpey, Yunseong Kim, linux-cifs,
	samba-technical, linux-kernel, linux-rdma, Bernard Metzler

On Tue, Sep 01, 2026 at 06:50:15PM +0200, Stefan Metzmacher wrote:
> Hi Leon,
> 
> > On Wed, Aug 05, 2026 at 02:02:00AM +0200, Yunseong Kim wrote:
> > > After a successful connect downcall, iw_cm_connect() returns with
> > > IWCM_F_CONNECT_WAIT still set and the cm_id in IW_CM_STATE_CONN_SENT.
> > > The only thing that clears the flag is the provider delivering
> > > IW_CM_EVENT_CONNECT_REPLY (cm_conn_rep_handler()).  Until that event
> > > arrives, iw_cm_disconnect() and destroy_cm_id() sleep uninterruptibly
> > > in
> > > 
> > > 	wait_event(cm_id_priv->connect_wait,
> > > 		   !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags));
> > > 
> > > and both state machines treat IW_CM_STATE_CONN_SENT as BUG(), so the
> > > API has no way to cancel a pending active connect.  If the provider
> > > never generates the reply, because the peer died in the middle of
> > > connection setup or because of a provider bug, every teardown path
> > > (rdma_disconnect(), rdma_destroy_id()) blocks in D state forever and
> > > the ULP cannot recover: there is no way to disconnect after
> > > rdma_connect() was called without risking an unbounded hang.
> > > 
> > > This class of problem is not theoretical.  The pending smbdirect
> > > change "smb: smbdirect: bound the disconnect wait in destroy_sync" [1]
> > > had to work around it on the ULP side:
> > > smbdirect_socket_destroy_sync() waited unbounded for the socket to
> > > reach SMBDIRECT_SOCKET_DISCONNECTED, a transition that depends on an
> > > asynchronous RDMA CM disconnect event, and when the peer died abruptly
> > > (a killed client, or Soft-RoCE/RXE where no graceful disconnect
> > > completes) that event never arrived.  The destroy ran on the single
> > > ksmbd-conn-release workqueue, every later connection release queued
> > > behind it in D state, and the whole server wedged until hung_task
> > > fired.  That change bounded the wait and drove the socket state
> > > machine to DISCONNECTED locally on timeout; ULPs should not have to
> > > resort to that, the CM should offer a teardown they can rely on.
> > > 
> > > IWCM_F_CONNECT_WAIT currently guards two different windows:
> > > 
> > >   * the connect/accept downcall into the provider being in progress;
> > >     teardown must keep waiting for that, it is short and bounded;
> > > 
> > >   * an issued active connect waiting for CONNECT_REPLY, which is
> > >     potentially unbounded.
> > > 
> > > Mark the second window with a new flag, IWCM_F_CONNECT_SENT: set by
> > > iw_cm_connect() once the downcall has returned successfully, cleared
> > > by cm_conn_rep_handler().  The teardown waits now complete when either
> > > the downcall has finished (!IWCM_F_CONNECT_WAIT, as before) or the
> > > pending-reply window has been entered (IWCM_F_CONNECT_SENT), and the
> > > previously BUG() CONN_SENT cases become:
> > > 
> > >   * iw_cm_disconnect(): return -ENOTCONN; there is no established
> > >     connection to disconnect, aborting is the destroy path's job;
> > > 
> > >   * destroy_cm_id(): abort the pending connect locally by moving to
> > >     DESTROYING and putting the QP into error so the provider tears the
> > >     connection attempt down.
> > > 
> > > A CONNECT_REPLY that arrives after the abort is dropped: either
> > > cm_work_handler() sees IWCM_F_DROP_EVENTS, or cm_conn_rep_handler()
> > > now recognizes IW_CM_STATE_DESTROYING (the abort and the reply
> > > serialize on cm_id_priv->lock) and frees the event without touching
> > > the QP that destroy_cm_id() already released.  The cm_id memory stays
> > > valid for such a late reply because the provider holds its own
> > > reference (cm_id->add_ref) for as long as it can deliver events.
> > > 
> > > The passive side has a sibling gap, where after a successful accept
> > > downcall the flag stays set until the provider's ESTABLISHED event
> > > arrives, which this patch deliberately does not change.
> > > 
> > > [1] https://github.com/smfrench/smb3-kernel/commit/26d0f82a02c8a9c9c8cdfc138acb7ed0bf8e01a9
> > > 
> > > Suggested-by: Stefan Metzmacher <metze@samba.org>
> > > Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
> > > ---
> > >   drivers/infiniband/core/iwcm.c | 76 +++++++++++++++++++++++++++++-----
> > >   drivers/infiniband/core/iwcm.h |  1 +
> > >   2 files changed, 67 insertions(+), 10 deletions(-)
> > 
> > I'm not sure what to do with this patch, as the iWARP folks have
> > remained silent.
> > 
> > Stefan,
> > 
> > Do you still need this patch? Have you tested it?
> 
> I'll test it soon.
> 
> But the problem is real and I hit it very often in the past and the only option
> was a reboot.

I see. I'll wait for a tag (Acked-by, Reviewed-by, Tested-by, etc.) from
either you or Bernard.

Thanks

> 
> I added Bernard explicitly...
> 
> metze
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY
  2026-09-01 16:50   ` Stefan Metzmacher
  2026-09-02  7:15     ` Leon Romanovsky
@ 2026-09-03 10:42     ` Bernard Metzler
  1 sibling, 0 replies; 5+ messages in thread
From: Bernard Metzler @ 2026-09-03 10:42 UTC (permalink / raw)
  To: Stefan Metzmacher, Leon Romanovsky, Yunseong Kim
  Cc: Jason Gunthorpe, Jacob Moroni, Bart Van Assche, Namjae Jeon,
	Tom Talpey, Yunseong Kim, linux-cifs, samba-technical,
	linux-kernel, linux-rdma

On 01.09.2026 18:50, Stefan Metzmacher wrote:
> Hi Leon,
> 
>> On Wed, Aug 05, 2026 at 02:02:00AM +0200, Yunseong Kim wrote:
>>> After a successful connect downcall, iw_cm_connect() returns with
>>> IWCM_F_CONNECT_WAIT still set and the cm_id in IW_CM_STATE_CONN_SENT.
>>> The only thing that clears the flag is the provider delivering
>>> IW_CM_EVENT_CONNECT_REPLY (cm_conn_rep_handler()).  Until that event
>>> arrives, iw_cm_disconnect() and destroy_cm_id() sleep uninterruptibly
>>> in
>>>
>>>     wait_event(cm_id_priv->connect_wait,
>>>            !test_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags));
>>>
>>> and both state machines treat IW_CM_STATE_CONN_SENT as BUG(), so the
>>> API has no way to cancel a pending active connect.  If the provider
>>> never generates the reply, because the peer died in the middle of
>>> connection setup or because of a provider bug, every teardown path
>>> (rdma_disconnect(), rdma_destroy_id()) blocks in D state forever and
>>> the ULP cannot recover: there is no way to disconnect after
>>> rdma_connect() was called without risking an unbounded hang.
>>>
>>> This class of problem is not theoretical.  The pending smbdirect
>>> change "smb: smbdirect: bound the disconnect wait in destroy_sync" [1]
>>> had to work around it on the ULP side:
>>> smbdirect_socket_destroy_sync() waited unbounded for the socket to
>>> reach SMBDIRECT_SOCKET_DISCONNECTED, a transition that depends on an
>>> asynchronous RDMA CM disconnect event, and when the peer died abruptly
>>> (a killed client, or Soft-RoCE/RXE where no graceful disconnect
>>> completes) that event never arrived.  The destroy ran on the single
>>> ksmbd-conn-release workqueue, every later connection release queued
>>> behind it in D state, and the whole server wedged until hung_task
>>> fired.  That change bounded the wait and drove the socket state
>>> machine to DISCONNECTED locally on timeout; ULPs should not have to
>>> resort to that, the CM should offer a teardown they can rely on.
>>>
>>> IWCM_F_CONNECT_WAIT currently guards two different windows:
>>>
>>>   * the connect/accept downcall into the provider being in progress;
>>>     teardown must keep waiting for that, it is short and bounded;
>>>
>>>   * an issued active connect waiting for CONNECT_REPLY, which is
>>>     potentially unbounded.
>>>
>>> Mark the second window with a new flag, IWCM_F_CONNECT_SENT: set by
>>> iw_cm_connect() once the downcall has returned successfully, cleared
>>> by cm_conn_rep_handler().  The teardown waits now complete when either
>>> the downcall has finished (!IWCM_F_CONNECT_WAIT, as before) or the
>>> pending-reply window has been entered (IWCM_F_CONNECT_SENT), and the
>>> previously BUG() CONN_SENT cases become:
>>>
>>>   * iw_cm_disconnect(): return -ENOTCONN; there is no established
>>>     connection to disconnect, aborting is the destroy path's job;
>>>
>>>   * destroy_cm_id(): abort the pending connect locally by moving to
>>>     DESTROYING and putting the QP into error so the provider tears the
>>>     connection attempt down.
>>>
>>> A CONNECT_REPLY that arrives after the abort is dropped: either
>>> cm_work_handler() sees IWCM_F_DROP_EVENTS, or cm_conn_rep_handler()
>>> now recognizes IW_CM_STATE_DESTROYING (the abort and the reply
>>> serialize on cm_id_priv->lock) and frees the event without touching
>>> the QP that destroy_cm_id() already released.  The cm_id memory stays
>>> valid for such a late reply because the provider holds its own
>>> reference (cm_id->add_ref) for as long as it can deliver events.
>>>
>>> The passive side has a sibling gap, where after a successful accept
>>> downcall the flag stays set until the provider's ESTABLISHED event
>>> arrives, which this patch deliberately does not change.
>>>
>>> [1] https://github.com/smfrench/smb3-kernel/commit/26d0f82a02c8a9c9c8cdfc138acb7ed0bf8e01a9
>>>
>>> Suggested-by: Stefan Metzmacher <metze@samba.org>
>>> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
>>> ---
>>>   drivers/infiniband/core/iwcm.c | 76 +++++++++++++++++++++++++++++-----
>>>   drivers/infiniband/core/iwcm.h |  1 +
>>>   2 files changed, 67 insertions(+), 10 deletions(-)
>>
>> I'm not sure what to do with this patch, as the iWARP folks have
>> remained silent.
>>
>> Stefan,
>>
>> Do you still need this patch? Have you tested it?
> 
> I'll test it soon.
> 
> But the problem is real and I hit it very often in the past and the only option
> was a reboot.
> 
> I added Bernard explicitly...
> 
> metze
> 
I follow Yunseong's and Stefan's argumentation.

Thanks,
Bernard.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-03 10:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  0:02 [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY Yunseong Kim
2026-09-01 12:41 ` Leon Romanovsky
2026-09-01 16:50   ` Stefan Metzmacher
2026-09-02  7:15     ` Leon Romanovsky
2026-09-03 10:42     ` Bernard Metzler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).