From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 044B65A65 for ; Mon, 26 Oct 2015 07:29:01 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 25 Oct 2015 23:29:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,200,1444719600"; d="scan'208";a="587971638" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by FMSMGA003.fm.intel.com with ESMTP; 25 Oct 2015 23:29:00 -0700 Date: Mon, 26 Oct 2015 14:30:07 +0800 From: Yuanhan Liu To: Tetsuya Mukawa Message-ID: <20151026063007.GZ3115@yliu-dev.sh.intel.com> References: <55C4E8E1.9090406@siemens.com> <2350656.p07ll6Er1F@xps13> <562DBFFF.7060808@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <562DBFFF.7060808@igel.co.jp> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: dev@dpdk.org, Jan Kiszka Subject: Re: [dpdk-dev] [PATCH] vchost: Notify application of ownership change X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Oct 2015 06:29:02 -0000 On Mon, Oct 26, 2015 at 02:54:07PM +0900, Tetsuya Mukawa wrote: > On 2015/10/25 2:16, Thomas Monjalon wrote: > > 2015-08-12 03:34, Xie, Huawei: > >> On 8/8/2015 1:21 AM, Jan Kiszka wrote: > >>> On VHOST_*_RESET_OWNER, we reinitialize the device but without telling > >>> the application. That will cause crashes when it continues to invoke > >>> vhost services on the device. Fix it by calling the destruction hook if > >>> the device is still in use. > > [...] > >>> --- a/lib/librte_vhost/virtio-net.c > >>> +++ b/lib/librte_vhost/virtio-net.c > >>> @@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx) > >>> > >>> ll_dev = get_config_ll_entry(ctx); > >>> > >>> + if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING)) > >>> + notify_ops->destroy_device(&ll_dev->dev); > >> To me this patch makes sense here. > >> Whether RESET_OWNER is really needed is another question. Whenever the > >> vhost itself needs to process the vhost device, we need to notify the > >> switch application to remove it from data plane. > > Huawei, > > some patches have been accepted for RESET_OWNER management. > > Is this patch obsolete? I think it's still appliable, at least so far. > > Hi Yuanhan and Huawei, > > I also have the same question. Do we have a patch for this issue? > > Today, I've download Yuanhan's multiple queues patches and applied it on > latest dpdk tree. > Then, tried to apply my vhost PMD patch on it. > > When I check the patch, it seems I've faced this issue. > Here are steps to reproduce. Above patch should fix your issue, right? If so, we need it. > > 1. Start vhost-user backend application. > (In my case, testpmd using vhost PMD is the application) > 2. Start a VM with vhost-user. > You can see below message from the backend application. > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE > VHOST_CONFIG: set queue enable: 1 to qp idx: 0 > (snip) > VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK > 3. After booting Linux on guest, bind the virtio-net device to igb_uio. > Then below messages are shown. > VHOST_CONFIG: read message VHOST_USER_RESET_OWNER > VHOST_CONFIG: read message VHOST_USER_RESET_OWNER > VHOST_CONFIG: read message VHOST_USER_GET_VRING_BASE > > The point is we will have VHOST_USER_RESET_OWNER before > VHOST_USER_GET_VRING_BASE. Note that there is an ongoing work at QEMU community (from me) to handle RESET_OWNER correctly: it will be moved to somewhere else instead of before VHOST_USER_GET_VRING_BASE. --yliu > Currently, in RESET_OWNER function, all virtio-net data is initialized. > As a result, we also initialize virtio-net flags. > When we get GET_VRING_BASE, we cannot call destroy callback handler > because RUNNING flag has been initialized already. > > I guess when we get RESET_OWNER message, I don't need to do anything. > And all finalizations should be done in GET_VRING_BASE. > (Or some finalizations might be done when next SET_MEM_TABLE is called.) > > Thanks, > Tetsuya