[Openib-windows] [PATCH] cq_checks

[Fabian Tillier]

Hi Yossi,

On 4/17/06, Yossi Leybovich <sleybo at mellanox.co.il> wrote:
> Fab
>
> Our regression tests run on verios scenarios of bad flows
> This patch add few sanity checks to destroy/creat cq verbs.
>
> 1. check that whendestroy CQ all attached QPs were destroy
> 2. CQ size not bigger then ca max_cqe
>
> This will enable our tests to run over more cases and more smoothly.
>
> pls review
>
> 10x
> Yossi
>
> Index: core/al/al_cq.c
> ===================================================================
> --- core/al/al_cq.c (revision 1260)
> +++ core/al/al_cq.c (working copy)
> @@ -83,6 +83,12 @@
>   return IB_INVALID_SETTING;
>  }
>
> + if( p_cq_create->size > h_ca->obj.p_ci_ca->p_pnp_attr->max_cqes)
> + {
> +  CL_TRACE_EXIT( AL_DBG_ERROR, g_al_dbg_lvl, ("IB_INVALID_CQ_SIZE\n")
> );
> +  return IB_INVALID_CQ_SIZE;
> + }
> +

I think this check belongs in the HCA driver.  The pnp attributes
pointer can change dynamically, so there's locking missing here
anyway.  I plan on making the IBAL layer thinner and thinner as we
move forward, so having this check in the HCA driver will remove the
need to move it at a later date.

>  h_cq = cl_zalloc( sizeof( ib_cq_t ) );
>  if( !h_cq )
>  {
> @@ -161,6 +167,11 @@
>   return IB_INVALID_CQ_HANDLE;
>  }
>
> + if(h_cq->qp_list.count != 0)
> + {
> +  CL_TRACE_EXIT( AL_DBG_ERROR, g_al_dbg_lvl, ("IB_RESOURCE_BUSY\n") );
> +  return IB_RESOURCE_BUSY;
> + }

This completely changes the behavior of IBAL, and creates an
inconsistency between CQ destruction behavior and all other resources.
 Currently, destruction cascades to all dependent objects - if you
destrly a CQ, it implies destruction of all attached QPs.  Likewise
with destruction of a PD, all QPs, MRs, and MWs get cleaned up
automatically.

Changing this requires that existing ULPs be examined to determine if
their cleanup paths need to be changed, and is a fairly significant
change.

That said, the cascading destruction is actually quite painful to
implement properly, so I envision destruction moving a model like you
propose in a future version, perhaps even 2.0.

I think the documentation is incorrect in ib_al.h, and should be
updated to reflect the cascading destruction behavior of IBAL.

- Fab