From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 66C0B2BC9;
 Tue, 22 May 2018 15:12:31 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id D0A95218DB;
 Tue, 22 May 2018 09:12:30 -0400 (EDT)
Received: from mailfrontend1 ([10.202.2.162])
 by compute1.internal (MEProxy); Tue, 22 May 2018 09:12:30 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 cc:content-transfer-encoding:content-type:date:from:in-reply-to
 :message-id:mime-version:references:subject:to:x-me-sender
 :x-me-sender:x-sasl-enc; s=mesmtp; bh=yM72zPR0t+FecUhP8YlgBtrRAA
 2ngrKXNy9xy+6GYUQ=; b=G3tYLolJO0rOEGX8QC9k3HOcKOcw7iWz0eZhwV5aBy
 uOw3kOu3j91tg5v9dws8Ei3Iux65jYo7sa4lsdmlxi1p6F/x1+gsl70++6W7BZgB
 UFJ8gL4pLilQR2WdjRqx7dWIEj68mbJbkLjQkIWlNPQP9Hstn4uSzuMKEHHfjlrM
 w=
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=yM72zP
 R0t+FecUhP8YlgBtrRAA2ngrKXNy9xy+6GYUQ=; b=mbpy0RoOmsGMagBCKNMBJL
 GtV3xvoaJZ9zlgtA34Ms0l8wILiE8Q40FaEEYFe0FGQm1gS5f4p3WVTOqT7f3YUt
 pWbwv8xDsy0U1Dg367zfR5pKw5FOY3vm7FQXkO6WCk2dU8Xde31NdPHXsDYiBrCr
 O5ntfW2zWrHFO/yxf6vnMbTCQGADAm+Ok9I50dbGfYUo3rq1RcStyMTvBmQvTTAQ
 cQrKNfQqGrgJqf0gF16ut73bLAv92bWIJ0hjmfTkHrn27syYbcCkYN3Sq/EZkTG8
 DEhyRjwgWXPnQCBtINUmIqqisRu/z+jeTx+M0QfiwlFaIvUtZ/iPgsBKvIw9Q1dw
 ==
X-ME-Proxy: <xmx:PhcEW6_w_QrUrvoaNDwm0cD8psOts5LX2_Tx49MAjgiGTxovDb9C1g>
X-ME-Proxy: <xmx:PhcEWy8PFUsxE6CMWeQvWi5Rr6N5K9L2TDXYmRZUSt3ul8rpawBvTg>
X-ME-Proxy: <xmx:PhcEW90TWiyTWWKTwGhjBNXJF4Up0_w6VKU7jjVIxYxaLN76aUD_mA>
X-ME-Proxy: <xmx:PhcEW3ZHyBoabqIL4uwStcGHfjz5w6BmKiifSPxk0tbPriEPRmY0hw>
X-ME-Proxy: <xmx:PhcEW5tf1IJkpZf0PTGTavluN2Yl_-vVxYAC2CygMEzEASx4XYt70A>
X-ME-Proxy: <xmx:PhcEW4mzNZHX59doySKCbTjgLXaB8BNxC7aNnVI5QTB5KTZl1EPcLg>
X-ME-Sender: <xms:PhcEW3dNmKNv6qkjzShv3Pxg547hz04ZmMRf9xruvJG7eRggDm-JkQ>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 789DFE490E;
 Tue, 22 May 2018 09:12:29 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Ferruh Yigit <ferruh.yigit@intel.com>, "Yang,
 Zhiyong" <zhiyong.yang@intel.com>
Cc: dev@dpdk.org, Matan Azrad <matan@mellanox.com>, "Iremonger,
 Bernard" <bernard.iremonger@intel.com>, "Yao, Lei A" <lei.a.yao@intel.com>,
 "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>, "Bie,
 Tiwei" <tiwei.bie@intel.com>, "stable@dpdk.org" <stable@dpdk.org>,
 Harry Van Haaren <harry.van.haaren@intel.com>
Date: Tue, 22 May 2018 15:12:28 +0200
Message-ID: <9489790.cv73CZovxU@xps>
In-Reply-To: <1865181.OGjELPnye7@xps>
References: <20180518095937.28710-1-zhiyong.yang@intel.com>
 <cc9122dd-3866-bb99-9815-b1e1e6461ca9@intel.com> <1865181.OGjELPnye7@xps>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: fix pmd_test_exit function
	for vdevs
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 22 May 2018 13:12:31 -0000

Any update to improve this workaround?

21/05/2018 18:38, Thomas Monjalon:
> 21/05/2018 18:32, Ferruh Yigit:
> > On 5/21/2018 11:54 AM, Thomas Monjalon wrote:
> > > 19/05/2018 16:19, Thomas Monjalon:
> > >> 18/05/2018 18:29, Ferruh Yigit:
> > >>> On 5/18/2018 4:55 PM, Matan Azrad wrote:
> > >>>> Hi all
> > >>>>
> > >>>> While this patch also applied I don't understand it.
> > >>>> Is it mandatory for each PMD to free all its resources in dev_close()?
> > >>>> Or it should be done by the rte_device remove function?
> > >>>>
> > >>>> If the resource cleanup should be done by the remove function I think it
> > >>>> should be called for all the devices (pci, vdev, etc).
> > >>>>
> > >>>> Is there an exit function for EAL to clean rte_eal_init()? If no, looks like we need it...
> > >>>
> > >>> Hi Matan,
> > >>>
> > >>> I believe there is a gap in resource cleanup.
> > >>> dev_close() it not for resource cleanup, it should be in PMD remove() functions,
> > >>> and PMDs have it. The problem is remove path is not called in application exit.
> > >>>
> > >>> As far as I know there is no simple API to clean the resources, having it may
> > >>> help application to do the cleanup.
> > >>>
> > >>> I have seen the rte_eal_cleanup() API by Harry, that can be extended to cover
> > >>> PMD resource cleanup if there is enough motivation for it.
> > >>
> > >> Yes, EAL resources should be removed by the function rte_eal_cleanup().
> > >> And ethdev ports must be removed by rte_eth_dev_close().
> > > 
> > > This patch is relying on rte_eth_dev_detach() to remove the EAL device.
> > > It should be done in rte_eal_cleanup().
> > > 
> > > I am concerned that this patch is workarounding a miss in rte_eal_cleanup,
> > > and takes a different action only for vdev. It is a bad example.
> > 
> > Indeed it does workaround, but it is needed to fix a defect in virtio-user.
> 
> The defect is still in virtio-user after this patch.
> To make this workaround acceptable, you need:
> 	1/ add the virtio-user known issue in release notes
> 	2/ add a FIXME comment in testpmd code explaining the workaround
> 	3/ commit to work on rte_eal_cleanup() in 18.08
> 
> > And currently rte_eal_cleanup() is not complete, it is not doing any device
> > related cleanup.
> 
> Yes, we need to add more code in rte_eal_cleanup().
> 
> > > And the function rte_eth_dev_detach() is fundamentally wrong and should be deprecated:
> > > 	http://dpdk.org/commit/b05b444d22
> > > 	http://dpdk.org/commit/b0fb266855
> > > 	http://dpdk.org/commit/df3e8ad73f
> > > 
> > > One more concern: it seems this patch is breaking failsafe use case.
> > > Note: bonding is managed as an exception in rte_eth_dev_detach().
> 
> I have sent a fix in vdev code for the failsafe issue.
> 
> No matter the vdev fix is applied or not, this workaround could target only virtio-user.