From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 981F2237 for ; Wed, 6 Dec 2017 12:40:29 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A366621BA2; Wed, 6 Dec 2017 11:40:28 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-175.rdu2.redhat.com [10.10.120.175]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA41D6E706; Wed, 6 Dec 2017 11:40:22 +0000 (UTC) To: Maxime Coquelin , dev@dpdk.org, yliu@fridaylinux.org, tiwei.bie@intel.com, jianfeng.tan@intel.com, lprosek@redhat.com References: <20171206092048.3568-1-maxime.coquelin@redhat.com> From: Laszlo Ersek Message-ID: <1389d513-e047-a936-7397-6bf483b2dbf1@redhat.com> Date: Wed, 6 Dec 2017 12:40:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171206092048.3568-1-maxime.coquelin@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 06 Dec 2017 11:40:28 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v3 0/4] Vhost: fix mq=on but VIRTIO_NET_F_MQ 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, 06 Dec 2017 11:40:29 -0000 On 12/06/17 10:20, Maxime Coquelin wrote: > Hi, > > This third revision reworks the VQs destruction loop to fixes the > of-by-one error reported by Laszlo. > > Having QEMU started with mq=on but guest driver not negotiating > VIRTIO_NET_F_MQ feature ends up in the vhost device to never > start. Indeed, more queues are created in the vhost backend than > configured. > > Guest drivers known to not advertise the VIRTIO_NET_F_MQ feature are > iPXE and OVMF Virtio-net drivers. > > Queues are created because before starting the guest, QEMU sends > VHOST_USER_SET_VRING_CALL requests for all queues declared in QEMU > command line. Also, once Virtio features negotiated, QEMU sends > VHOST_USER_SET_VRING_ENABLE requests to disable all but the first > queue pair. > > This series fixes this by destroying all but first queue pair in > the backend if VIRTIO_NET_F_MQ isn't negotiated. First patches > makes sure that VHOST_USER_SET_FEATURES request doesn't change > Virtio features while the device is running, which should never > happen as per the Virtio spec. This helps to make sure vitqueues > aren't destroyed while being processed, but also protect from > other illegal features changes (e.g. VIRTIO_NET_F_MRG_RXBUF). > > > Changes since v2: > ================= > - Patch 2: Rework & fix VQs destruction loop (Laszlo) > Changes since v1: > ================= > - Patch 1: shift bits in the right direction (Ladi) > > Maxime Coquelin (4): > vhost: prevent features to be changed while device is running > vhost: propagate VHOST_USER_SET_FEATURES handling error > vhost: extract virtqueue cleaning and freeing functions > vhost: destroy unused virtqueues when multiqueue not negotiated > > lib/librte_vhost/vhost.c | 22 ++++++++++++---------- > lib/librte_vhost/vhost.h | 3 +++ > lib/librte_vhost/vhost_user.c | 39 +++++++++++++++++++++++++++++++++++++-- > 3 files changed, 52 insertions(+), 12 deletions(-) > I have no background in DPDK (as I've stated) and I didn't look at a wider code context than what is captured in the patches. So whatever opinion I hold may have little value. With that disclaimer, if you want it: Acked-by: Laszlo Ersek Thanks, Laszlo