[dpdk-dev] [PATCH v5] vfio: change to use generic multi-process channel

Burakov, Anatoly anatoly.burakov at intel.com
Tue Mar 20 11:33:00 CET 2018


On 19-Mar-18 6:53 AM, Tan, Jianfeng wrote:
> Hi Anatoly,
> 
> Thank you for the review. All your comments will be addressed in next version, except for below concern which might be taken care of in another patch if it also concerns you.
> 
>> -----Original Message-----
>> From: Burakov, Anatoly
>> Sent: Wednesday, March 14, 2018 9:27 PM
>> To: Tan, Jianfeng; dev at dpdk.org
>> Cc: Richardson, Bruce; Ananyev, Konstantin; thomas at monjalon.net
>> Subject: Re: [PATCH v5] vfio: change to use generic multi-process channel
> [...]
>>
>>> +	mp_req.len_param = sizeof(*p);
>>> +	mp_req.num_fds = 0;
>>> +
>>> +	vfio_group_fd = -1;
>>> +	if (rte_mp_request(&mp_req, &mp_reply, &ts) == 0 &&
>>> +	    mp_reply.nb_received == 1) {
>>> +		mp_rep = &mp_reply.msgs[0];
>>> +		p = (struct vfio_mp_param *)mp_rep->param;
>>> +		if (p->result == SOCKET_OK && mp_rep->num_fds == 1) {
>>> +			cur_grp->group_no = iommu_group_no;
>>> +			vfio_group_fd = mp_rep->fds[0];
>>> +			cur_grp->fd = vfio_group_fd;
>>> +			vfio_cfg.vfio_active_groups++;
>>>    		}
>>> +		free(mp_reply.msgs);
>>>    	}
>>> -	return -1;
>>> +
>>> +	if (vfio_group_fd < 0)
>>> +		RTE_LOG(ERR, EAL, "  cannot request group fd\n");
>>> +	return vfio_group_fd;
>>
>> p->result can be SOCKET_NO_FD, in which case returned value should be
>> zero. I think this is missing from this code. There probably should be
>> an "else if (p->result == SOCKET_NO_FD)" clause that sets return value to 0.
>>
>> You should be able to test this by trying to set up a device for VFIO
>> that isn't bound to VFIO driver, in a secondary process.
> 
> OK, I will fix this.
> 
> But really, "zero" could be ambiguous as a fd could, theoretically, be zero too.

You're correct. Maybe return 0/-1 in case of success/failure and put fd 
into a pointer? i.e.

int func(int *vfio_group_fd) {
<...>
*vfio_group_fd = fd;
return 0;
}

> 
> Thanks,
> Jianfeng
> 


-- 
Thanks,
Anatoly


More information about the dev mailing list