From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [PATCH] vchost: Notify application of ownership change Date: Mon, 26 Oct 2015 17:33:24 +0900 Message-ID: <562DE554.2090109@igel.co.jp> References: <55C4E8E1.9090406@siemens.com> <2350656.p07ll6Er1F@xps13> <562DBFFF.7060808@igel.co.jp> <20151026063007.GZ3115@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, Jan Kiszka To: Yuanhan Liu Return-path: Received: from mail-pa0-f48.google.com (mail-pa0-f48.google.com [209.85.220.48]) by dpdk.org (Postfix) with ESMTP id BDB695682 for ; Mon, 26 Oct 2015 09:33:27 +0100 (CET) Received: by pacfv9 with SMTP id fv9so190168017pac.3 for ; Mon, 26 Oct 2015 01:33:27 -0700 (PDT) In-Reply-To: <20151026063007.GZ3115@yliu-dev.sh.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2015/10/26 15:30, Yuanhan Liu wrote: > 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. Yes, the patch will fix the issue. >> 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 Sounds great! Thanks for handling it. Tetsuya