[ofw] RE: patch: Support APM on IBAL code

[Tzachi Dar]

Thanks for the info Fab.

The following patch stores the cid in the connection in the
proxy_cep_pre_req function as you have suggested.

Thanks
Tzachi


Index: al_proxy_cep.c
===================================================================
--- al_proxy_cep.c	(revision 4961)
+++ al_proxy_cep.c	(working copy)
@@ -208,6 +208,13 @@
 
 	p_ioctl->in.cm_req.h_qp = h_qp;
 
+
+	if(h_qp->type == IB_QPT_RELIABLE_CONN ||
+			h_qp->type == IB_QPT_UNRELIABLE_CONN) 
+		{
+			((al_conn_qp_t *)(h_qp))->cid = p_ioctl->in.cid;
+		}
+
 	p_ioctl->out.status = al_cep_pre_req( p_context->h_al,
p_ioctl->in.cid,
 		&p_ioctl->in.cm_req, &p_ioctl->out.init );
  

> -----Original Message-----
> From: Fab Tillier [mailto:ftillier at microsoft.com] 
> Sent: Thursday, September 24, 2009 8:12 PM
> To: Tzachi Dar; Sean Hefty; ofw at lists.openfabrics.org
> Subject: RE: [ofw] RE: patch: Support APM on IBAL code
> 
> Hi Tzachi,
> 
> Tzachi Dar wrote on Thu, 24 Sep 2009 at 07:33:35:
> 
> > It seems that one change in my code is causing build break, 
> and I have 
> > there for reverted this part on 2467.
> > 
> > It seems that the function al_cep_pre_req is now replaced with 
> > kal_cep_pre_req witch doesn't have the IB QP, as part of it.
> > 
> > In order to allow me to return this code, it seems that the best 
> > solution is to add a new member to the struct iba_cm_req:
> > iq_qp_hqndle_t h_qp.
> 
> The CID is stored in the QP in the __cep_conn_req function 
> (al_cm_qp.c), and if you want to have the CID stored in the 
> kernel QP object for a user-mode QP, you should do that in 
> the CM proxy.  The problem you have if you do that, is that 
> you must be careful that the CID, if destroyed by the kernel, 
> is no longer referenced anywhere in user-mode (otherwise the 
> CID could get reused for that same process and later 
> erroneously destroyed by user-mode when cleaning up an old reference.)
> 
> You could store the CID in the QP in the proxy_cep_pre_req 
> and proxy_cep_pre_rep handlers.
> 
> You need to be very careful about managing the CID's 
> lifetime.  I had to rewrite a ton of the CM code to fix a bug 
> where the CID was being recycled before all user-mode 
> references had been released.  You likely need to add code to 
> destroying_qp to only destroy the CEP if it's a kernel QP 
> object.  You'll also need to make sure you don't end up with 
> potential deadlocks during cleanup.  The code looks fine here 
> though (destroying_al will cleanup CEPs before destroying 
> children objects).
> 
>  
> > Are there any objections to this? (In other words, am I 
> breaking some 
> > kind of abstraction here?)
> 
> Yes, the we're trying to make the CEP manager more 
> independent of AL, so that WinVerbs can use it more naturally.
> 
> -Fab
> 
>