[dpdk-dev] [PATCH v5] vfio: change to use generic multi-process channel
Burakov, Anatoly
anatoly.burakov at intel.com
Tue Mar 20 11:56:28 CET 2018
On 20-Mar-18 10:33 AM, Burakov, Anatoly wrote:
> 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;
> }
Or rather return 1/0/-1 depending on whether we got SOCKET_OK,
SOCKET_NO_FD or SOCKET_ERR.
>
>>
>> Thanks,
>> Jianfeng
>>
>
>
--
Thanks,
Anatoly
More information about the dev
mailing list