On Tue, Mar 06, 2018 at 03:24:27PM +0100, Maxime Coquelin wrote:
> On 03/06/2018 11:43 AM, Tiwei Bie wrote:
[...]
> > +
> > +static int vhost_user_slave_set_vring_file(struct virtio_net *dev,
> > + uint32_t request,
> > + struct vhost_vring_file *file)
> Why passing the request as an argument?
> It seems to be called only with the same request ID.
I thought there may be other requests that also need to
send a file descriptor for a ring in the future. So I
made this a common routine. Maybe it's not really helpful.
I won't pass the request as an argument in next version.
>
> > +{
> > + int *fdp = NULL;
> > + size_t fd_num = 0;
> > + int ret;
> > + struct VhostUserMsg msg = {
> > + .request.slave = request,
> > + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY,
> > + .payload.u64 = file->index & VHOST_USER_VRING_IDX_MASK,
> > + .size = sizeof(msg.payload.u64),
> > + };
> > +
> > + if (file->fd < 0)
> > + msg.payload.u64 |= VHOST_USER_VRING_NOFD_MASK;
> > + else {
> > + fdp = &file->fd;
> > + fd_num = 1;
> > + }
> > +
> > + ret = send_vhost_message(dev->slave_req_fd, &msg, fdp, fd_num);
> > + if (ret < 0) {
> > + RTE_LOG(ERR, VHOST_CONFIG,
> > + "Failed to send slave message %u (%d)\n",
> > + request, ret);
> > + return ret;
> > + }
> > +
> > + return process_slave_message_reply(dev, &msg);
>
> Maybe not needed right now, but we'll need a lock to avoid concurrent
> requests sending and waiting for reply.
Yeah, probably, we need a lock for each slave channel. I didn't
check the code of Linux. Maybe it will cause problems when two
threads send e.g. below messages at the same time:
thread A:
IOTLB miss message
thread B:
VFIO group message which has a file descriptor
Thanks for the comments! :)
Best regards,
Tiwei Bie
