[ofw] [PATCH] Use the CM protocol to exchange initiator depth and responder resources

Sean Hefty sean.hefty at intel.com
Tue Jul 1 15:15:49 PDT 2008


> al_cep_get_pdata(
>        IN                              ib_al_handle_t
>h_al,
>        IN                              net32_t
>cid,
>+        OUT         uint8_t                     *p_init_depth,
>+        OUT         uint8_t                     *p_resp_res,
>        IN      OUT                     uint8_t
>*p_psize,
>                OUT                     uint8_t*
>pdata );

The function name should change.  It currently implies retrieving private data,
which it is no longer limited to doing.


>diff -up -r -X trunk\docs\dontdiff.txt -I \$Id: old\core\al\al_dev.h
>trunk\core\al\al_dev.h
>--- old\core\al\al_dev.h        Tue Jul 01 14:55:29 2008
>+++ trunk\core\al\al_dev.h      Tue Jul 01 14:54:53 2008
>@@ -55,7 +55,7 @@
> #define AL_DEVICE_NAME L"\\Device\\ibal"
> #define        ALDEV_KEY               (0x3B)  /* Matches
>FILE_DEVICE_INFINIBAND from wdm.h */
>
>-#define AL_IOCTL_VERSION                       (6)
>+#define AL_IOCTL_VERSION                       (7)

How about updating the IOCTL version once at the end of this series, rather than
on each patch?

> /* max number of devices with non-default pkey */
> #define        MAX_NUM_PKEY    16
>diff -up -r -X trunk\docs\dontdiff.txt -I \$Id: old\core\al\kernel\al_cm_cep.c
>trunk\core\al\kernel\al_cm_cep.c
>--- old\core\al\kernel\al_cm_cep.c      Tue Jul 01 14:55:30 2008
>+++ trunk\core\al\kernel\al_cm_cep.c    Tue Jul 01 14:54:53 2008
>@@ -6379,6 +6379,8 @@ NTSTATUS
> al_cep_get_pdata(
>        IN                              ib_al_handle_t
>h_al,
>        IN                              net32_t
>cid,
>+        OUT         uint8_t                     *p_init_depth,
>+        OUT         uint8_t                     *p_resp_res,
>        IN      OUT                     uint8_t
>*p_psize,
>                OUT                     uint8_t*
>pdata )
> {
>@@ -6399,6 +6401,9 @@ al_cep_get_pdata(
>                        ("CEP not found for cid %d, h_al %p\n", cid, h_al ));
>                return STATUS_CONNECTION_INVALID;
>        }
>+
>+    *p_init_depth = p_cep->init_depth;
>+    *p_resp_res = p_cep->resp_res;
>
>     copy_len = min( *p_psize, p_cep->psize );
>
>diff -up -r -X trunk\docs\dontdiff.txt -I \$Id: old\core\al\kernel\al_ndi_cm.c
>trunk\core\al\kernel\al_ndi_cm.c
>--- old\core\al\kernel\al_ndi_cm.c      Tue Jul 01 14:55:29 2008
>+++ trunk\core\al\kernel\al_ndi_cm.c    Tue Jul 01 14:54:53 2008
>@@ -184,29 +184,26 @@ __cep_timewait_qp(
> static ib_api_status_t
> __ndi_qp2rts(
>        IN              ib_qp_handle_t  const                           h_qp,
>-       IN              uint8_t
>init_depth,
>-       IN              uint8_t
>resp_res,
>-       IN              PIRP
>p_irp,
>-       IN      OUT     ib_qp_mod_t
>*p_qp_mod
>+       IN              PIRP
>p_irp
>        )
> {
>        ib_api_status_t status;
>+       ib_qp_mod_t qp_mod;
>
>        AL_ENTER( AL_DBG_NDI );
>
>        /* fill required qp attributes */
>        status = al_cep_get_rtr_attr( qp_get_al( h_qp ),
>-               ((al_conn_qp_t*)h_qp)->cid, p_qp_mod );
>+               ((al_conn_qp_t*)h_qp)->cid, &qp_mod );
>        if ( status != IB_SUCCESS )
>        {
>                AL_PRINT( TRACE_LEVEL_ERROR, AL_DBG_ERROR,
>                        ("al_cep_get_rtr_attr returned %s\n", ib_get_err_str(
>status )) );
>                goto exit;
>        }
>-       p_qp_mod->state.rtr.resp_res = resp_res;
>
>        /* perform the request: INIT->RTR */
>-       status = ndi_modify_qp( h_qp, p_qp_mod,
>+       status = ndi_modify_qp( h_qp, &qp_mod,
>                cl_ioctl_out_size( p_irp ), cl_ioctl_out_buf( p_irp ) );
>        if ( status != IB_SUCCESS )
>        {
>@@ -217,17 +214,16 @@ __ndi_qp2rts(
>
>        /* fill required qp attributes */
>        status = al_cep_get_rts_attr( qp_get_al( h_qp ),
>-               ((al_conn_qp_t*)h_qp)->cid, p_qp_mod );
>+               ((al_conn_qp_t*)h_qp)->cid, &qp_mod );
>        if ( status != IB_SUCCESS )
>        {
>                AL_PRINT( TRACE_LEVEL_ERROR, AL_DBG_ERROR,
>                        ("al_cep_get_rts_attr returned %s\n", ib_get_err_str(
>status )) );
>                goto exit;
>        }
>-       p_qp_mod->state.rts.init_depth = init_depth;
>
>        /* perform the request: RTR->RTS */
>-       status = ndi_modify_qp( h_qp, p_qp_mod,
>+       status = ndi_modify_qp( h_qp, &qp_mod,
>                cl_ioctl_out_size( p_irp ), cl_ioctl_out_buf( p_irp ) );
>        if ( status != IB_SUCCESS )
>        {
>@@ -848,8 +849,8 @@ __ndi_fill_cm_req(
>
>        p_cm_req->qp_type = IB_QPT_RELIABLE_CONN;
>        p_cm_req->h_qp = h_qp;
>-       p_cm_req->resp_res = 0;
>-       p_cm_req->init_depth = 0;
>+       p_cm_req->resp_res = p_req->resp_res;
>+       p_cm_req->init_depth = p_req->init_depth;
>
>        p_cm_req->remote_resp_timeout =
>                ib_path_rec_pkt_life( p_path_rec ) + CM_REMOTE_TIMEOUT;
>@@ -1218,13 +1219,8 @@ __ndi_rtu_cm(
>        IN                              PIRP
>p_irp )
> {
>        NTSTATUS nt_status;
>-       ib_qp_mod_t qp_mod;
>        ib_api_status_t status;
>-       ib_qp_handle_t VOID_PTR64 h_qp = p_irp->Tail.Overlay.DriverContext[0];
>-       ual_ndi_rtu_cm_ioctl_in_t *p_rtu =
>-               (ual_ndi_rtu_cm_ioctl_in_t*)cl_ioctl_in_buf( p_irp );
>-       uint8_t pdata[IB_REJ_PDATA_SIZE];
>-       uint8_t psize = sizeof(pdata);
>+       ib_qp_handle_t h_qp = p_irp->Tail.Overlay.DriverContext[0];

Did something validate this handle, or was this context allocated in the kernel?

>
>        UNUSED_PARAM(p_dev_obj);
>
>@@ -1235,17 +1231,15 @@ __ndi_rtu_cm(
>                IoFreeWorkItem( p_irp->Tail.Overlay.DriverContext[1] );
>
>        /* change the QP state to RTS */
>-       status = __ndi_qp2rts( h_qp, p_rtu->init_depth,
>-               p_rtu->resp_res, p_irp, &qp_mod );
>+       status = __ndi_qp2rts( h_qp, p_irp );
>        if ( status != IB_SUCCESS )
>        {
>                goto err;
>        }
>
>        /* send RTU */
>-       al_cep_get_pdata( qp_get_al( h_qp ), ((al_conn_qp_t*)h_qp)->cid,
>&psize, pdata );
>-       status = al_cep_rtu( qp_get_al( h_qp ), ((al_conn_qp_t*)h_qp)->cid,
>pdata, psize );
>-       if( status != IB_SUCCESS && status != IB_INVALID_STATE )
>+       status = al_cep_rtu( qp_get_al( h_qp ), ((al_conn_qp_t*)h_qp)->cid,
>NULL, 0 );
>+       if( status != IB_SUCCESS )
>        {
>                net32_t                         cid;
> err:
>@@ -1333,7 +1327,6 @@ __ndi_rep_cm(
>        IN                              PIRP
>p_irp )
> {
>        NTSTATUS nt_status;
>-       ib_qp_mod_t qp_mod;
>        ib_api_status_t status;
>        ib_qp_handle_t VOID_PTR64 h_qp = p_irp->Tail.Overlay.DriverContext[0];
>        ual_ndi_rep_cm_ioctl_in_t *p_rep =
>@@ -1348,8 +1341,7 @@ __ndi_rep_cm(
>                IoFreeWorkItem( p_irp->Tail.Overlay.DriverContext[1] );
>
>        /* change the QP state to RTS */
>-       status = __ndi_qp2rts( h_qp, p_rep->init_depth,
>-               p_rep->resp_res, p_irp, &qp_mod );
>+       status = __ndi_qp2rts( h_qp, p_irp );
>        if ( status != IB_SUCCESS )
>        {
>                nt_status = STATUS_CONNECTION_ABORTED;
>@@ -1417,7 +1409,7 @@ __ndi_fill_cm_rep(
>        p_cm_rep->h_qp = h_qp;
>
>        p_cm_rep->access_ctrl = IB_AC_RDMA_READ | IB_AC_RDMA_WRITE |
>IB_AC_LOCAL_WRITE;
>-       p_cm_rep->init_depth = 0;
>+       p_cm_rep->init_depth = p_rep->init_depth;
>        p_cm_rep->target_ack_delay = 10;
>        p_cm_rep->failover_accepted = IB_FAILOVER_ACCEPT_UNSUPPORTED;
>        p_cm_rep->flow_ctrl = TRUE;     /* HCAs must support end-to-end flow
>control. */
>diff -up -r -X trunk\docs\dontdiff.txt -I \$Id:
>old\core\al\kernel\al_proxy_cep.c trunk\core\al\kernel\al_proxy_cep.c
>--- old\core\al\kernel\al_proxy_cep.c   Tue Jul 01 14:55:29 2008
>+++ trunk\core\al\kernel\al_proxy_cep.c Tue Jul 01 14:54:53 2008
>@@ -935,7 +935,8 @@ proxy_cep_get_pdata(
>        }
>
>        p_ioctl->out.pdata_len = sizeof(p_ioctl->out.pdata);
>-       status = al_cep_get_pdata( p_context->h_al, p_ioctl->in.cid,
>+       status = al_cep_get_pdata( p_context->h_al, p_ioctl->in.cid,
>+        &p_ioctl->out.init_depth, &p_ioctl->out.resp_res,
>                (uint8_t*)&p_ioctl->out.pdata_len, p_ioctl->out.pdata );
>
>        if( NT_SUCCESS( status ) )
>diff -up -r -X trunk\docs\dontdiff.txt -I \$Id: old\inc\iba\ib_al_ioctl.h
>trunk\inc\iba\ib_al_ioctl.h
>--- old\inc\iba\ib_al_ioctl.h   Thu Jun 26 20:35:14 2008
>+++ trunk\inc\iba\ib_al_ioctl.h Tue Jul 01 14:54:53 2008
>@@ -3130,6 +3130,8 @@ typedef union _ual_cep_get_pdata_ioctl
>        {
>                uint32_t                                pdata_len;
>                uint8_t
>pdata[IB_REJ_PDATA_SIZE];
>+        uint8_t                 resp_res;
>+        uint8_t                 init_depth;
>
>        }       out;

I would also rename this structure, and probably whatever the IOCTL call is as
well.

>@@ -3516,6 +3518,8 @@ typedef struct _ual_ndi_req_cm_ioctl_in
>        uint8_t                                         prot;
>        uint8_t                                         pdata_size;
>        ib_cm_rdma_req_t                        pdata;
>+    uint8_t                     resp_res;
>+    uint8_t                     init_depth;

Not sure, but this structure may give better alignment putting the uint8_t's
together.

- Sean





More information about the ofw mailing list