From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com
 [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 914301B426
 for <dev@dpdk.org>; Tue, 23 Oct 2018 14:37:39 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id 18BE121D25;
 Tue, 23 Oct 2018 08:37:39 -0400 (EDT)
Received: from mailfrontend1 ([10.202.2.162])
 by compute1.internal (MEProxy); Tue, 23 Oct 2018 08:37:39 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 from:to:cc:subject:date:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding:content-type; s=mesmtp;
 bh=wvUFnGXmhaGUnGP6ivweWQleEg0Rgx+zH+oNI1ll1qs=; b=PvZoxLNPHJpM
 pR15lg7Le2Kix4MSMyhEiAqcpdcrvfOSNEOBW5cRKnxcSWGxVNPyTvitIQrgI6Lg
 WwvfxmPVnpKHjQbqITLj5JaihB3jHSslQmjneX2Jx7AmX+lZUzG1MIZCAxJhap6i
 SsbbH0UrLikWWT11lIfgStIWabfdcVw=
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-proxy:x-me-proxy:x-me-sender:x-me-sender
 :x-sasl-enc; s=fm1; bh=wvUFnGXmhaGUnGP6ivweWQleEg0Rgx+zH+oNI1ll1
 qs=; b=bKgDgEa+CrNvnqnBI8qfaxjWTOCpN2S+3qD15u0limUn9Lq1mRyqMeBm+
 U/+evI+fktUSDK4bbz2xSH1WcL5L1VigcPlyGNc2jot59jG4+nCbqBhoGgl0QAoH
 7ZfXwEYTt0Dv+MZPr8CktFUZFbFvPO1H2b90/xoVgOuw1Yr8fZXV23BLp0Wbd9s0
 1DBy6xVi7FgXbETAOZhBMN25cGvBysEBNGHyepgblEooRXeIbcTxrikZQykL4Ht0
 96YE+wuyU9vmUIsmSm61Ux56aMTot815ykraaIPiJI1fPTWhv5qex75B0+mpsDur
 25jhWHhqeMjKGVPsJrKWpb4IPzh/A==
X-ME-Sender: <xms:EhbPW-OFJkhP1goVHwocLPcpx0IDY2IAmyqmJu7Xn1nYehzczLCTDg>
X-ME-Proxy: <xmx:EhbPW7HwLukwKOaEsG-WvtV_MrTxo2WmkPhqqDDTM_QZgUoAAxapCw>
 <xmx:EhbPWw1D-rNige50Cl8kPwZXpO4fDUzDRIj-ob4PVs8QABINuDAwLg>
 <xmx:EhbPWydXBfmySRO31YL78AbhlN_3ijUXUw3FXtU6gEdWCD1rYJYQeQ>
 <xmx:EhbPWx30pUoTnCcMQq4WpdBKPxndkkBGD0iLwcfc39DilJAqPk5i-g>
 <xmx:EhbPW4dqZhP-9RA42qgr1phVCi31EwXeGi9NUBJQ-wis_jKKxOy87A>
 <xmx:ExbPW8s6ehrYMXTHbxHezy49d_xhmnLwwFf1lLl44cSdwvOTQd5qfQ>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 565D7E484F;
 Tue, 23 Oct 2018 08:37:37 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: "Iremonger, Bernard" <bernard.iremonger@intel.com>, "Yigit,
 Ferruh" <ferruh.yigit@intel.com>
Cc: dev@dpdk.org, "gaetan.rivet@6wind.com" <gaetan.rivet@6wind.com>,
 "ophirmu@mellanox.com" <ophirmu@mellanox.com>,
 "wisamm@mellanox.com" <wisamm@mellanox.com>,
 "arybchenko@solarflare.com" <arybchenko@solarflare.com>
Date: Tue, 23 Oct 2018 14:37:38 +0200
Message-ID: <1721220.7rV5GyacGl@xps>
In-Reply-To: <2058868.Os1dUGhjLa@xps>
References: <20181007222554.4886-1-thomas@monjalon.net>
 <3243790.6gorE4o5SW@xps> <2058868.Os1dUGhjLa@xps>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching
	device twice
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://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 23 Oct 2018 12:37:39 -0000

I want to submit two more patches to clean testpmd for attach/detach.

I propose to drop this patch from this series,
and I will submit a new series dedicated to testpmd cleanup,
including this patch.


23/10/2018 14:13, Thomas Monjalon:
> 23/10/2018 14:03, Thomas Monjalon:
> > 23/10/2018 12:01, Iremonger, Bernard:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > The command "port detach" is removing the EAL rte_device of the ethdev
> > > > port specified as parameter.
> > > > 
> > > > After detaching, the pointer, which maps a port to its device, is resetted. This
> > > 
> > > Typo:  "resetted" should be "reset"
> > > 
> > > > way, it is possible to check whether a port is still associated to a (not
> > > > removed) device.
> > > > 
> > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > > ---
> > > >  app/test-pmd/testpmd.c | 24 +++++++++++++++++++++---
> > > >  1 file changed, 21 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > > 14647fa19..d5998fddc 100644
> > > > --- a/app/test-pmd/testpmd.c
> > > > +++ b/app/test-pmd/testpmd.c
> > > > @@ -2353,8 +2353,17 @@ setup_attached_port(portid_t pi)  void
> > > > detach_port(portid_t port_id)  {
> > > > +	struct rte_device *dev;
> > > > +	portid_t sibling;
> > > > +
> > > >  	printf("Removing a device...\n");
> > > 
> > > The functionality of the detach_port() function has changed now to
> > > removing a device, should the function name be changed to reflect
> > > the new functionality.
> > 
> > No it doesn't change, it has always removed the rte_device of the port.
> > But the naming is a bit strange, I agree.
> > I just changed the log to make it a bit clearer.
> > 
> > > How about detach_device() instead of detach detach_port().
> > 
> > The strange thing with testpmd is that every commands take a port id.
> > The rte_device is hidden in testpmd.
> > So the detach command is detaching the underlying device of the port,
> > and all its sibling ports of course.
> > 
> > What about detach_device_of_port() ?
> 
> Or detach_port_device()?
> 
> > [...]
> > > > -	if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
> > > > +	if (rte_dev_remove(dev) != 0) {
> > > >  		TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
> > > 
> > > Should the log message be ( ERR "Failed to detach device %s\n", dev->name) ?
> > 
> > Yes!
> > 
> > [...]
> > > > -	printf("Port %u is detached. Now total ports is %d\n",
> > > > -			port_id, nb_ports);
> > > 
> > > How about printf("Device %s is detached, Now total ports is %d\n"
> > > 	dev->name, nb_ports);
> > 
> > The issue is that we cannot get the device name after detach.
> > I can reword it differently:
> > 	Device of port %u is detached, Now total ports is %d