From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 6F8AF25E5 for ; Wed, 13 Dec 2017 04:16:48 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Dec 2017 19:16:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,396,1508828400"; d="scan'208";a="186185311" Received: from deepin-15.sh.intel.com (HELO debian-xvivbkq) ([10.67.104.165]) by fmsmga005.fm.intel.com with ESMTP; 12 Dec 2017 19:16:44 -0800 Date: Wed, 13 Dec 2017 11:16:15 +0800 From: Tiwei Bie To: Maxime Coquelin Cc: dev@dpdk.org, yliu@fridaylinux.org, jianfeng.tan@intel.com, lprosek@redhat.com, lersek@redhat.com Message-ID: <20171213031615.t7rh2ccjf5gml73h@debian-xvivbkq> References: <20171211151503.19195-1-maxime.coquelin@redhat.com> <20171211151503.19195-5-maxime.coquelin@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20171211151503.19195-5-maxime.coquelin@redhat.com> User-Agent: NeoMutt/20170609 (1.8.3) Subject: Re: [dpdk-dev] [PATCH v4 4/4] vhost: destroy unused virtqueues when multiqueue not negotiated 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, 13 Dec 2017 03:16:48 -0000 On Mon, Dec 11, 2017 at 04:15:03PM +0100, Maxime Coquelin wrote: > QEMU sends VHOST_USER_SET_VRING_CALL requests for all queues > declared in QEMU command line before the guest is started. > It has the effect in DPDK vhost-user backend to allocate vrings > for all queues declared by QEMU. > > If the first driver being used does not support multiqueue, > the device never changes to VIRTIO_DEV_RUNNING state as only > the first queue pair is initialized. One driver impacted by > this bug is virtio-net's iPXE driver which does not support > VIRTIO_NET_F_MQ feature. > > It is safe to destroy unused virtqueues in SET_FEATURES request > handler, as it is ensured the device is not in running state > at this stage, so virtqueues aren't being processed. > > Signed-off-by: Maxime Coquelin > --- > lib/librte_vhost/vhost_user.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index 471b1612c..d5ca1ac90 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -216,6 +216,24 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features) > (dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? "on" : "off", > (dev->features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off"); > > + if (!(dev->features & (1ULL << VIRTIO_NET_F_MQ))) { > + /* > + * Remove all but first queue pair if MQ hasn't been > + * negotiated. This is safe because the device is not > + * running at this stage. > + */ > + while (dev->nr_vring > 2) { > + struct vhost_virtqueue *vq; > + > + vq = dev->virtqueue[--dev->nr_vring]; > + if (!vq) > + continue; > + > + cleanup_vq(vq, 1); > + free_vq(vq); Hi, Sorry, I didn't look into this patch in last version. The freed dev->virtqueue[$idx] also needs to be zeroed. Otherwise, it won't be allocated in the future due to the below check in vhost_user_check_and_alloc_queue_pair(), and the freed memory will be used again. /* * Allocate a queue pair if it hasn't been allocated yet */ static int vhost_user_check_and_alloc_queue_pair(struct virtio_net *dev, VhostUserMsg *msg) { ........ if (dev->virtqueue[vring_idx]) return 0; return alloc_vring_queue(dev, vring_idx); } Best regards, Tiwei Bie > + } > + } > + > return 0; > } > > -- > 2.14.3 >