[openib-general] [PATCH] mthca: various bug fixes for mthca_query_qp

[Jack Morgenstein]

On Wednesday 23 August 2006 23:25, Roland Dreier wrote:
>  > 5. Return the send_cq, receive cq and srq handles.  ib_query_qp() needs them
>  >    (required by IB Spec). ibv_query_qp() overwrites these values in user-space
>  >    with appropriate user-space values.
> 
>  > +	qp_init_attr->send_cq       = ibqp->send_cq;
>  > +	qp_init_attr->recv_cq       = ibqp->recv_cq;
>  > +	qp_init_attr->srq           = ibqp->srq;
> 
> I really disagree with this change.  It's silly to do this copying
> since the consumer already has the ibqp pointer.  And it's especially
> silly to put this in a low-level driver, since there's nothing
> device-specific about it.
> 
Note that the same thing is done in user-space (albeit in the verbs layer):

libibverbs/src/cmd.c, lines 678 ff. (procedure ibv_cmd_query_qp() ):

 	init_attr->send_cq                  = qp->send_cq;
	init_attr->recv_cq                  = qp->recv_cq;
	init_attr->srq                      = qp->srq;

Either return these values in both kernel and user, or remove them from
the user-space verb.

Regarding putting this in the low level kernel driver, I agree.
A patch for core/verbs.c is given below.

(P.S. IMHO, the correct thing to do is to remove the above lines from cmd.c,
 and not return to the user stuff that s/he already has available).

- Jack

-------------
Index: ofed_1_1/drivers/infiniband/core/verbs.c
===================================================================
--- ofed_1_1.orig/drivers/infiniband/core/verbs.c	2006-08-03 14:30:21.000000000 +0300
+++ ofed_1_1/drivers/infiniband/core/verbs.c	2006-08-24 10:03:51.831333000 +0300
@@ -556,9 +556,17 @@ int ib_query_qp(struct ib_qp *qp,
 		int qp_attr_mask,
 		struct ib_qp_init_attr *qp_init_attr)
 {
-	return qp->device->query_qp ?
-		qp->device->query_qp(qp, qp_attr, qp_attr_mask, qp_init_attr) :
-		-ENOSYS;
+	int rc = -ENOSYS;
+	if (qp->device->query_qp) {
+		rc = qp->device->query_qp(qp, qp_attr,
+					  qp_attr_mask, qp_init_attr);
+		if(!rc) {
+			qp_init_attr->recv_cq = qp->recv_cq;
+			qp_init_attr->send_cq = qp->send_cq;
+			qp_init_attr->srq = qp->srq;
+		}
+	}
+	return rc;
 }
 EXPORT_SYMBOL(ib_query_qp);