Re: [rds-devel] [External] : Re: [PATCH net] rds: ib: use rds_conn_drop() on protocol version mismatch

Hi,

On 2026-09-09 00:09, Allison Henderson via rds-devel wrote:

On Tue, 2026-09-08 at 20: 33 +0800, henrymei wrote: > From: Aohan Mei <henrymei@ tencent. com> > > rds_ib_cm_connect_complete() runs from the RDMA-CM event handler with > conn->c_cm_lock held. When the peer negotiates a protocol
On Tue, 2026-09-08 at 20:33 +0800, henrymei wrote:

quoted

From: Aohan Mei <redacted>

rds_ib_cm_connect_complete() runs from the RDMA-CM event handler with
conn->c_cm_lock held.  When the peer negotiates a protocol version
older than RDS_PROTOCOL_COMPAT_VERSION, the handler calls
rds_conn_destroy(), which is only safe in the rmmod path: it
synchronously tears the connection down and flush_work()es the
shutdown work cp_down_w.

That shutdown work (rds_conn_shutdown()) needs cp_cm_lock, which is
the very lock the event handler still holds, so the flush never
completes: the two workers wait on each other and the RDS connection
workqueues stall for good.

All other RDMA-CM failure paths (REJECTED, CONNECT_ERROR,
DISCONNECTED) use rds_conn_drop(), which marks the connection
RDS_CONN_ERROR and schedules the shutdown work asynchronously.  Use
it here as well.

I don't get this.

So RDS should just keep on retrying to connect every second instead
of giving up on that incompatible peer? Forever?

The version number the peer uses remains the same until it gets upgraded somehow.

I understand how the rds_conn_destroy() path is full of bugs.

But shouldn't that just mean those bugs ought to be fixed,
instead of no longer retiring any connection ever,
even to old & incompatible peers we can't really talk to?

Thanks,

  Gerd

quoted

Fixes: cdc306a5c9cd ("rds: make v3.1 as compat version")

Hi Aohan,

Thanks for the catch, this looks correct to me.  Just one nit on the
fixes tag.  I think we want: f147dd9ecabf ("RDS/IB: Disallow connections
less than RDS 3.1").  That looks like the commit that added the
rds_conn_destroy() call to the version check.  Other than that, I think
this patch looks good.  With the Fixes tag corrected, you can add my rvb:

Reviewed-by: Allison Henderson <achender@kernel.org>

Thanks!
Allison

quoted

Reported-by: TencentOS Corvus AI <redacted>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <redacted>
---
 net/rds/ib_cm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index d46146887ba4..2909da8363f3 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c

@@ -115,7 +115,7 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
 				  &conn->c_laddr, &conn->c_faddr,
 				  RDS_PROTOCOL_MAJOR(conn->c_version),
 				  RDS_PROTOCOL_MINOR(conn->c_version));
-			rds_conn_destroy(conn);
+			rds_conn_drop(conn);
 			return;
 		}
 	}

_______________________________________________
rds-devel mailing list
rds-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/rds-devel