[ofw] ConnectX functionality is completely broken

[Smith, Stan]

Hello,
  Although there has been excellent discussion on the topic, I'm a bit confused as to what's been committed to svn?
Is the head of svn now functional or do I need to apply patch(es)?

thanks,

stan.

________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Leonid Keller
Sent: Sunday, August 03, 2008 1:24 AM
To: Hefty, Sean; Anatoly Greenblatt; ofw at lists.openfabrics.org
Subject: RE: [ofw] ConnectX functionality is completely broken

I agree with the comment, but it is a separate issue.
It's worth to change it in all the verbs.

________________________________
From: Sean Hefty [mailto:sean.hefty at intel.com]
Sent: Friday, August 01, 2008 8:33 PM
To: Leonid Keller; Anatoly Greenblatt; ofw at lists.openfabrics.org
Subject: RE: [ofw] ConnectX functionality is completely broken

To recall, 1435 patch has improved event notification mechanism for cq, qp and srq objects.
I found one problem in the patch, which repeats itself for all three objects and for both drivers: new event handlers get the old (and wrong) context values.
The new (and right) context values are nor used. As a result, IBAL callbacks are called with wrong handle parameter, which ends up with crash.

I tested the patch below on mthca, and it's working fine for me.  If anyone knows how to force the CQ, QP, or SRQ async events with an existing test, let me know and I will run it.  I do have one comment below:


Index: hw/mthca/kernel/hca_verbs.c
===================================================================
@@ -906,12 +904,8 @@
   goto err_create_srq;
  }

- // fill the object
- srq_p = (struct mthca_srq *)ib_srq_p;
- srq_p->srq_context = (void*)srq_context;
-
  // return the result
- if (ph_srq) *ph_srq = (ib_srq_handle_t)srq_p;
+ if (ph_srq) *ph_srq = (ib_srq_handle_t)ib_srq_p;

ph_srq isn't really optional here.  If one isn't provided, we end up leaking memory.  Personally, I would just remove the if check, but it could also be moved to the top of the function with a failure return if the output parameter is not provided.

Similar checks are provided in _create_qp() and mthca_create_cq().

- Sean
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20080805/c405de1e/attachment.html>