From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id AB29C5F13 for ; Wed, 7 Mar 2018 10:00:41 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Mar 2018 01:00:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,435,1515484800"; d="scan'208";a="36621841" Received: from debian.sh.intel.com (HELO debian) ([10.67.104.164]) by orsmga001.jf.intel.com with ESMTP; 07 Mar 2018 01:00:39 -0800 Date: Wed, 7 Mar 2018 16:59:05 +0800 From: Tiwei Bie To: Maxime Coquelin Cc: dev@dpdk.org, jianfeng.tan@intel.com, yliu@fridaylinux.org, zhihong.wang@intel.com, xiao.w.wang@intel.com, cunming.liang@intel.com, dan.daly@intel.com Message-ID: <20180307085905.pist45rli7kysokg@debian> References: <20180306104327.14470-1-tiwei.bie@intel.com> <20180306104327.14470-4-tiwei.bie@intel.com> <4a4025f9-6886-8adc-51ed-c8508d81a003@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4a4025f9-6886-8adc-51ed-c8508d81a003@redhat.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH 3/3] vhost: support VFIO based accelerator X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Mar 2018 09:00:42 -0000 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