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 14:54:07 +0900 Message-ID: <562DBFFF.7060808@igel.co.jp> References: <55C4E8E1.9090406@siemens.com> <2350656.p07ll6Er1F@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, Jan Kiszka , "Liu, Yuanhan" To: "Xie, Huawei" Return-path: Received: from mail-pa0-f47.google.com (mail-pa0-f47.google.com [209.85.220.47]) by dpdk.org (Postfix) with ESMTP id 23E1D5A64 for ; Mon, 26 Oct 2015 06:54:11 +0100 (CET) Received: by pasz6 with SMTP id z6so177502436pas.2 for ; Sun, 25 Oct 2015 22:54:10 -0700 (PDT) In-Reply-To: <2350656.p07ll6Er1F@xps13> 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/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? 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. 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. 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