[ofw] [patch][IBAL] move sanity checks to the beginning of the function to prevent crash.

[Smith, Stan]

Hi Leo,
  The patch looks reasonable.
One question, why is it that when creating a QP, both a send and recv CQs are required? Otherwise the create QP call fails.
Why is not possible to have a send-only QP or a receive only QP?
How do you suggest to program around this QP creation limitation?

thanks,

stan.

________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Leonid Keller
Sent: Wednesday, October 13, 2010 11:56 AM
To: 'ofw at lists.openfabrics.org'
Subject: [ofw] [patch][IBAL] move sanity checks to the beginning of the function to prevent crash.

create_qp() first creates some basic QP, then adds QP_type specific functionality.
But before that it makes some sanity checks.
If one of CQ handles is incorrect create_qp()  exits from switch and performs destroy QP which crashes for RC QP
in destroy_cep because the latter presumes that QP as al_object is attached to AL.
But al_attach is called from raw_qp() which is called from specific actions which was never called.



Index: core/al/al_qp.c
===================================================================
--- core/al/al_qp.c           (revision 2969)
+++ core/al/al_qp.c        (working copy)
@@ -302,6 +302,45 @@
                                AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_SRQ_HANDLE\n") );
                                return IB_INVALID_SRQ_HANDLE;
                }
+
+             switch( p_qp_create->qp_type )
+             {
+             case IB_QPT_RELIABLE_CONN:
+             case IB_QPT_UNRELIABLE_CONN:
+                             if( AL_OBJ_INVALID_HANDLE( p_qp_create->h_sq_cq, AL_OBJ_TYPE_H_CQ ) ||                 <======== requires both Tx & Rx CQs?
+                                             AL_OBJ_INVALID_HANDLE( p_qp_create->h_rq_cq, AL_OBJ_TYPE_H_CQ ) )
+                             {
+                                             AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_CQ_HANDLE\n") );
+                                             return IB_INVALID_CQ_HANDLE;
+                             }
+                             break;
+
+             case IB_QPT_UNRELIABLE_DGRM:
+                             if( AL_OBJ_INVALID_HANDLE( p_qp_create->h_sq_cq, AL_OBJ_TYPE_H_CQ ) ||
+                                             AL_OBJ_INVALID_HANDLE( p_qp_create->h_rq_cq, AL_OBJ_TYPE_H_CQ ) )
+                             {
+                                             AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_CQ_HANDLE\n") );
+                                             return IB_INVALID_CQ_HANDLE;
+                             }
+                             break;
+
+             case IB_QPT_MAD:
+                             if( p_qp_create->h_sq_cq || p_qp_create->h_rq_cq )
+                             {
+                                             AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_CQ_HANDLE\n") );
+                                             return IB_INVALID_CQ_HANDLE;
+                             }
+                             break;
+
+             default:
+                             CL_ASSERT( p_qp_create->qp_type == IB_QPT_RELIABLE_CONN ||
+                                             p_qp_create->qp_type == IB_QPT_UNRELIABLE_CONN ||
+                                             p_qp_create->qp_type == IB_QPT_UNRELIABLE_DGRM ||
+                                             p_qp_create->qp_type == IB_QPT_MAD );
+                             AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_SETTING\n") );
+                             return IB_INVALID_SETTING;
+             }
+

                /* Allocate a QP. */
                status = alloc_qp( p_qp_create->qp_type, &h_qp );
@@ -320,42 +359,17 @@
                {
                case IB_QPT_RELIABLE_CONN:
                case IB_QPT_UNRELIABLE_CONN:
-                              if( AL_OBJ_INVALID_HANDLE( p_qp_create->h_sq_cq, AL_OBJ_TYPE_H_CQ ) ||
-                                              AL_OBJ_INVALID_HANDLE( p_qp_create->h_rq_cq, AL_OBJ_TYPE_H_CQ ) )
-                              {
-                                              status = IB_INVALID_CQ_HANDLE;
-                                              break;
-                              }
                                status = init_conn_qp( (al_conn_qp_t*)h_qp, h_pd, p_qp_create, p_umv_buf );
                                break;

                case IB_QPT_UNRELIABLE_DGRM:
-                              if( AL_OBJ_INVALID_HANDLE( p_qp_create->h_sq_cq, AL_OBJ_TYPE_H_CQ ) ||
-                                              AL_OBJ_INVALID_HANDLE( p_qp_create->h_rq_cq, AL_OBJ_TYPE_H_CQ ) )
-                              {
-                                              status = IB_INVALID_CQ_HANDLE;
-                                              break;
-                              }
                                status = init_dgrm_qp( (al_dgrm_qp_t*)h_qp, h_pd, p_qp_create, p_umv_buf );
                                break;

                case IB_QPT_MAD:
-                              if( p_qp_create->h_sq_cq || p_qp_create->h_rq_cq )
-                              {
-                                              status = IB_INVALID_CQ_HANDLE;
-                                              break;
-                              }
                                status = init_mad_qp( (al_mad_qp_t*)h_qp, h_pd, p_qp_create,
                                                pfn_qp_event_cb );
                                break;
-
-              default:
-                              CL_ASSERT( h_qp->type == IB_QPT_RELIABLE_CONN ||
-                                              h_qp->type == IB_QPT_UNRELIABLE_CONN ||
-                                              h_qp->type == IB_QPT_UNRELIABLE_DGRM ||
-                                              h_qp->type == IB_QPT_MAD );
-                              status = IB_INVALID_SETTING;
-                              break;
                }

                if( status != IB_SUCCESS )
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20101015/8284b218/attachment.html>