From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4CC35A0555; Wed, 19 Feb 2020 23:17:12 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 22E0A1B13C; Wed, 19 Feb 2020 23:17:11 +0100 (CET) Received: from mail.valinux.co.jp (mail.valinux.co.jp [210.128.90.3]) by dpdk.org (Postfix) with ESMTP id 628401DBD for ; Wed, 19 Feb 2020 23:17:09 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by mail.valinux.co.jp (Postfix) with ESMTP id 03DB4B5EA8; Thu, 20 Feb 2020 07:17:08 +0900 (JST) X-Virus-Scanned: Debian amavisd-new at valinux.co.jp Received: from mail.valinux.co.jp ([127.0.0.1]) by localhost (mail.valinux.co.jp [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ulfTvaFB9lof; Thu, 20 Feb 2020 07:17:07 +0900 (JST) Received: from [127.0.0.1] (vagw.valinux.co.jp [210.128.90.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.valinux.co.jp (Postfix) with ESMTPS id D561DB5E4E; Thu, 20 Feb 2020 07:17:07 +0900 (JST) Date: Thu, 20 Feb 2020 07:17:07 +0900 From: Itsuro ODA To: Maxime Coquelin Cc: Tiwei Bie , dev@dpdk.org, yinan.wang@intel.com, amorenoz@redhat.com, david.marchand@redhat.com In-Reply-To: <65a4582d-14f6-9833-0d15-b91052972e09@redhat.com> References: <20200219034357.GA975055@___> <65a4582d-14f6-9833-0d15-b91052972e09@redhat.com> Message-Id: <20200220071707.E1BE.277DD91C@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Mailer: Becky! ver. 2.74.02 [ja] Subject: Re: [dpdk-dev] [PATCH 2/2] net/vhost: prevent multiple setup on reconfig 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Maxime, Tiewi, On Wed, 19 Feb 2020 09:17:41 +0100 Maxime Coquelin wrote: > Hi Tiwei, > > On 2/19/20 4:44 AM, Tiwei Bie wrote: > > On Tue, Feb 18, 2020 at 06:22:40PM +0100, Maxime Coquelin wrote: > >> Ethdev's .dev_configure callback can be called multiple > >> time during a device life-time, but Vhost makes the > >> wrong assumption that it is not the case and try to > >> setup again the device on reconfiguration. > >> > >> This patch ensures the device hasn't been already setup > >> before proceeding. > >> > >> Fixes: 3d01b759d267 ("net/vhost: delay driver setup") > >> > >> Reported-by: Yinan Wang > >> Signed-off-by: Maxime Coquelin > >> Tested-by: Yinan Wang > >> Reviewed-by: David Marchand > >> --- > >> drivers/net/vhost/rte_eth_vhost.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c > >> index 4a7b1b608c..458ed58f5f 100644 > >> --- a/drivers/net/vhost/rte_eth_vhost.c > >> +++ b/drivers/net/vhost/rte_eth_vhost.c > >> @@ -876,6 +876,11 @@ vhost_driver_setup(struct rte_eth_dev *eth_dev) > >> unsigned int numa_node = eth_dev->device->numa_node; > >> const char *name = eth_dev->device->name; > >> > >> + /* Don't try to setup again if it has already been done. */ > >> + list = find_internal_resource(internal->iface_name); > >> + if (list) > >> + return 0; > >> + > >> list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node); > >> if (list == NULL) > >> return -1; > >> -- > >> 2.24.1 > > > > Thanks Maxime for the fix! > > > > Reviewed-by: Tiwei Bie > > > > Not related to this fix, it seems there is a potential leak after > > delaying the driver setup to _configure, as the list won't be > > registered until users configure the device. So internal->iface_name > > allocated in _create will leak if the device is released without > > doing _configure first. > > > > https://github.com/DPDK/dpdk/blob/e6c78e736d77/drivers/net/vhost/rte_eth_vhost.c#L1058 > > https://github.com/DPDK/dpdk/blob/e6c78e736d77/drivers/net/vhost/rte_eth_vhost.c#L1075 > > > > It's not a common case and it's quite late in this release, > > probably it's fine to fix it later. > > That's a valid point, and I also agree there is no urgency for v20.02. > Itsuro, would you take care of fixing it for v20.05? Sure, I will do it. Thanks. Itsuro Oda > Thanks, > Maxime > > Thanks, > > Tiwei > > -- Itsuro ODA