Skip to contentSkip to conversation

svcrdma: Limit ORD based on client's advertised IRD

PATCH In review Tom Tucker 2 participants 2 messages May 19, 2008 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore

Tom Tucker

author
#0 lore raw

PATCH 1/1 svcrdma: Limit ORD based on client's advertised IRD

svcrdma: Limit ORD based on client's advertised IRD

When adapters have differing IRD limits, the RDMA transport will fail to connect properly. The RDMA transport should use the client's advertised inbound read limit when computing its outbound read limit. For iWARP transports, there is currently no standard for exchanging IRD/ORD during connection establishment so the 'responder_resources' field in the connect event is the local device's limit. The RDMA transport can be configured to use a smaller ORD by writing the desired number to the /proc/sys/sunrpc/svc_rdma/max_outbound_read_requests file.

---
Re-posted per Tom's suggestions. There was also a cast missing in the min macro that caused a warning.

 net/sunrpc/xprtrdma/svc_rdma_transport.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)
net/sunrpc/xprtrdma/svc_rdma_transport.c +13 -3
index 68908b5..e1ae326 100644
@@ -580,7 +580,7 @@ int svc_rdma_post_recv(struct svcxprt_rdma *xprt)
  * will call the recvfrom method on the listen xprt which will accept the new
  * connection.
  */
-static void handle_connect_req(struct rdma_cm_id *new_cma_id)
+static void handle_connect_req(struct rdma_cm_id *new_cma_id, size_t client_ird)
 {
 	struct svcxprt_rdma *listen_xprt = new_cma_id->context;
 	struct svcxprt_rdma *newxprt;
@@ -597,6 +597,9 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id)
 	dprintk("svcrdma: Creating newxprt=%p, cm_id=%p, listenxprt=%p\n",
 		newxprt, newxprt->sc_cm_id, listen_xprt);
 
+	/* Save client advertised inbound read limit for use later in accept. */
+	newxprt->sc_ord = client_ird;
+
 	/* Set the local and remote addresses in the transport */
 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
 	svc_xprt_set_remote(&newxprt->sc_xprt, sa, svc_addr_len(sa));
@@ -633,7 +636,8 @@ static int rdma_listen_handler(struct rdma_cm_id *cma_id,
 	case RDMA_CM_EVENT_CONNECT_REQUEST:
 		dprintk("svcrdma: Connect request on cma_id=%p, xprt = %p, "
 			"event=%d\n", cma_id, cma_id->context, event->event);
-		handle_connect_req(cma_id);
+		handle_connect_req(cma_id,
+				   event->param.conn.responder_resources);
 		break;
 
 	case RDMA_CM_EVENT_ESTABLISHED:
@@ -807,8 +811,14 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
 				   (size_t)svcrdma_max_requests);
 	newxprt->sc_sq_depth = RPCRDMA_SQ_DEPTH_MULT * newxprt->sc_max_requests;
 
+	/* 
+	 * Limit ORD based on client limit, local device limit, and
+	 * configured svcrdma limit.
+	 */
 	newxprt->sc_ord =  min((size_t)devattr.max_qp_rd_atom,
-			       (size_t)svcrdma_ord);
+			       (size_t)newxprt->sc_ord);
+	newxprt->sc_ord = min((size_t)svcrdma_ord,
+			      (size_t)newxprt->sc_ord);
 
 	newxprt->sc_pd = ib_alloc_pd(newxprt->sc_cm_id->device);
 	if (IS_ERR(newxprt->sc_pd)) {

A message that is not in the archive (sent off-list or not yet received): 1211224653.31725.62.camel-SMNkleLxa3ZimH42XvhXlA@public.gmane.org

#2 lore raw

net/sunrpc/xprtrdma/svc_rdma_transport.c int svc_rdma_post_recv(struct svcxprt_rdma *xprt) · line 583

Quoted 2 lines · Tom Tucker
net/sunrpc/xprtrdma/svc_rdma_transport.c +1 -1
-static void handle_connect_req(struct rdma_cm_id *new_cma_id)
+static void handle_connect_req(struct rdma_cm_id *new_cma_id, size_t client_ird)

Wow! That'll do it - 4 billion RDMA reads here we come!

Looks good to me with one tiny suggestion:

net/sunrpc/xprtrdma/svc_rdma_transport.c static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt) · line 821

Quoted 6 lines
	newxprt->sc_ord =  min((size_t)devattr.max_qp_rd_atom,
net/sunrpc/xprtrdma/svc_rdma_transport.c +3 -1
-			       (size_t)svcrdma_ord);
+			       (size_t)newxprt->sc_ord);
+	newxprt->sc_ord = min((size_t)svcrdma_ord,
+			      (size_t)newxprt->sc_ord);

You can code these a tiny bit more cleanly as min_t(size_t, foo, bar) and thereby avoid all four casts.

Tom.