On Tue, Jul 11, 2017 at 7:25 PM, Gaetan Rivet <gaetan.ri...@6wind.com> wrote: > The hotplug API requires a few properties that were not previously > explicitly enforced: > > - Idempotency, two consecutive scans should result in the same state. > - Upon returning, internal devices are now allocated and available > through the new `find_device` operator, meaning that they should be > identifiable. > > The current rte_eal_hotplug_add implementation identifies devices by > their names, as it is readily available and easy to define. > > The device name must be passed to the internal rte_device handle in > order to be available during scan, when it is then assigned to the > device. The current way of passing down this information from the device > declaration is through the global rte_devargs list.
This only true for the virtual bus (vdev). > > Furthermore, the rte_device cannot take a bus-specific generated name, > as it is then not identifiable by the `find_device` operator. The device > must take the user-defined name. Ideally, an rte_device name should not > change during its existence. > > This commit generates a new rte_devargs associated with the plugged > device and inserts it in the global rte_devargs list. It consequently > releases it upon device removal. So unplugging a device which devargs have been passed via the command line is implicitly removing the command line parameter? This is a surprising side-effect and it sound wrong to do. What happens if the device is getting replugged? It seems to me that you want to make the hotplug add functionality behave like a force attach. You can achieve this by calling rte_eal_devargs_add/parse before calling into hotplug add. > Fixes: a3ee360f4440 ("eal: add hotplug add/remove device") > > Signed-off-by: Gaetan Rivet <gaetan.ri...@6wind.com> > --- > lib/librte_eal/common/eal_common_dev.c | 57 > ++++++++++++++++++++++++++++++++-- > 1 file changed, 54 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_dev.c > b/lib/librte_eal/common/eal_common_dev.c > index 32e12b5..f5566a6 100644 > --- a/lib/librte_eal/common/eal_common_dev.c > +++ b/lib/librte_eal/common/eal_common_dev.c > @@ -118,11 +118,32 @@ int rte_eal_dev_detach(struct rte_device *dev) > return ret; > } > > +static char * > +full_dev_name(const char *bus, const char *dev, const char *args) > +{ > + char *name; > + size_t len; > + > + len = strlen(bus) + 1 + > + strlen(dev) + 1 + > + strlen(args) + 1; > + name = calloc(1, len); > + if (name == NULL) { > + RTE_LOG(ERR, EAL, "Could not allocate full device name\n"); > + return NULL; > + } > + snprintf(name, len, "%s:%s,%s", bus, dev, > + args ? args : ""); > + return name; > +} > + > int rte_eal_hotplug_add(const char *busname, const char *devname, > const char *devargs) > { > struct rte_bus *bus; > struct rte_device *dev; > + struct rte_devargs *da; > + char *name; > int ret; > > bus = rte_bus_find_by_name(busname); > @@ -137,21 +158,49 @@ int rte_eal_hotplug_add(const char *busname, const char > *devname, > return -ENOTSUP; > } > > + name = full_dev_name(busname, devname, devargs); > + if (name == NULL) > + return -ENOMEM; > + > + da = calloc(1, sizeof(*da)); > + if (da == NULL) { > + ret = -ENOMEM; > + goto err_name; > + } > + > + ret = rte_eal_devargs_parse(name, da); > + if (ret) > + goto err_devarg; > + > + ret = rte_eal_devargs_insert(da); > + if (ret) > + goto err_devarg; > + > ret = bus->scan(); > if (ret) > - return ret; > + goto err_devarg; > > dev = bus->find_device(NULL, cmp_detached_dev_name, devname); > if (dev == NULL) { > RTE_LOG(ERR, EAL, "Cannot find unplugged device (%s)\n", > devname); > - return -EINVAL; > + ret = -ENODEV; > + goto err_devarg; > } > > ret = bus->plug(dev, devargs); > - if (ret) > + if (ret) { > RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", > dev->name); > + goto err_devarg; > + } > + free(name); > + return 0; > + > +err_devarg: > + rte_eal_devargs_remove(busname, devname); > +err_name: > + free(name); > return ret; > } > > @@ -179,6 +228,8 @@ int rte_eal_hotplug_remove(const char *busname, const > char *devname) > return -EINVAL; > } > > + rte_eal_devargs_remove(busname, devname); > + dev->devargs = NULL; > ret = bus->unplug(dev); > if (ret) > RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n", > -- > 2.1.4 >