Re: [PATCH v2] RDMA/vmw_pvrdma: Use resource ids from physical device if available

[Adit Ranadive <aditr@vmware.com>]

On 10/28/19 9:27 AM, Jason Gunthorpe wrote:
> On Tue, Oct 22, 2019 at 08:06:50PM +0000, Adit Ranadive wrote:
>> @@ -195,7 +198,9 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
>>  	union pvrdma_cmd_resp rsp;
>>  	struct pvrdma_cmd_create_qp *cmd = &req.create_qp;
>>  	struct pvrdma_cmd_create_qp_resp *resp = &rsp.create_qp_resp;
>> +	struct pvrdma_cmd_create_qp_resp_v2 *resp_v2 = &rsp.create_qp_resp_v2;
>>  	struct pvrdma_create_qp ucmd;
>> +	struct pvrdma_create_qp_resp qp_resp = {};
>>  	unsigned long flags;
>>  	int ret;
>>  	bool is_srq = !!init_attr->srq;
>> @@ -260,6 +265,15 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
>>  				goto err_qp;
>>  			}
>>  
>> +			/* Userspace supports qpn and qp handles? */
>> +			if (dev->dsr_version >= PVRDMA_QPHANDLE_VERSION &&
>> +			    udata->outlen != sizeof(qp_resp)) {
> 
> Is != really what you want? Or is <= better? != means you can't ever
> make qp_resp bigger.

I thought about using != or < before sending the patch. Since we removed
the flag anyways using != here made sense to be more strict about what's
acceptable. I'm not sure if we'll ever make it bigger.

> 
>> +	if (!qp->is_kernel) {
>> +		if (udata->outlen == sizeof(qp_resp)) {
> 
> Same comment, this really should be min()? Didn't I mention that already?
>

Why? I didn't add the min since I used != above anyways.

> Jason
>