From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id 1DE722BBD for ; Wed, 23 May 2018 09:35:59 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1569FBB424; Wed, 23 May 2018 07:35:59 +0000 (UTC) Received: from [10.36.112.28] (ovpn-112-28.ams2.redhat.com [10.36.112.28]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EB08B20A847B; Wed, 23 May 2018 07:35:57 +0000 (UTC) To: luca.boccassi@gmail.com, Tomasz Kulasek Cc: Dariusz Stojaczyk , Jianfeng Tan , dpdk stable References: <20180501104509.17238-1-luca.boccassi@gmail.com> <20180501104509.17238-7-luca.boccassi@gmail.com> From: Maxime Coquelin Message-ID: <3003cecb-1c1d-c21b-8a41-0639bdd01511@redhat.com> Date: Wed, 23 May 2018 09:35:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180501104509.17238-7-luca.boccassi@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Wed, 23 May 2018 07:35:59 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Wed, 23 May 2018 07:35:59 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:'' Subject: Re: [dpdk-stable] patch 'vhost: fix device cleanup at stop' has been queued to LTS release 16.11.7 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 May 2018 07:36:00 -0000 Hi Luca, Tomasz, While testing 16.11 branch, I noticed vhost lib is broken. The symptoms is no packets are sent or received. I ran a bisect which points to this commit. Reverting it solves the issue. Debugging a bit more, I can see that the callfd is valid when no packets are being transmitted. I think the problem is that the callfd is received after the kickfd: VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK VHOST_CONFIG: vring kick idx:0 file:16 VHOST_CONFIG: virtio is not ready for processing. VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL VHOST_CONFIG: vring call idx:0 file:17 And in 16.11, the new_device callback is called from the VHOST_USER_SET_VRING_KICK handling, only if the device is ready. This is different in later versions, where the new_device callback can be called for any request. The right way to fix this would be to move the new_device callback call for any request, but I think there is a non-negligible risk of regression so we'd need to be careful doing that. Other option is to simply revert Tomasz patch in 16.11 LTS. This is not ideal because Tomasz patch is fixing a real issue (new_device get called with an outdated/invalid callfd). However, I don't know if it has real consequences, as the callfd is updated right after. Except if someone face real issue due to callfd being updated after new_device is called, then my suggestion would be to revert Tomasz patch as we are at 6 months of 16.11 EOL. Luca, Tomasz, what's your take on this? Regards, Maxime On 05/01/2018 12:44 PM, luca.boccassi@gmail.com wrote: > Hi, > > FYI, your patch has been queued to LTS release 16.11.7 > > Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. > It will be pushed if I get no objections before 05/03/18. So please > shout if anyone has objections. > > Thanks. > > Luca Boccassi > > --- > From 357f27736c79d0258409666277b6e113d11c757b Mon Sep 17 00:00:00 2001 > From: Tomasz Kulasek > Date: Fri, 9 Feb 2018 18:10:00 +0100 > Subject: [PATCH] vhost: fix device cleanup at stop > > [ upstream commit ace7b6b7859e1dc410589610a8e436c1a3b430f3 ] > > This prevents from destroying & recreating user device in "incomplete" > vring state. virtio_is_ready() was returning true for devices with > vrings which did not have valid callfd (their VHOST_USER_SET_VRING_CALL > hasn't arrived yet) > > Fixes: 8f972312b8f4 ("vhost: support vhost-user") > > Signed-off-by: Dariusz Stojaczyk > Signed-off-by: Tomasz Kulasek > Reviewed-by: Jianfeng Tan > Reviewed-by: Maxime Coquelin > --- > lib/librte_vhost/vhost_user.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index abcb6ac14..0d30f11ef 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -800,6 +800,11 @@ vhost_user_get_vring_base(struct virtio_net *dev, > > vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD; > > + if (vq->callfd >= 0) > + close(vq->callfd); > + > + vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD; > + > if (dev->dequeue_zero_copy) > free_zmbufs(vq); > rte_free(vq->shadow_used_ring); >