[openib-general] RE: [PATCH v2] [RFC] - example user mode rdma ping/pongprogramusing CMA

[Sean Hefty]

>I like what you did on making it more modular! I don't see any real
>functional changes, but you removed the simple private data exchange.
>Any particular reason?

I didn't see that the private data was being checked.  I dropped it from one of
the function calls when I was merging some of the calls together.  It's probably
not that hard to add back in.

>> +static int rping_create_qp(struct rping_cb *cb)
>> +{
>> +	struct ibv_qp_init_attr init_attr;
>> + //       struct ibv_qp_attr qp_attr;
>> +	int ret;
>> +
>> +	memset(&init_attr, 0, sizeof(init_attr));
>> +	init_attr.cap.max_send_wr = RPING_SQ_DEPTH;
>> +	init_attr.cap.max_recv_wr = 2;
>> +	init_attr.cap.max_recv_sge = 1;
>> +	init_attr.cap.max_send_sge = 1;
>> +	init_attr.qp_type = IBV_QPT_RC;
>> +	init_attr.send_cq = cb->cq;
>> +	init_attr.recv_cq = cb->cq;
>> +
>> +	if (cb->server) {
>> +		ret = rdma_create_qp(cb->child_cm_id, cb->pd,
>> &init_attr);
>> +		if (!ret)
>> +			cb->qp = cb->child_cm_id->qp;
>> +	} else {
>> +		ret = rdma_create_qp(cb->cm_id, cb->pd, &init_attr);
>> +		if (!ret)
>> +			cb->qp = cb->cm_id->qp;
>> +	}
>> +
>> +//	if (ret) {
>> +//		cb->qp = NULL;
>> +//		return ret;
>> +//	}
>> +//
>> +//        qp_attr.qp_access_flags = IBV_ACCESS_REMOTE_READ|
>> +//				  IBV_ACCESS_REMOTE_WRITE;
>> +//      ret = ibv_modify_qp(cb->qp, &qp_attr, IBV_QP_ACCESS_FLAGS);
>> +//	if (ret)
>> +//		printf("ibv_modify_qp returned %d\n", ret);
>> +	return ret;
>> +}
>
>I added this qp_modify code during testing because the server's initial
>rdma read request wasn't being processed.  I later found out that this
>was due to the server path not reaping the "CONNECTED" event prior to
>posting the rdma read wr.  Now I notice you commented out this code (but
>left it in).

Uhm... I didn't mean to leave the code like this before submitting the patch.  I
knew that the IB CM set the QP attributes for RDMA read based on the requested
responder_resources being > 0, and writes were enabled by default.  I commented
out the code to verify that the modify wasn't needed.

We should always be able to determine if reads should be enabled.  Enabling
writes by default is debatable, but that's the way the code is currently
written.

- Sean