Re: [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY

[Stefan Metzmacher <metze@samba.org>]

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