> -----Original Message----- > From: Ophir Munk > Sent: Thursday, September 13, 2018 9:30 AM > To: Thomas Monjalon <tho...@monjalon.net>; dev@dpdk.org > Cc: gaetan.ri...@6wind.com; Olga Shern <ol...@mellanox.com>; Shahaf > Shuler <shah...@mellanox.com>; Asaf Penso <as...@mellanox.com> > Subject: RE: [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed > device > > > > > -----Original Message----- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > > Sent: Saturday, September 08, 2018 2:10 AM > > To: dev@dpdk.org > > Cc: gaetan.ri...@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 be 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) a 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_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. > > 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. > To make it clearer: the flow initiated by calling rte_eal_hotplug_removed() should eventually call the PMD remote() callback which in turn should manage a reference count to all representor devices and decide which ones should be closed. > > Signed-off-by: Thomas Monjalon <tho...@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
