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 BF701A0555; Wed, 19 Feb 2020 04:44:31 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 91A081C0AC; Wed, 19 Feb 2020 04:44:30 +0100 (CET) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 20A831C07C for ; Wed, 19 Feb 2020 04:44:28 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Feb 2020 19:44:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,459,1574150400"; d="scan'208";a="229680491" Received: from dpdk-virtio-tbie-2.sh.intel.com (HELO ___) ([10.67.104.74]) by fmsmga008.fm.intel.com with ESMTP; 18 Feb 2020 19:44:25 -0800 Date: Wed, 19 Feb 2020 11:44:16 +0800 From: Tiwei Bie To: Maxime Coquelin , oda@valinux.co.jp Cc: dev@dpdk.org, yinan.wang@intel.com, amorenoz@redhat.com, david.marchand@redhat.com Message-ID: <20200219034357.GA975055@___> References: <20200218172240.558516-1-maxime.coquelin@redhat.com> <20200218172240.558516-3-maxime.coquelin@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200218172240.558516-3-maxime.coquelin@redhat.com> User-Agent: Mutt/1.9.4 (2018-02-28) 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" 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. Thanks, Tiwei