[ofw] ConnectX functionality is completely broken

[Leonid Keller]

I believe, it's functional.


________________________________

	From: Smith, Stan [mailto:stan.smith at intel.com] 
	Sent: Tuesday, August 05, 2008 7:10 PM
	To: Leonid Keller; Hefty, Sean; Anatoly Greenblatt;
ofw at lists.openfabrics.org
	Subject: RE: [ofw] ConnectX functionality is completely broken
	
	
	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/7e624eae/attachment.html>