[ofw] [PATCH 3/7] ib/cm: export CM only interface

[Fab Tillier]

> I have no way to test the ND code, but here's what I added to
> al_ndi_cm.c:

Your changes should be transparent if you keep the existing return values.

> __ndi_send_req():
>
>         cl_spinlock_acquire( &h_qp->obj.lock ); if( h_qp->obj.state !=
>         CL_DESTROYING && p_qp->cid == AL_INVALID_CID ) {
>                 status = al_create_cep( qp_get_al( h_qp ),
> __ndi_cm_handler,
>                                                 &h_qp->obj,
>                                                 deref_al_obj, &p_qp->cid
>                                                 );
>         }
>         else
>         {
>                 status = IB_INVALID_HANDLE;

Existing code returns IB_RESOURCE_BUSY here.

>         }
>         cl_spinlock_release( &h_qp->obj.lock );
>
>
> __ndi_send_rep():
>
>         /* prepare Passive CEP for connection */ p_qp =
>         (al_conn_qp_t*)h_qp; cl_spinlock_acquire( &h_qp->obj.lock ); if(
>         h_qp->obj.state != CL_DESTROYING && p_qp->cid == AL_INVALID_CID
>         ) {
>                 status = kal_config_cep(qp_get_al( h_qp ), p_rep->cid,
>                                                 __ndi_cm_handler,
>                                                 &h_qp->obj,
> deref_al_obj);
>                 if( status == IB_SUCCESS )
>                 {
>                         ref_al_obj( &h_qp->obj ); /* Take CEP reference.
>                         */ status = kal_cep_pre_rep(qp_get_al( h_qp ),
> p_rep->cid,
>                                                          &cm_rep,
>
> QP_ATTRIB_RNR_NAK_TIMEOUT, &qp_mod );
>                         if( status == IB_SUCCESS )
>                         {
>                                 p_qp->cid = p_rep->cid;
>                         }

Note that you could config the CEP and call kal_cep_pre_rep without the QP's lock, and then perform the checks in the success case and then perform the CID checks and make the assignment.  You can probably do this in other places too in order to avoid holding the lock over the CEP calls.

>                 }
>         }
>         else
>         {
>                 status = IB_INVALID_HANDLE;

Same here, IB_RESOURCE_BUSY.

>         }
>         cl_spinlock_release( &h_qp->obj.lock );
>
> I also fixed al_destroy_cep() to read the *p_cid under lock.  The
> requirement is that the QP lock can be held when trying to acquire the
> CEP mgr lock, but I haven't been able to verify that this is done in
> the existing code.

That shouldn't be a problem.  The only issue I can imagine is the destroy callback for the QP trying to take the same lock (which it doesn't seem like it does.)

> The other area of the code that I would like to cleanup is getting
> callers to invoke al_destroy_cep() only once for a given CID.  But
> this looks much more difficult to achieve.

I think if you manage the QP's CID assignment at a higher level you can do all the necessary checks on the CID before calling destroy, so only one call path should end up calling it.

-Fab