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

[Fab Tillier]

>> 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.

Ooo, I know, I'll call it al_cep_get_pdata_and_initiator_depth_and_responder_resources. :P

The function name change will happen later, as doing it now would really muck with my ability to generate patches, which is complicated enough as it is.

It'll be al_cep_get_data.

>> 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?

That would work if the patches were all getting committed at once.  They're not, so it's better to just bump it up so that things are functional at each patch.

>> @@ -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?

Yes, the user-mode handle is validated (in al_proxy_ndi.c), and the kernel QP handle is stored in DriverContext[0].

>> 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.

I'll defer this to a later patch, along with the rename of the function above.

>
>> @@ -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.

I'll update and resend.