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

Tan, Jianfeng jianfeng.tan at intel.com
Mon Mar 19 07:53:33 CET 2018


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.

Thanks,
Jianfeng


More information about the dev mailing list