mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Bernard Metzler <bernard.metzler@linux.dev>
To: Stefan Metzmacher <metze@samba.org>,
	Leon Romanovsky <leon@kernel.org>,
	Yunseong Kim <yunseong.kim@est.tech>
Cc: Jason Gunthorpe <jgg@ziepe.ca>, Jacob Moroni <jmoroni@google.com>,
	Bart Van Assche <bvanassche@acm.org>,
	Namjae Jeon <linkinjeon@kernel.org>, Tom Talpey <tom@talpey.com>,
	Yunseong Kim <yunseong.kim@ericsson.com>,
	linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
	linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org
Subject: Re: [RFC PATCH] RDMA/iwcm: allow aborting an active connect awaiting CONNECT_REPLY
Date: Thu, 3 Sep 2026 12:42:09 +0200	[thread overview]
Message-ID: <46f83a1c-02ed-4c64-9eeb-2fc435fdfb24@linux.dev> (raw)
In-Reply-To: <47142579-3af7-453b-875c-ff6d610f26a7@samba.org>

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.

      parent reply	other threads:[~2026-09-03 10:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05  0:02 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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46f83a1c-02ed-4c64-9eeb-2fc435fdfb24@linux.dev \
    --to=bernard.metzler@linux.dev \
    --cc=bvanassche@acm.org \
    --cc=jgg@ziepe.ca \
    --cc=jmoroni@google.com \
    --cc=leon@kernel.org \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=metze@samba.org \
    --cc=samba-technical@lists.samba.org \
    --cc=tom@talpey.com \
    --cc=yunseong.kim@ericsson.com \
    --cc=yunseong.kim@est.tech \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®