[ofa-general] Re: [PATCH] iw cm conn sent destroy

[Pete Wyckoff]

swise at opengridcomputing.com wrote on Thu, 03 Jul 2008 15:44 -0500:
> Pete Wyckoff wrote:
>> sean.hefty at intel.com wrote on Thu, 03 Jul 2008 13:03 -0700:
>>   
>>>> That makes sense.  I couldn't find anyway to get that bit
>>>> IWCM_F_CONNECT_WAIT cleared (without massive layering violations).
>>>> Is there anything like rdma_give_up_on_the_connect() that works
>>>> on the client side?  Am I expected just to wait for the transport
>>>> layer to time out the connect?  I could imagine a Ctrl-C situation
>>>> where immediate client-side cancellation is desired.
>>>>       
>>> From the perspective of the rdma_cm interface, rdma_destroy_id() is
>>> intended to provide this functionality.
>>>     
>>
>> That is a sensible approach.  The implementation in iwcm.c just
>> passively waits for a connection to time out before allowing
>> rdma_destroy_id() to complete.  Maybe I should look at adding a case
>> in cma_cancel_operation() for CMA_CONNECT that calls into a
>> device-specific function that can actively quit the connection.
>>
>> Or should this be a special case in iwcm's destroy_cm_id()?  Where
>> a CONN_SENT state will bypass the check on IWCM_F_CONNECT_WAIT, and
>> call into a device function that tries to cancel the connection
>> attepmt first?
>
> If we define a method that every iwarp provider supports to cancel a  
> connect attempt, then we can do away with the IWCM_F_CONNECT_WAIT bit  
> altogether and always cancel the operation instead.  For cxgb3, the  
> connect request to the HW really cannot be canceled.  So the driver  
> would have to keep its endpoint struct around until the HW finally posts  
> the connect attempt results, but mark the endpoint as "no cm_id  
> attached".  The if the connection succeeds, the driver will immediately  
> reset it I guess...

This defines an optional iwcm method that cancels the connect
operation if the device supports that.  Otherwise it will wait
on the CONNECT_WAIT bit as usual.  Doing it like this does not require
changes in existing devices.  It would not be pretty to implement
a mandatory connect_cancel() as you describe for cxgb3.

(I dropped the first-attempt patch in this thread.)

		-- Pete


iwcm: connect cancel

Add an interface to cancel a connection attempt.  Used on the client side
of rdmacm to cancel a connection, for devices that support such an
operation.  This can avoid waiting for the device to timeout the operation
itself.

Signed-off-by: Pete Wyckoff <pw at osc.edu>
---
 drivers/infiniband/core/iwcm.c |   14 ++++++++++++++
 include/rdma/iw_cm.h           |    2 ++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index 81c9195..56aa0d6 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -327,6 +327,20 @@ static void destroy_cm_id(struct iw_cm_id *cm_id)
 	int ret;
 
 	cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);
+
+	/*
+	 * Give the device a chance to cancel the connect, if possible.
+	 * Else we wait until it times out.
+	 */
+	if (cm_id_priv->state == IW_CM_STATE_CONN_SENT &&
+	    cm_id->device->iwcm->connect_cancel) {
+		ret = cm_id->device->iwcm->connect_cancel(cm_id);
+		if (ret == 0) {
+			clear_bit(IWCM_F_CONNECT_WAIT, &cm_id_priv->flags);
+			cm_id_priv->state = IW_CM_STATE_IDLE;
+		}
+	}
+
 	/*
 	 * Wait if we're currently in a connect or accept downcall. A
 	 * listening endpoint should never block here.
diff --git a/include/rdma/iw_cm.h b/include/rdma/iw_cm.h
index aeefa9b..046690c 100644
--- a/include/rdma/iw_cm.h
+++ b/include/rdma/iw_cm.h
@@ -119,6 +119,8 @@ struct iw_cm_verbs {
 	int		(*connect)(struct iw_cm_id *cm_id,
 				   struct iw_cm_conn_param *conn_param);
 
+	int		(*connect_cancel)(struct iw_cm_id *cm_id);
+
 	int		(*accept)(struct iw_cm_id *cm_id,
 				  struct iw_cm_conn_param *conn_param);
 
-- 
1.5.5.1