From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bernard.iremonger@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id A2CD61B4C0
 for <dev@dpdk.org>; Tue, 23 Oct 2018 14:39:46 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga006.fm.intel.com ([10.253.24.20])
 by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 23 Oct 2018 05:39:45 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.54,416,1534834800"; d="scan'208";a="274849069"
Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28])
 by fmsmga006.fm.intel.com with ESMTP; 23 Oct 2018 05:39:44 -0700
Received: from irsmsx107.ger.corp.intel.com ([169.254.10.56]) by
 irsmsx105.ger.corp.intel.com ([169.254.7.89]) with mapi id 14.03.0319.002;
 Tue, 23 Oct 2018 13:39:43 +0100
From: "Iremonger, Bernard" <bernard.iremonger@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>
CC: "dev@dpdk.org" <dev@dpdk.org>, "gaetan.rivet@6wind.com"
 <gaetan.rivet@6wind.com>, "ophirmu@mellanox.com" <ophirmu@mellanox.com>,
 "wisamm@mellanox.com" <wisamm@mellanox.com>, "Yigit, Ferruh"
 <ferruh.yigit@intel.com>, "arybchenko@solarflare.com"
 <arybchenko@solarflare.com>
Thread-Topic: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching
 device twice
Thread-Index: AQHUaqqGgoRGowT2F06TFSecGoYGl6Uskf7QgAAYr4CAAALwAIAAFyVg
Date: Tue, 23 Oct 2018 12:39:42 +0000
Message-ID: <8CEF83825BEC744B83065625E567D7C260D15681@IRSMSX107.ger.corp.intel.com>
References: <20181007222554.4886-1-thomas@monjalon.net>
 <8CEF83825BEC744B83065625E567D7C260D15558@IRSMSX107.ger.corp.intel.com>
 <3243790.6gorE4o5SW@xps> <2058868.Os1dUGhjLa@xps>
In-Reply-To: <2058868.Os1dUGhjLa@xps>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmIwMzkyNjctNTZiZC00NWNmLTk5ZGMtYTdjZTU1NWVmZTlmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiMDNYclVGcWFRQkJlS3MrN0FySnB6ZHd0MmNJK0pSYkZpQUw1QzNwS3dRSVM1QzUxbjlzb2RKb3lIakVCd21MQiJ9
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.400.15
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
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:39:47 -0000

Hi Thomas

<snip>
> Subject: Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching
> device twice
>=20
> 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() ?
>=20
> Or detach_port_device()?

detach_port_device() looks fine to me.

>=20
> > [...]
> > > > -	if (rte_dev_remove(rte_eth_devices[port_id].device) !=3D 0) {
> > > > +	if (rte_dev_remove(dev) !=3D 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

Looks fine to me.

Regards,

Bernard
=20