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 08EE02935 for ; Fri, 28 Apr 2017 09:24:02 +0200 (CEST) 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 38733B080A; Fri, 28 Apr 2017 07:24:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 38733B080A Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=maxime.coquelin@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 38733B080A Received: from [10.36.112.41] (ovpn-112-41.ams2.redhat.com [10.36.112.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 068489AE54; Fri, 28 Apr 2017 07:23:56 +0000 (UTC) To: Yuanhan Liu Cc: Zhiyong Yang , dev@dpdk.org, ciara.loftus@intel.com, =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , "Michael S. Tsirkin" References: <1493274893-40764-1-git-send-email-zhiyong.yang@intel.com> <57212715-6192-dba2-418a-2418a5d14953@redhat.com> <20170427082047.GL11512@yliu-dev.sh.intel.com> <0bb0c833-1178-c587-8085-12137ebd91d5@redhat.com> <20170428022558.GM11512@yliu-dev.sh.intel.com> From: Maxime Coquelin Message-ID: <9ecbbd0b-a2a0-7674-e84d-c8beeeeff0e6@redhat.com> Date: Fri, 28 Apr 2017 09:23:54 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 MIME-Version: 1.0 In-Reply-To: <20170428022558.GM11512@yliu-dev.sh.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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.39]); Fri, 28 Apr 2017 07:24:01 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH] vhost: fix MQ fails to startup 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: Fri, 28 Apr 2017 07:24:02 -0000 On 04/28/2017 04:25 AM, Yuanhan Liu wrote: > On Thu, Apr 27, 2017 at 10:52:20AM +0200, Maxime Coquelin wrote: >> >> >> On 04/27/2017 10:20 AM, Yuanhan Liu wrote: >>> On Thu, Apr 27, 2017 at 09:56:47AM +0200, Maxime Coquelin wrote: >>>> Hi Zhiyong, >>>> >>>> +Marc-André >>>> >>>> On 04/27/2017 08:34 AM, Zhiyong Yang wrote: >>>>> vhost since dpdk17.02 + qemu2.7 and above will cause failures of >>>>> new connection when negotiating to set MQ. (one queue pair works >>>>> well).Because there exist some bugs in qemu code when introducing >>>>> VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. when dealing with the vhost >>>>> message VHOST_USER_SET_MEM_TABLE for the second time, qemu indeed >>>>> doesn't send the messge (The message needs to be sent only once)but >>>>> still will be waiting for dpdk's reply ack, then, qemu is always >>>>> freezing. DPDK code works in the right way. >>>> >>>> I'm looking at Qemu's vhost_user_set_mem_table() function, but fail to >>>> see how it could wait for the reply-ack if it didn't send the >>>> VHOST_USER_SET_MEM_TABLE request before. >>>> >>>>> But the feature >>>>> VHOST_USER_PROTOCOL_F_REPLY_ACK has to be disabled by default at the >>>>> dpdk side in order to avoid the feature support of DPDK + qemu at >>>>> the same time. if doing like that, MQ can works well. Once Qemu bugs >>>>> have been fixed and upstreamed, we can enable it. >>>> >>>> The problem is for DPDK to detect whether bug is fixed in Qemu. >>>> Maybe only way would be to have a new protocol feature flag, which is >>>> not really its role. >>> >>> Wouldn't that be an overkill, judging that REPLY_ACK is not a must >>> feature? >> >> Yes, maybe. But it was introduced to fix (possible) race conditions: >> https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06173.html > > But AFAIK, that commit has been reverted: > > commit 94c9cb31c04737f86be29afefbff401cd23bc24d > Author: Michael S. Tsirkin > Date: Mon Aug 15 16:35:24 2016 +0300 > > Revert "vhost-user: Attempt to fix a race with set_mem_table." > > This reverts commit 28ed5ef16384f12500abd3647973ee21b03cbe23. > > I still think it's the right thing to do, but > tests have been failing sporadically. > > Revert for now, and hope to fix it before the release. No, what has been reverted is a workaround when REPLY_ACK protocol feature has not been negotiated. Instead of waiting for the backend to send the ack, the workaround consisted in sending a GET_FEATURES request after having sent the SET_MEM_TABLE request, in order to ensure SET_MEM_TABLE request handling was done before. The problem is that it sometimes created a deadlock when when running QEMU's vhost-user-test in TCG mode. >> >> Note that I planned to use this feature for the device IOTLB >> implementation to let the backend decide whether it wants the IOTLB >> misses synchronous or asynchronous. But I can still change the protocol >> spec to make this behavior specific to this request. > > Maybe we could introduce a version message? With that, we could tell > whether the frontend has fixed the known bug or not. That's a possibility, but this is not really the role of a protocol version. As in this case, the protocol does not change, just an implementation. > Note that we already has the "version" info in current vhost-user spec. > It's just 2 bits in the message "flag" field though, which is not quite > enough. Indeed, it does not let room for lots of bugs :) Thanks, Maxime > --yliu >