[openib-general] [PATCH] [RFC] RDMA generic CMA updates

[James Lentini]

On Wed, 21 Sep 2005, Sean Hefty wrote:

> Here's the updated implementation.  It compiles, but that's it.

Sean,

Overall this looks very good. 

A few comments:

Why would this module be a ULP and not part of the core? Especially 
since the rdma_cma.h include file is intended for the core include 
area, include/rdma. 

I expect that the IB_CM_REQ_RECEIVED callback will be confusing to 
ULPs. The ULP will receive a new cma_id with an old context value. If 
the ULP wanted to make an adjustments to the cma_id that received the 
request, it would need to store a reference to it in the old cma_id's 
context value. I suggest you make the new cma_id part of the event 
data (see patch below).

In cma_get_service_id: I assume that the IB_OPENIB_OUI will be 
replaced by an IBTA OUI when this address resolution mechanism is 
standardized.

How will the IP address and port number in the connection request be 
delivered to the ULP?

Would the design be cleaner if instead of sprinkling the code with "switch 
(cma_id->device->node_type)" statements, we used function pointers?

Here's a patch that: 

- move listen declaration closer to accept and reject
- add private data and new cma_id fields to event structure
- record need to address information in the event structure
- implement private data handling for IB_CM_REQ_RECEIVED and 
  IB_CM_REP_RECEIVED
- on white space fix for rdma_cma_reject

Signed-off-by: James Lentini <jlentini at netapp.com>

Index: ulp/cma/cma.c
===================================================================
--- ulp/cma/cma.c	(revision 3541)
+++ ulp/cma/cma.c	(working copy)
@@ -177,8 +177,6 @@
 	if (!route->path_rec)
 		goto err;
 
-	ib_event->private_data += sizeof *addr;
-
 	route->path_rec[0] = *ib_event->param.req_rcvd.primary_path;
 	if (route->num_paths == 2)
 		route->path_rec[1] = *ib_event->param.req_rcvd.alternate_path;
@@ -229,7 +227,7 @@
 
 static int cma_ib_handler(struct ib_cm_id *cm_id, struct ib_cm_event *ib_event)
 {
-	struct cma_id_private *cma_id_priv;
+	struct cma_id_private *cma_id_priv, new_cma_id_priv;
 	struct rdma_cma_event event;
 
 	cma_id_priv = cm_id->context;
@@ -240,13 +238,22 @@
 		event.event = RDMA_CMA_EVENT_UNREACHABLE;
 		break;
 	case IB_CM_REQ_RECEIVED:
-		cma_id_priv = cma_req_recv(cma_id_priv, ib_event);
-		if (!cma_id_priv)
+		new_cma_id_priv = cma_req_recv(cma_id_priv, ib_event);
+		if (!new_cma_id_priv)
 			return -ENOMEM;
 		event.event = RDMA_CMA_EVENT_CONNECT_REQUEST;
+		event.private_data = ib_event->private_data + 
+				     sizeof struct cma_addr;
+		event.private_data_len = IB_CM_REQ_PRIVATE_DATA_SIZE - 
+					 sizeof struct cma_addr;
+		event.new_cma_id = new_cma_id_priv->cma_id;
 		break;
 	case IB_CM_REP_RECEIVED:
 		event.event = cma_rep_recv(cma_id_priv);
+		event.private_data = ib_event->private_data + 
+				     sizeof struct cma_addr;
+		event.private_data_len = IB_CM_REQ_PRIVATE_DATA_SIZE - 
+					 sizeof struct cma_addr;
 		break;
 	case IB_CM_RTU_RECEIVED:
 		event.event = cma_rtu_recv(cma_id_priv);
@@ -606,8 +613,8 @@
 }
 EXPORT_SYMBOL(rdma_cma_accept);
 
-int rdma_cma_reject(struct rdma_cma_id *cma_id,
-		    const void *private_data, u8 private_data_len)
+int rdma_cma_reject(struct rdma_cma_id *cma_id, const void *private_data, 
+		    u8 private_data_len)
 {
 	struct cma_id_private *cma_id_priv;
 	int ret;
Index: include/rdma/rdma_cma.h
===================================================================
--- include/rdma/rdma_cma.h	(revision 3541)
+++ include/rdma/rdma_cma.h	(working copy)
@@ -53,13 +53,20 @@
 	int num_paths;
 };
 
+struct rdma_cma_id;
+
 struct rdma_cma_event {
 	enum rdma_cma_event_type event;
+	/* for RDMA_CMA_EVENT_CONNECT_REQUEST and */
+	/* active side RDMA_CMA_EVENT_ESTABLISHED */
 	void			*private_data;
+	u8 			private_data_len;
+	/* for RDMA_CMA_EVENT_CONNECT_REQUEST */
+	rdma_cma_id		*new_cma_id;
+
+	/* TODO need to add the RDMA_CMA_EVENT_CONNECT_REQUEST's IP and port */
 };
 
-struct rdma_cma_id;
-
 typedef void (*rdma_cma_event_handler)(struct rdma_cma_id *cma_id,
 				       struct rdma_cma_event *event);
 
@@ -76,12 +83,6 @@
 
 void rdma_cma_destroy_id(struct rdma_cma_id *cma_id);
 
-/**
- * rdma_cma_listen - this function is called by the passive side. It is
- *   listening on a the specified port for incomming connection requests.
- */
-int rdma_cma_listen(struct rdma_cma_id *cma_id, struct sockaddr *addr);
-
 int rdma_cma_resolve_route(struct rdma_cma_id *cma_id,
 			   struct sockaddr *src_addr,
 			   struct sockaddr *dst_addr);
@@ -109,6 +110,13 @@
 		     struct rdma_cma_conn_param *conn_param);
 
 /**
+ * rdma_cma_listen - this function is called by the passive side. It is
+ *   listening on a the specified port for incoming connection requests.
+ */
+int rdma_cma_listen(struct rdma_cma_id *cma_id, struct sockaddr *addr);
+
+
+/**
  * rdma_cma_accept - call on the passive side to accept a connection request
  *   note that if the function returned with error - a reject message was
  *   sent to the remote side and the cma_id was destroyed.