From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 128EC2C18 for ; Fri, 28 Apr 2017 04:29:28 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Apr 2017 19:29:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,386,1488873600"; d="scan'208";a="1141123646" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga001.fm.intel.com with ESMTP; 27 Apr 2017 19:29:26 -0700 Date: Fri, 28 Apr 2017 10:25:58 +0800 From: Yuanhan Liu To: Maxime Coquelin Cc: Zhiyong Yang , dev@dpdk.org, ciara.loftus@intel.com, =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , "Michael S. Tsirkin" Message-ID: <20170428022558.GM11512@yliu-dev.sh.intel.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0bb0c833-1178-c587-8085-12137ebd91d5@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) 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 02:29:29 -0000 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. > > 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. 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. --yliu