[ofa-general] [PATCH 2/5][v2.0] dapl: fixes for IB UD extensions in common code and socket cm provider.

Arlin Davis arlin.r.davis at intel.com
Wed Sep 24 13:44:01 PDT 2008


 - Manage EP states base on attribute service type.
 - Allow multiple connections (remote_ah resolution)
   and accepts on UD type endpoints.
 - Supply private data on CR conn establishment
 - Add UD extension conn event type - DAT_IB_UD_PASSIVE_REMOTE_AH

Signed-off by: Arlin Davis ardavis at ichips.intel.com
---
 dapl/common/dapl_cr_accept.c     |    8 +++--
 dapl/common/dapl_ep_connect.c    |    3 +-
 dapl/common/dapl_ep_disconnect.c |    3 +-
 dapl/openib_scm/dapl_ib_cm.c     |   37 ++++++++++------------
 dapl/openib_scm/dapl_ib_qp.c     |   63 ++++++++++++++++++++++++-------------
 5 files changed, 67 insertions(+), 47 deletions(-)

diff --git a/dapl/common/dapl_cr_accept.c b/dapl/common/dapl_cr_accept.c
index e221fdc..1059e0c 100644
--- a/dapl/common/dapl_cr_accept.c
+++ b/dapl/common/dapl_cr_accept.c
@@ -123,15 +123,17 @@ dapl_cr_accept (
     if ( ep_handle == NULL )
     {
 	 ep_handle = cr_ptr->param.local_ep_handle;
-	if ( (((DAPL_EP *) ep_handle)->param.ep_state != DAT_EP_STATE_TENTATIVE_CONNECTION_PENDING)
-	     && (((DAPL_EP *)ep_handle)->param.ep_state != DAT_EP_STATE_PASSIVE_CONNECTION_PENDING)
)
+	if ( ((((DAPL_EP *) ep_handle)->param.ep_state != DAT_EP_STATE_TENTATIVE_CONNECTION_PENDING)
+	     && (((DAPL_EP *)ep_handle)->param.ep_state != DAT_EP_STATE_PASSIVE_CONNECTION_PENDING))
&&
+	     (((DAPL_EP *)ep_handle)->param.ep_attr.service_type == DAT_SERVICE_TYPE_RC))
 	{
 	    return DAT_INVALID_STATE;
 	}
     } else
     {
 	 /* ensure this EP isn't connected or in use*/
-	if ( ((DAPL_EP *) ep_handle)->param.ep_state != DAT_EP_STATE_UNCONNECTED )
+	if ( (((DAPL_EP *)ep_handle)->param.ep_state != DAT_EP_STATE_UNCONNECTED) &&
+	     (((DAPL_EP *)ep_handle)->param.ep_attr.service_type == DAT_SERVICE_TYPE_RC))
 	{
 	    return DAT_INVALID_STATE;
 	}
diff --git a/dapl/common/dapl_ep_connect.c b/dapl/common/dapl_ep_connect.c
index f290ebe..0c3f10a 100755
--- a/dapl/common/dapl_ep_connect.c
+++ b/dapl/common/dapl_ep_connect.c
@@ -234,7 +234,8 @@ dapl_ep_connect (
 	}
     }
 
-    if ( ep_ptr->param.ep_state != DAT_EP_STATE_UNCONNECTED )
+    if (ep_ptr->param.ep_state != DAT_EP_STATE_UNCONNECTED &&
+	ep_ptr->param.ep_attr.service_type == DAT_SERVICE_TYPE_RC)
     {
 	dapl_os_unlock ( &ep_ptr->header.lock );
 	dat_status = DAT_ERROR (DAT_INVALID_STATE, dapls_ep_state_subtype (ep_ptr));
diff --git a/dapl/common/dapl_ep_disconnect.c b/dapl/common/dapl_ep_disconnect.c
index 0c1dd38..fabce92 100644
--- a/dapl/common/dapl_ep_disconnect.c
+++ b/dapl/common/dapl_ep_disconnect.c
@@ -95,7 +95,8 @@ dapl_ep_disconnect (
     dapl_os_lock ( &ep_ptr->header.lock );
 
     /* Disconnecting a disconnected EP is a no-op. */
-    if ( ep_ptr->param.ep_state == DAT_EP_STATE_DISCONNECTED )
+    if (ep_ptr->param.ep_state == DAT_EP_STATE_DISCONNECTED ||
+	ep_ptr->param.ep_attr.service_type != DAT_SERVICE_TYPE_RC)
     {
 	dapl_os_unlock ( &ep_ptr->header.lock );
 	dat_status = DAT_SUCCESS;
diff --git a/dapl/openib_scm/dapl_ib_cm.c b/dapl/openib_scm/dapl_ib_cm.c
index cf5891d..5a6aa97 100644
--- a/dapl/openib_scm/dapl_ib_cm.c
+++ b/dapl/openib_scm/dapl_ib_cm.c
@@ -108,7 +108,7 @@ static void dapli_cm_destroy(struct ib_cm_handle *cm_ptr)
 
 	dapl_os_lock(&cm_ptr->lock);
 	cm_ptr->state = SCM_DESTROY;
-	if (cm_ptr->ep) 
+	if ((cm_ptr->ep) && (cm_ptr->ep->cm_handle == cm_ptr))
 		cm_ptr->ep->cm_handle = IB_INVALID_HANDLE;
 
 	/* close socket if still active */
@@ -185,18 +185,20 @@ dapli_socket_disconnect(dp_ib_cm_handle_t	cm_ptr)
 	}
 	dapl_os_unlock(&cm_ptr->lock);
 
-
-	if (ep_ptr->cr_ptr) {
-		dapls_cr_callback(cm_ptr,
-				  IB_CME_DISCONNECTED,
-				  NULL,
-				  ((DAPL_CR *)ep_ptr->cr_ptr)->sp_ptr);
-	} else {
-		dapl_evd_connection_callback(ep_ptr->cm_handle,
-					     IB_CME_DISCONNECTED,
-					     NULL,
-					     ep_ptr);
-	}	
+	/* disconnect events for RC's only */
+	if (ep_ptr->param.ep_attr.service_type == DAT_SERVICE_TYPE_RC) {
+		if (ep_ptr->cr_ptr) {
+			dapls_cr_callback(cm_ptr,
+					IB_CME_DISCONNECTED,
+					NULL,
+					((DAPL_CR *)ep_ptr->cr_ptr)->sp_ptr);
+		} else {
+			dapl_evd_connection_callback(ep_ptr->cm_handle,
+						IB_CME_DISCONNECTED,
+						NULL,
+						ep_ptr);
+		}	
+	}
 
 	/* scheduled destroy via disconnect clean in callback */
 	return DAT_SUCCESS;
@@ -477,7 +479,6 @@ dapli_socket_connect_rtu(dp_ib_cm_handle_t	cm_ptr)
 			   ep_ptr->param.remote_ia_address_ptr)->sin_addr)); 
 		goto bail;
 	}
-
 	if (dapls_modify_qp_state(ep_ptr->qp_handle, 
 				  IBV_QPS_RTS, cm_ptr) != DAT_SUCCESS) {
 		dapl_log(DAPL_DBG_TYPE_ERR, 
@@ -487,9 +488,6 @@ dapli_socket_connect_rtu(dp_ib_cm_handle_t	cm_ptr)
 			   ep_ptr->param.remote_ia_address_ptr)->sin_addr)); 
 		goto bail;
 	}
-		 
-	ep_ptr->qp_state = IB_QP_STATE_RTS;
-
 	dapl_dbg_log(DAPL_DBG_TYPE_EP," connect_rtu: send RTU\n"); 
 
 	/* complete handshake after final QP state change */
@@ -801,7 +799,6 @@ dapli_socket_accept_usr(DAPL_EP		*ep_ptr,
 				&cm_ptr->dst.ia_address)->sin_addr)); 
 		goto bail;
 	}
-	ep_ptr->qp_state = IB_QP_STATE_RTS;
 	
 	/* save remote address information */
 	dapl_os_memcpy( &ep_ptr->remote_ia_address, 
@@ -911,7 +908,7 @@ dapli_socket_accept_rtu(dp_ib_cm_handle_t	cm_ptr)
 
 		/* post EVENT, modify_qp created ah */
 		xevent.status = 0;
-		xevent.type = DAT_IB_UD_REMOTE_AH;
+		xevent.type = DAT_IB_UD_PASSIVE_REMOTE_AH;
 		xevent.remote_ah.ah = cm_ptr->ah;
 		xevent.remote_ah.qpn = cm_ptr->dst.qpn;
 		dapl_os_memcpy( &xevent.remote_ah.ia_addr,
@@ -1529,7 +1526,7 @@ void cr_thread(void *arg)
 			else
 				dapli_socket_connected(cr,errno);
 		} else {
-			dapl_log(DAPL_DBG_TYPE_WARN,
+			dapl_log(DAPL_DBG_TYPE_CM,
 				 " CM poll ERR, wrong state(%d) -> %s SKIP\n",
 				 cr->state,
 				 inet_ntoa(((struct sockaddr_in*)
diff --git a/dapl/openib_scm/dapl_ib_qp.c b/dapl/openib_scm/dapl_ib_qp.c
index 4fae307..9c8a881 100644
--- a/dapl/openib_scm/dapl_ib_qp.c
+++ b/dapl/openib_scm/dapl_ib_qp.c
@@ -156,7 +156,6 @@ dapls_ib_qp_alloc (
 		return DAT_INTERNAL_ERROR;
 	}
 
-	ep_ptr->qp_state = IB_QP_STATE_INIT;
 	return DAT_SUCCESS;
 }
 
@@ -193,7 +192,6 @@ dapls_ib_qp_free (
 			return(dapl_convert_errno(errno,"destroy_qp"));
 
 		ep_ptr->qp_handle = IB_INVALID_HANDLE;
-		ep_ptr->qp_state = IB_QP_STATE_ERROR;
 	}
 
 	return DAT_SUCCESS;
@@ -241,7 +239,6 @@ dapls_ib_qp_modify (
 	/* move to error state if necessary */
 	if ((ep_ptr->qp_state == IB_QP_STATE_ERROR) &&
 	    (ep_ptr->qp_handle->state != IBV_QPS_ERR)) {
-		ep_ptr->qp_state = IB_QP_STATE_ERROR;
 		return (dapls_modify_qp_state(ep_ptr->qp_handle, 
 					      IBV_QPS_ERR, NULL));
 	}
@@ -295,16 +292,17 @@ void
 dapls_ib_reinit_ep (
 	IN  DAPL_EP	*ep_ptr)
 {
-	if ( ep_ptr->qp_handle != IB_INVALID_HANDLE ) {
+	if (ep_ptr->qp_handle != IB_INVALID_HANDLE &&
+	    ep_ptr->qp_handle->qp_type != IBV_QPT_UD) {
 		/* move to RESET state and then to INIT */
 		dapls_modify_qp_state(ep_ptr->qp_handle, IBV_QPS_RESET, 0);
 		dapls_modify_qp_state(ep_ptr->qp_handle, IBV_QPS_INIT, 0);
-		ep_ptr->qp_state = IB_QP_STATE_INIT;
 	}
 }
 
 /* 
  * Generic QP modify for init, reset, error, RTS, RTR
+ * For UD, create_ah on RTR, qkey on INIT
  */
 DAT_RETURN
 dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
@@ -316,20 +314,21 @@ dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
 	DAPL_EP			*ep_ptr = (DAPL_EP*)qp_handle->qp_context;
 	DAPL_IA			*ia_ptr = ep_ptr->header.owner_ia;
 	ib_qp_cm_t		*qp_cm = &cm_ptr->dst;
+	int			ret;
 			
 	dapl_os_memzero((void*)&qp_attr, sizeof(qp_attr));
 	qp_attr.qp_state = qp_state;
-
 	switch (qp_state) {
 		/* additional attributes with RTR and RTS */
 		case IBV_QPS_RTR:
 		{
 			dapl_dbg_log(DAPL_DBG_TYPE_EP,
-				     " QPS_RTR: type %d qpn %x lid %x"
-				     " port %x\n",
-			             qp_handle->qp_type,
-				     qp_cm->qpn, qp_cm->lid, qp_cm->port);
-
+				     " QPS_RTR: type %d state %d qpn %x lid %x"
+				     " port %x ep %p qp_state %d\n",
+			             qp_handle->qp_type, qp_handle->qp_type,
+				     qp_cm->qpn, qp_cm->lid, qp_cm->port, 
+				     ep_ptr, ep_ptr->qp_state);
+			
 			mask |= IBV_QP_AV                 |
 				IBV_QP_PATH_MTU           |
 				IBV_QP_DEST_QPN           |
@@ -337,7 +336,6 @@ dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
 				IBV_QP_MAX_DEST_RD_ATOMIC |
 				IBV_QP_MIN_RNR_TIMER;
 
-			qp_attr.qp_state = IBV_QPS_RTR;
 			qp_attr.dest_qp_num = qp_cm->qpn;
 			qp_attr.rq_psn = 1;
 			qp_attr.path_mtu = 
@@ -346,8 +344,8 @@ dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
 				ep_ptr->param.ep_attr.max_rdma_read_out;
 			qp_attr.min_rnr_timer =
 				ia_ptr->hca_ptr->ib_trans.rnr_timer;
-			
-			/* address handle */
+						
+			/* address handle. RC and UD */
 			qp_attr.ah_attr.dlid = qp_cm->lid;
 			if (ia_ptr->hca_ptr->ib_trans.global) {
 				qp_attr.ah_attr.is_global = 1;
@@ -372,19 +370,24 @@ dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
 				if (!cm_ptr->ah)
 					return(dapl_convert_errno(errno,
 								  "ibv_ah"));
+				
+				/* already RTR, multi remote AH's on QP */
+				if (ep_ptr->qp_state == IBV_QPS_RTR ||
+				    ep_ptr->qp_state == IBV_QPS_RTS)
+					return DAT_SUCCESS;
 			}
 #endif			
 			break;
 		}		
 		case IBV_QPS_RTS: 
 		{
-			mask |= IBV_QP_SQ_PSN;
+			/* RC only */
 			if (qp_handle->qp_type == IBV_QPT_RC) {
-				mask |= IBV_QP_TIMEOUT            |
+				mask |= IBV_QP_SQ_PSN             |
+					IBV_QP_TIMEOUT            |
 					IBV_QP_RETRY_CNT          |
 					IBV_QP_RNR_RETRY          |
 					IBV_QP_MAX_QP_RD_ATOMIC;
-
 				qp_attr.timeout	= 
 				    ia_ptr->hca_ptr->ib_trans.ack_timer;
 				qp_attr.retry_cnt = 
@@ -394,15 +397,25 @@ dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
 				qp_attr.max_rd_atomic = 
 				    ep_ptr->param.ep_attr.max_rdma_read_out;
 			}
+			/* RC and UD */
 			qp_attr.qp_state	= IBV_QPS_RTS;
 			qp_attr.sq_psn		= 1;
 
 			dapl_dbg_log(DAPL_DBG_TYPE_EP,
 				" QPS_RTS: psn %x rd_atomic %d ack %d "
-				" retry %d rnr_retry %d\n",
+				" retry %d rnr_retry %d ep %p qp_state %d\n",
 				qp_attr.sq_psn, qp_attr.max_rd_atomic, 
 				qp_attr.timeout, qp_attr.retry_cnt, 
-				qp_attr.rnr_retry );
+				qp_attr.rnr_retry, ep_ptr, ep_ptr->qp_state);
+#ifdef DAT_EXTENSIONS
+			if (qp_handle->qp_type == IBV_QPT_UD) {
+				/* already RTS, multi remote AH's on QP */
+				if (ep_ptr->qp_state == IBV_QPS_RTS)
+					return DAT_SUCCESS;
+				else
+					mask = IBV_QP_STATE | IBV_QP_SQ_PSN;
+			}
+#endif
 			break;
 		}
 		case IBV_QPS_INIT: 
@@ -419,6 +432,9 @@ dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
 			}
 #ifdef DAT_EXTENSIONS
 			if (qp_handle->qp_type == IBV_QPT_UD) {
+				/* already INIT, multi remote AH's on QP */
+				if (ep_ptr->qp_state == IBV_QPS_INIT)
+					return DAT_SUCCESS;
 				mask |= IBV_QP_QKEY;
 				qp_attr.qkey = SCM_UD_QKEY;
 			}
@@ -437,10 +453,13 @@ dapls_modify_qp_state ( IN ib_qp_handle_t	qp_handle,
 		
 	}
 
-	if (ibv_modify_qp(qp_handle, &qp_attr, mask))
+	ret = ibv_modify_qp(qp_handle, &qp_attr, mask);
+	if (ret == 0) {
+		ep_ptr->qp_state = qp_state;
+		return DAT_SUCCESS;
+	} else {
 		return(dapl_convert_errno(errno,"modify_qp_state"));
-	
-	return DAT_SUCCESS;
+	}
 }
 
 /*
-- 
1.5.2.5





More information about the general mailing list