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 A6831271 for ; Wed, 13 Dec 2017 11:11:13 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A20C8C04AC45; Wed, 13 Dec 2017 10:11:12 +0000 (UTC) Received: from [10.36.112.48] (ovpn-112-48.ams2.redhat.com [10.36.112.48]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A1EE25C544; Wed, 13 Dec 2017 10:11:05 +0000 (UTC) To: Paolo Bonzini , dev@dpdk.org, yliu@fridaylinux.org, tiwei.bie@intel.com, jianfeng.tan@intel.com, lprosek@redhat.com, lersek@redhat.com, "Michael S. Tsirkin" References: <20171213085109.9891-1-maxime.coquelin@redhat.com> From: Maxime Coquelin Message-ID: Date: Wed, 13 Dec 2017 11:11:04 +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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 13 Dec 2017 10:11:12 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v5 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, 13 Dec 2017 10:11:14 -0000 Hi Paolo, On 12/13/2017 10:15 AM, Paolo Bonzini wrote: > On 13/12/2017 09:51, Maxime Coquelin wrote: >> 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). > > Hi Maxime, > > I think this series is wrong from the virtio spec's point of view. If > the driver requests VIRTIO_NET_F_MQ, that does not mean "the driver > knows about multiqueue", it only means that "the driver wants to read > max_virtqueue_pairs" from configuration space. Actually, my series fixes half of the problem, the case where driver does not know about multiqueue. In this case, there is no point in the backend to wait for the initialization of queues that does not exist. So I think my series is not enough, but not wrong. > Just like it's perfectly fine for a device to propose VIRTIO_NET_F_MQ > and set max_virtqueue_pairs=1, a driver can negotiate VIRTIO_NET_F_MQ > and then skip initialization of some virtqueues. > > In fact, for virtio-net there is an explicit way to say how many > virtqueue pairs are available; the virtio spec's section 5.1.5 (Network > device, Device Initialization) mentions that > > Even with VIRTIO_NET_F_MQ, only receiveq1, transmitq1 and > controlq are used by default. The driver would send the > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command specifying the number of > the transmit and receive queues to use. Yes, I agree. I was planning to send a vhost-user protocol spec update to forward this information to the backend with a new request. Currently, DPDK will increment the queue counter each time it receives a request for a new queue. Then it waits for all queues to be initialized (but not necessarily enabled) to start the vhost device. The problem is that QEMU, when receiving the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command, will send VHOST_USER_SET_VRING_ENABLE request to the backend to enable first queue pair, and disable all others. We cannot destroy a queue on disable, because it could happen in other cases, where it would be re-enabled without being re-initialized. So on DPDK side, my understanding is that we cannot deduce the number of queues that we have to wait to be initialized before starting the device. DPDK currently assume a queue to be initialized if rings addresses are set and if it has received both call and kick fds. I only fixed half of the problem as a first step because current Kernel & DPDK virtio-net drivers always allocate the maximum number of queues exposed by QEMU, even if it does use them all. But it is not compliant with the Virtio spec, which does not imply this (and the spec is right). Thanks, Maxime > Thanks, > > Paolo >