From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ophirmu@mellanox.com>
Received: from EUR04-DB3-obe.outbound.protection.outlook.com
 (mail-eopbgr60074.outbound.protection.outlook.com [40.107.6.74])
 by dpdk.org (Postfix) with ESMTP id 9801A2BD8
 for <dev@dpdk.org>; Sun, 16 Sep 2018 12:14:38 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
 s=selector1;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=TETrj2Ca5yNWb8SRAIaxK1Nxm3jxtiFjK42DwXR37QQ=;
 b=jR7zP2+YqSygdJc9ieWWt5QNLNIxoDU3ZY8/NNGZqPQX/HPSYxOdJTz4ZimOf1va0MKnAngpcEw9USOXVea6PFasT/oceYn7difw98fdoOFR/bbwi3kCN30yIJMpMCt7zVGrHfDLUHi9BFvzst4n/zqA10KFB+vtkdwq29u3pFQ=
Received: from VI1PR0502MB3743.eurprd05.prod.outlook.com (52.134.8.154) by
 VI1PR0502MB3920.eurprd05.prod.outlook.com (52.134.8.157) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.1122.17; Sun, 16 Sep 2018 10:14:37 +0000
Received: from VI1PR0502MB3743.eurprd05.prod.outlook.com
 ([fe80::29c9:a95f:afaf:1b95]) by VI1PR0502MB3743.eurprd05.prod.outlook.com
 ([fe80::29c9:a95f:afaf:1b95%2]) with mapi id 15.20.1143.017; Sun, 16 Sep 2018
 10:14:37 +0000
From: Ophir Munk <ophirmu@mellanox.com>
To: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>
CC: "gaetan.rivet@6wind.com" <gaetan.rivet@6wind.com>, Olga Shern
 <olgas@mellanox.com>, Shahaf Shuler <shahafs@mellanox.com>, Asaf Penso
 <asafp@mellanox.com>
Thread-Topic: [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed
 device
Thread-Index: AQHURv/v7ja+NYglY0Cgqh67kq59O6Ttx6FwgATZPmA=
Date: Sun, 16 Sep 2018 10:14:36 +0000
Message-ID: <VI1PR0502MB374331CF1AE0304B59BF2C79D11F0@VI1PR0502MB3743.eurprd05.prod.outlook.com>
References: <20180907230958.21402-1-thomas@monjalon.net>
 <VI1PR0502MB37432D841E4B2FEA47689F48D11A0@VI1PR0502MB3743.eurprd05.prod.outlook.com>
In-Reply-To: <VI1PR0502MB37432D841E4B2FEA47689F48D11A0@VI1PR0502MB3743.eurprd05.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [193.47.165.251]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; VI1PR0502MB3920;
 6:tm7Y7joXhwtssSyL1u+obAhW9sh2RBFBlSrVWz4bv6LN2UZiSyyNbPMHyvacOr/xc+LE01r3eb77dKCW1PF0j0FNOYUB0YpUDS11NMMFjwsJaRHtnS8s8CyE5QGRftutGr4Ori7PKJ0lYvMJ9QGJbQAkgaYfP9NwJAg8TEIR5Sh63Lbb6WEoc3NzUT1eqIrrZ/OV0F5x98BXD+Luop8B+S8fekeUifk1fGv3zo570W/r7mtL9EUT7E1o4XEc3BMuYAxKqTaPC8e29LmKlMCQ0CYHDiw5Ng3O1vkukfN95FxZvNGPxkuaiVxa2GdsFmQnK27o65nCeOIdQR527XC4/ytHbkIsTDW2mHM766gAhTWhaMk7/C6omHL5H4x9KCzDt3dIvjhMk5kaYh8/A7J/fwBh0ejlipOE6doRd1ItXzSvSaZ80/HUFWzEOKx3xu4e7hya9NgWtyBjdEuIyQpREQ==;
 5:SC5mhuq7Fu0uu9o3zb6w+ddaVZjvKWjwYUCsAQE2orLftHAV41EPdBCofJbwL5MUJcbj/ZPs8PN4r+Z75JBRIgtGrliUi9x2f2fJHZhuxu2cqxbt++Q1cemUPsSaWjBWoKCCiJheVt/75Ct34PNHojUcolEaboTt42qrxYPhEbE=;
 7:XGa2/RDN/Rpw1lUvi3PCU/tZq6ngrEyLGVsvKeLjpZLKWM5Vnd7jUk9ATlEiPNHar1JQxSvHl2W4Sac3PBP+dWuXwkdgsYi9oqYLkI9YvCLPPdD4u4YD4PeGrXU0MUf041VWJrdqQhRvJq6IUp2DHhk8C+F17a5L0LoJeRc0qLMVwvKnP781JkgclIeshR94Ho5fc6h4sgWQrmtdC06qu5qb/5SR9ZY/yNdFETJ/9bQXFX5kkYycVO8ysTMsHT/5
x-ms-exchange-antispam-srfa-diagnostics: SOS;
x-ms-office365-filtering-correlation-id: f99fabf6-5df9-4a7b-23e5-08d61bbd3503
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0; PCL:0;
 RULEID:(7020095)(4652040)(8989137)(4534165)(4627221)(201703031133081)(201702281549075)(8990107)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020);
 SRVR:VI1PR0502MB3920; 
x-ms-traffictypediagnostic: VI1PR0502MB3920:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <VI1PR0502MB3920CA4D883C98DFC5928442D11F0@VI1PR0502MB3920.eurprd05.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:;
x-ms-exchange-senderadcheck: 1
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0;
 RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231355)(944501410)(52105095)(3002001)(93006095)(93001095)(10201501046)(6055026)(149027)(150027)(6041310)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123558120)(20161123562045)(201708071742011)(7699050)(76991041);
 SRVR:VI1PR0502MB3920; BCL:0; PCL:0; RULEID:; SRVR:VI1PR0502MB3920; 
x-forefront-prvs: 079756C6B9
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10009020)(346002)(366004)(396003)(39860400002)(376002)(136003)(13464003)(199004)(189003)(6246003)(446003)(486006)(476003)(478600001)(4326008)(99286004)(2900100001)(86362001)(8676002)(7736002)(107886003)(14444005)(305945005)(25786009)(11346002)(105586002)(97736004)(74316002)(256004)(53546011)(106356001)(102836004)(5660300001)(76176011)(316002)(7696005)(110136005)(26005)(5250100002)(2906002)(6116002)(6506007)(229853002)(33656002)(8936002)(81166006)(81156014)(3846002)(53936002)(6346003)(2501003)(14454004)(6436002)(55016002)(54906003)(9686003)(186003)(66066001)(68736007);
 DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0502MB3920;
 H:VI1PR0502MB3743.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en;
 PTR:InfoNoRecords; A:1; MX:1; 
received-spf: None (protection.outlook.com: mellanox.com does not designate
 permitted sender hosts)
authentication-results: spf=none (sender IP is )
 smtp.mailfrom=ophirmu@mellanox.com; 
x-microsoft-antispam-message-info: 9EK2Q59uWyBa2T1k2BkdYIJPSoyybsQJSgY6mc9lnEpYIJzAQ3e0jtrmkFoMQVbbNMcZEptPxmqdz7gCH55nB11G64YZDigLPaT5P2s82kfpF4z0EwH1Ahv5fFctudpHVfWH21BkGc53S3YJLYMyez96rpasCs1A+L1438Se02jYFvqPrJkEUAwqd2RHKmcgV5l97eJCt1jD7ogRRMkh1OWwnH7oZsUAE7Ap804FXI9geKuQZDhjCdSth9jHRjPeyZMq9XPri6THSL+zd+5AV2H4goEdC8iIkj/U4eE1qHHDfq0aTLpKXGgatNKg6zTmdoxbQu5nFO/YLHMiEp0vwQjVH0ufKKkdQbOougfdCU8=
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f99fabf6-5df9-4a7b-23e5-08d61bbd3503
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Sep 2018 10:14:37.0350 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0502MB3920
Subject: Re: [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed
 device
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: Sun, 16 Sep 2018 10:14:39 -0000



> -----Original Message-----
> From: Ophir Munk
> Sent: Thursday, September 13, 2018 9:30 AM
> To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> Cc: gaetan.rivet@6wind.com; Olga Shern <olgas@mellanox.com>; Shahaf
> Shuler <shahafs@mellanox.com>; Asaf Penso <asafp@mellanox.com>
> Subject: RE: [dpdk-dev] [RFC] eal: allow hotplug to skip an already probe=
d
> device
>=20
>=20
>=20
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Saturday, September 08, 2018 2:10 AM
> > To: dev@dpdk.org
> > Cc: gaetan.rivet@6wind.com
> > Subject: [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed
> > device
> >
> > In the devargs syntax for device representors, it is possible to add
> > several devices at once: -w dbdf,representor=3D[0-3] It will become a
> > more frequent case when introducing wildcards and ranges in the new
> devargs syntax.
> >
> > If a devargs string is provided for probing, and updated with a bigger
> > range for a new probing, then we do not want it to fail because part
> > of this range was already probed previously.
> >
>=20
> When having devargs with representors ("dbdf,representor=3D<args>") there=
 is
> actually just one PCI device to probe (whose address is "dbdf", the maste=
r)
> while the representors themselves are net devices all using the same PCI
> "dbdf" address.
> The way to see it: when running "lspci": only the "dpdf" PCI device appea=
rs
> while when executing "ifconfig" - all representors are shown as net devic=
es.
> When calling rte_eal_hotplug_add() for the first time there is a flow whi=
ch
> eventually calls the PMD probe callback (e.g. mlx5_pci_probe() in case of
> mlx5 PMD).
> When calling rte_eal_hotplug_add() for several times we should skip failu=
res
> till we reach the PMD probe callback.
>=20
> Skipping failures can be done as follows:
> 1. In file ./lib/librte_eal/common/eal_common_dev.c, function:
> rte_eal_hotplug_add(), remove the following code:
>=20
> if (dev->driver !=3D NULL) {
> 	RTE_LOG(ERR, EAL, "Device is already plugged\n");
> 	return -EEXIST}
>=20
> 2. In file ./drivers/bus/pci/pci_common.cm function: pci_probe_all_driver=
s(),
> remove the following code:
>=20
> /* Check if a driver is already loaded */ if (dev->driver !=3D NULL)
> 	return -1;
>=20
>=20
> However the substantial major changes are in each individual PMD probe
> callback when it is called several times with different devargs. For exam=
ple it
> should not fail an already probed PCI device and just create new eth devi=
ces
> for new representors.
>=20
>=20
> > On the opposite, we could require rte_eal_hotplug_add() to try to add
> > all matching devices, and fail if one is already probed.
> >
> > That's why a new parameter is added to specify if the function must
> > fail or not when trying to add an already probed device.
> >
>=20
> Please note this new parameter ("fail_existing") will have to be propagat=
ed
> to all PMD probe callbacks.
> Otherwise, in case (fail_existing =3D=3D false) a second call to
> rte_eal_hotplug_add() will call the PMD probe callback, which may fail un=
less
> it is aware of "fail_existing" parameter.
> Alternatively "fail_existing" may be better named "enable_multi_probes".
> Anyway - if the PMD probe() callback has to be updated to return a
> success/failure value (for more than one probe) - maybe we do not need a
> new parameter and can rely on the PMD probe() callback the take the
> decision by returning success/failure value.
>=20
> The counter part of rte_eal_hotplug_add() is rte_eal_hotplug_remove()
> which must be updated as well. For example when representors 1 and 2 exis=
t
> - then removing just representor 1 will have to make sure that the PCI de=
vice
> used for both representors is not unplugged since representor 2 is not
> removed and it uses the same PCI device as representor 1.
>=20

To make it clearer: the flow initiated by calling rte_eal_hotplug_removed()=
 should eventually call the PMD remote() callback which in turn should mana=
ge a reference count to all representor devices and decide which ones shoul=
d be closed.

> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > This patch contains only the change in the function itself as RFC.
> >
> > This idea was presented at Dublin during the "hotplug talk".
> > ---
> >  lib/librte_eal/common/eal_common_dev.c  | 4 +++-
> > lib/librte_eal/common/include/rte_dev.h | 5 ++++-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_dev.c
> > b/lib/librte_eal/common/eal_common_dev.c
> > index 678dbcac7..17d7e9089 100644
> > --- a/lib/librte_eal/common/eal_common_dev.c
> > +++ b/lib/librte_eal/common/eal_common_dev.c
> > @@ -128,7 +128,7 @@ int rte_eal_dev_detach(struct rte_device *dev)  }
> >
> >  int __rte_experimental rte_eal_hotplug_add(const char *busname, const
> > char *devname,
> > -			const char *devargs)
> > +			const char *devargs, bool fail_existing)
> >  {
> >  	struct rte_bus *bus;
> >  	struct rte_device *dev;
> > @@ -173,6 +173,8 @@ int __rte_experimental rte_eal_hotplug_add(const
> > char *busname, const char *devn
> >  	}
> >
> >  	if (dev->driver !=3D NULL) {
> > +		if (!fail_existing)
> > +			return 0;
> >  		RTE_LOG(ERR, EAL, "Device is already plugged\n");
> >  		return -EEXIST;
> >  	}
> > diff --git a/lib/librte_eal/common/include/rte_dev.h
> > b/lib/librte_eal/common/include/rte_dev.h
> > index b80a80598..10a1cd2b4 100644
> > --- a/lib/librte_eal/common/include/rte_dev.h
> > +++ b/lib/librte_eal/common/include/rte_dev.h
> > @@ -201,11 +201,14 @@ int rte_eal_dev_detach(struct rte_device *dev);
> >   *   capable of handling it and pass it to the driver probing function=
.
> >   * @param devargs
> >   *   Device arguments to be passed to the driver.
> > + * @param fail_existing
> > + *   If true and a matching device is already probed, then return -EEX=
IST.
> > + *   If false, then skip the already probed device without returning a=
n
> error.
> >   * @return
> >   *   0 on success, negative on error.
> >   */
> >  int __rte_experimental rte_eal_hotplug_add(const char *busname, const
> > char *devname,
> > -			const char *devargs);
> > +			const char *devargs, bool fail_existing);
> >
> >  /**
> >   * @warning
> > --
> > 2.18.0