DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed device
@ 2018-09-07 23:09 Thomas Monjalon
  2018-09-13  6:29 ` Ophir Munk
                   ` (4 more replies)
  0 siblings, 5 replies; 58+ messages in thread
From: Thomas Monjalon @ 2018-09-07 23:09 UTC (permalink / raw)
  To: dev; +Cc: gaetan.rivet

In the devargs syntax for device representors, it is possible to add
several devices at once: -w dbdf,representor=[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.

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.

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 != 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 -EEXIST.
+ *   If false, then skip the already probed device without returning an 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

^ permalink raw reply	[flat|nested] 58+ messages in thread
* Re: [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed device
@ 2018-09-12 22:50 Ophir Munk
  0 siblings, 0 replies; 58+ messages in thread
From: Ophir Munk @ 2018-09-12 22:50 UTC (permalink / raw)
  To: dev, Gaetan Rivet; +Cc: Olga Shern, Thomas Monjalon, Shahaf Shuler, Asaf Penso


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Saturday, September 08, 2018 12: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=[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.
> 

When having devargs with representors ("dbdf,representor=<args>") there is actually just one PCI device to probe (whose address is "dbdf", the master) 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 appears while when executing "ifconfig" - all representors are shown as net devices.
When calling rte_eal_hotplug_add() for the first time there is a flow which 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 failures till we reach the PMD probe callback.

Skipping failures can done as follows:
1. In file ./lib/librte_eal/common/eal_common_dev.c, function: rte_eal_hotplug_add(), remove the following code:

if (dev->driver != NULL) {
	RTE_LOG(ERR, EAL, "Device is already plugged\n");
	return -EEXIST}

2. In file ./drivers/bus/pci/pci_common.cm function: pci_probe_all_drivers(), remove the following code:

/* Check if a driver is already loaded */
if (dev->driver != NULL)
	return -1;


However the substantial major changes are in each individual PMD probe callback when it is called several times with different devargs. For example it should not fail an already probed PCI device and just create new eth devices for new representors.

> 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.
> 

Please note this new parameter ("fail_existing") will have to be propagated to all PMD probe callbacks.
Otherwise, in case (fail_existing == false) the second call to rte_eal_hotplug_add() will call the PMD probe callback, which may fail unless it is aware of "fail_existing" parameter.
Alternatively "fail_existing" may be better named "enable_multi_probe".
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.

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 exist - then removing just representor 1 will have to make sure that the PCI device used for both representors is not unplugged since representor 2 is not removed and it uses the same PCI device as representor 1. 

> 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 != 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 -EEXIST.
> + *   If false, then skip the already probed device without returning an 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

^ permalink raw reply	[flat|nested] 58+ messages in thread

end of thread, other threads:[~2018-10-17 11:37 UTC | newest]

Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07 23:09 [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed device Thomas Monjalon
2018-09-13  6:29 ` Ophir Munk
2018-09-16 10:14   ` Ophir Munk
2018-09-28 16:40 ` [dpdk-dev] [PATCH v2 0/3] " Thomas Monjalon
2018-09-28 16:40   ` [dpdk-dev] [PATCH v2 1/3] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-09-28 16:40   ` [dpdk-dev] [PATCH v2 2/3] eal: add function to query device status Thomas Monjalon
2018-09-28 16:40   ` [dpdk-dev] [PATCH v2 3/3] eal: allow probing a device again Thomas Monjalon
2018-10-04  9:44     ` Doherty, Declan
2018-10-04 14:25       ` Thomas Monjalon
2018-10-07 22:09 ` [dpdk-dev] [PATCH v3 0/3] eal: allow hotplug to skip an already probed device Thomas Monjalon
2018-10-07 22:09   ` [dpdk-dev] [PATCH v3 1/3] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-10-08  8:05     ` Andrew Rybchenko
2018-10-11 10:53     ` Andrew Rybchenko
2018-10-11 11:45       ` Thomas Monjalon
2018-10-11 11:54         ` Andrew Rybchenko
2018-10-11 12:59           ` Thomas Monjalon
2018-10-11 13:15             ` Andrew Rybchenko
2018-10-11 15:29               ` Thomas Monjalon
2018-10-11 15:41                 ` Andrew Rybchenko
2018-10-11 16:00                   ` Thomas Monjalon
2018-10-07 22:09   ` [dpdk-dev] [PATCH v3 2/3] eal: add function to query device status Thomas Monjalon
2018-10-08  8:05     ` Andrew Rybchenko
2018-10-07 22:09   ` [dpdk-dev] [PATCH v3 3/3] eal: allow probing a device again Thomas Monjalon
2018-10-08  8:05     ` Andrew Rybchenko
2018-10-11 21:02 ` [dpdk-dev] [PATCH v4 0/4] eal: allow hotplug to skip an already probed device Thomas Monjalon
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 1/4] ethdev: rename memzones allocated for DMA Thomas Monjalon
2018-10-12  7:53     ` Andrew Rybchenko
2018-10-12 16:40       ` Thomas Monjalon
2018-10-12 16:42         ` Andrew Rybchenko
2018-10-12 16:46           ` Andrew Rybchenko
2018-10-12 17:18             ` Thomas Monjalon
2018-10-12 17:21               ` Thomas Monjalon
2018-10-12 17:51                 ` Andrew Rybchenko
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 2/4] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-10-12  7:44     ` Andrew Rybchenko
2018-10-12  8:32       ` Jan Remeš
2018-10-12 10:45         ` Thomas Monjalon
2018-10-12 15:50       ` Thomas Monjalon
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 3/4] eal: add function to query device status Thomas Monjalon
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 4/4] eal: allow probing a device again Thomas Monjalon
2018-10-12  9:26   ` [dpdk-dev] [PATCH v4 0/4] eal: allow hotplug to skip an already probed device Andrew Rybchenko
2018-10-14 20:47 ` [dpdk-dev] [PATCH v5 0/7] " Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 1/7] net/mlx5: remove useless driver name comparison Thomas Monjalon
2018-10-14 20:49     ` Thomas Monjalon
2018-10-15  5:53       ` Shahaf Shuler
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 2/7] ethdev: rename memzones allocated for DMA Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 3/7] cryptodev: remove driver name from logs Thomas Monjalon
2018-10-15  8:51     ` Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 4/7] compressdev: " Thomas Monjalon
2018-10-15  8:51     ` Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 5/7] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-10-14 20:53     ` Thomas Monjalon
2018-10-15  6:11       ` Xu, Rosen
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 6/7] eal: add function to query device status Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 7/7] eal: allow probing a device again Thomas Monjalon
2018-10-16 10:40     ` Shreyansh Jain
2018-10-17 11:37   ` [dpdk-dev] [PATCH v5 0/7] allow hotplug to skip an already probed device Thomas Monjalon
2018-09-12 22:50 [dpdk-dev] [RFC] eal: " Ophir Munk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).