[v1] eal: add manual probing option
Checks
Commit Message
Add a new EAL option enabling manual probing in the EAL.
This command line option will configure the EAL so that buses
will not trigger their probe step on their own.
Applications are then expected to hotplug devices as they see fit.
Devices declared on the command line by the user (using -w and --vdev),
will be probed using the hotplug API, in the order they are declared.
This has the effect of offering a way for users to control probe order
of their devices, for drivers requiring it.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
I haven't heard many opinions on the matter, please shout if you see an issue
with this approach.
@Slava: I have tested rather quickly that it does not break anything,
and that it works as intended for basic cases.
Can you test it further for your use-case and tell me if it works fine?
Beyond the obvious difference between both probe mode, something to keep in mind:
while using -w on invalid devices would not block (PCI) bus probing, it will stop manual
probing in its track. All devices need to exist and be valid device IDs.
doc/guides/rel_notes/release_19_11.rst | 9 +++++++
lib/librte_eal/common/eal_common_bus.c | 6 +++++
lib/librte_eal/common/eal_common_dev.c | 41 ++++++++++++++++++++++++++++++
lib/librte_eal/common/eal_common_options.c | 8 ++++++
lib/librte_eal/common/eal_internal_cfg.h | 1 +
lib/librte_eal/common/eal_options.h | 2 ++
lib/librte_eal/common/eal_private.h | 9 +++++++
lib/librte_eal/common/include/rte_eal.h | 17 +++++++++++++
lib/librte_eal/freebsd/eal/eal.c | 5 ++++
lib/librte_eal/linux/eal/eal.c | 5 ++++
lib/librte_eal/rte_eal_version.map | 7 +++++
11 files changed, 110 insertions(+)
Comments
Gaetan Rivet <gaetan.rivet@6wind.com> writes:
> Add a new EAL option enabling manual probing in the EAL.
> This command line option will configure the EAL so that buses
> will not trigger their probe step on their own.
>
> Applications are then expected to hotplug devices as they see fit.
>
> Devices declared on the command line by the user (using -w and --vdev),
> will be probed using the hotplug API, in the order they are declared.
>
> This has the effect of offering a way for users to control probe order
> of their devices, for drivers requiring it.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>
> I haven't heard many opinions on the matter, please shout if you see an issue
> with this approach.
>
> @Slava: I have tested rather quickly that it does not break anything,
> and that it works as intended for basic cases.
> Can you test it further for your use-case and tell me if it works fine?
>
> Beyond the obvious difference between both probe mode, something to keep in mind:
> while using -w on invalid devices would not block (PCI) bus probing, it will stop manual
> probing in its track. All devices need to exist and be valid device IDs.
>
> doc/guides/rel_notes/release_19_11.rst | 9 +++++++
> lib/librte_eal/common/eal_common_bus.c | 6 +++++
> lib/librte_eal/common/eal_common_dev.c | 41 ++++++++++++++++++++++++++++++
> lib/librte_eal/common/eal_common_options.c | 8 ++++++
> lib/librte_eal/common/eal_internal_cfg.h | 1 +
> lib/librte_eal/common/eal_options.h | 2 ++
> lib/librte_eal/common/eal_private.h | 9 +++++++
> lib/librte_eal/common/include/rte_eal.h | 17 +++++++++++++
> lib/librte_eal/freebsd/eal/eal.c | 5 ++++
> lib/librte_eal/linux/eal/eal.c | 5 ++++
> lib/librte_eal/rte_eal_version.map | 7 +++++
> 11 files changed, 110 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index 27cfbd9e3..3996e429d 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -56,6 +56,15 @@ New Features
> Also, make sure to start the actual text at the margin.
> =========================================================
>
> +* **EAL wil now propose to enforce manual probing of devices.**
> +
> + Previously, a user could not force an order when probing declared devices.
> + This could cause issues for drivers depending on another device being present.
> + A new option ``--manual-probe`` is now available to do just that.
> + This new option relies on the device bus supporting hotplug. It can
> + also be used to disable automatic probing from the ``PCI`` bus without
> + having to disable the whole bus.
> +
>
> Removed Items
> -------------
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..145a96812 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -6,6 +6,7 @@
> #include <string.h>
> #include <sys/queue.h>
>
> +#include <rte_eal.h>
> #include <rte_bus.h>
> #include <rte_debug.h>
> #include <rte_string_fns.h>
> @@ -63,6 +64,11 @@ rte_bus_probe(void)
> int ret;
> struct rte_bus *bus, *vbus = NULL;
>
> + if (rte_eal_manual_probe()) {
> + RTE_LOG(DEBUG, EAL, "Manual probing enabled.\n");
> + return rte_dev_probe_devargs_list();
> + }
> +
> TAILQ_FOREACH(bus, &rte_bus_list, next) {
> if (!strcmp(bus->name, "vdev")) {
> vbus = bus;
> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> index 9e4f09d83..3b7769a3c 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -109,6 +109,47 @@ build_devargs(const char *busname, const char *devname,
> }
>
> int
> +rte_dev_probe_devargs_list(void)
> +{
> + struct rte_device *dev;
> + struct rte_devargs *da;
> + int ret;
> +
> + RTE_EAL_DEVARGS_FOREACH(NULL, da) {
> + dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
> + if (dev == NULL) {
> + RTE_LOG(ERR, EAL, "Unable to find device %s on bus %s\n",
> + da->name, da->bus->name);
> + continue;
> + }
> +
> + if (rte_dev_is_probed(dev))
> + continue;
> +
> + if (dev->bus->plug == NULL) {
> + RTE_LOG(ERR, EAL, "Manual probing (hotplug) not supported by bus %s, "
> + "required by device %s\n",
> + dev->bus->name, dev->name);
> + continue;
> + }
> +
> + ret = dev->bus->plug(dev);
> + /* Ignore positive return values, they are possibly
> + * triggered by blacklisted devices on the PCI bus. Probing
> + * should then continue.
> + */
> + if (ret < 0 || !rte_dev_is_probed(dev)) {
> + RTE_LOG(ERR, EAL, "Driver cannot attach device %s\n",
> + dev->name);
> + /* Fail on first real probe error. */
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +int
> rte_eal_hotplug_add(const char *busname, const char *devname,
> const char *drvargs)
> {
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 05cae5f75..66c232b14 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -81,6 +81,7 @@ eal_long_options[] = {
> {OPT_LEGACY_MEM, 0, NULL, OPT_LEGACY_MEM_NUM },
> {OPT_SINGLE_FILE_SEGMENTS, 0, NULL, OPT_SINGLE_FILE_SEGMENTS_NUM},
> {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> + {OPT_MANUAL_PROBE, 0, NULL, OPT_MANUAL_PROBE_NUM },
> {0, 0, NULL, 0 }
> };
>
> @@ -1408,6 +1409,9 @@ eal_parse_common_option(int opt, const char *optarg,
> return -1;
> }
> break;
> + case OPT_MANUAL_PROBE_NUM:
> + conf->manual_probe = 1;
> + break;
>
> /* don't know what to do, leave this to caller */
> default:
> @@ -1634,6 +1638,10 @@ eal_common_usage(void)
> " --"OPT_VDEV" Add a virtual device.\n"
> " The argument format is <driver><id>[,key=val,...]\n"
> " (ex: --vdev=net_pcap0,iface=eth2).\n"
> + " --"OPT_MANUAL_PROBE" Enable manual probing.\n"
> + " Disable probe step for all buses.\n"
> + " Devices will need to be probed using the hotplug API.\n"
> + " PCI and vdev declarations will be treated in order as hotplug commands.\n"
> " --"OPT_IOVA_MODE" Set IOVA mode. 'pa' for IOVA_PA\n"
> " 'va' for IOVA_VA\n"
> " -d LIB.so|DIR Add a driver or driver directory\n"
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index a42f34923..0006f903f 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -44,6 +44,7 @@ struct internal_config {
> unsigned hugepage_unlink; /**< true to unlink backing files */
> volatile unsigned no_pci; /**< true to disable PCI */
> volatile unsigned no_hpet; /**< true to disable HPET */
> + volatile unsigned manual_probe; /**< true to enable manual device probing. */
> volatile unsigned vmware_tsc_map; /**< true to use VMware TSC mapping
> * instead of native TSC */
> volatile unsigned no_shconf; /**< true if there is no shared config */
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 9855429e5..588fa32a6 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -69,6 +69,8 @@ enum {
> OPT_IOVA_MODE_NUM,
> #define OPT_MATCH_ALLOCATIONS "match-allocations"
> OPT_MATCH_ALLOCATIONS_NUM,
> +#define OPT_MANUAL_PROBE "manual-probe"
> + OPT_MANUAL_PROBE_NUM,
> OPT_LONG_MAX_NUM
> };
>
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 798ede553..fd7ac8e37 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -381,4 +381,13 @@ rte_option_init(void);
> void
> rte_option_usage(void);
>
> +/**
> + * Go through the devargs list and probe everything in order.
> + *
> + * @return
> + * 0 on success, negative on error.
> + */
> +int
> +rte_dev_probe_devargs_list(void);
> +
> #endif /* _EAL_PRIVATE_H_ */
> diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
> index b7cf91214..bd9087bcc 100644
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -465,6 +465,23 @@ int rte_eal_has_hugepages(void);
> int rte_eal_has_pci(void);
>
> /**
> + * Whether EAL probe is manual.
> + * Enabled by the --manual-probe option.
> + *
> + * When manual probing is enabled, batched bus probe of
> + * their devices is disabled. All devices need to be probed
> + * using the proper rte_dev API.
> + *
> + * In this mode, devices declared on the command line will
> + * be probed using the bus hotplug API. It is used to enforce
> + * a specific probe order.
> + *
> + * @return
> + * Nonzero if manual device probing is enabled.
> + */
> +int rte_eal_manual_probe(void);
> +
> +/**
> * Whether the EAL was asked to create UIO device.
> *
> * @return
> diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> index d53f0fe69..2daa63b23 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -961,6 +961,11 @@ int rte_eal_has_pci(void)
> return !internal_config.no_pci;
> }
>
> +int rte_eal_manual_probe(void)
> +{
> + return internal_config.manual_probe;
> +}
> +
> int rte_eal_create_uio_dev(void)
> {
> return internal_config.create_uio_dev;
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 946222ccd..476fc2e4a 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -1376,6 +1376,11 @@ int rte_eal_create_uio_dev(void)
> return internal_config.create_uio_dev;
> }
>
> +int rte_eal_manual_probe(void)
> +{
> + return internal_config.manual_probe;
> +}
> +
> enum rte_intr_mode
> rte_eal_vfio_intr_mode(void)
> {
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 7cbf82d37..fe5395c24 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -312,6 +312,13 @@ DPDK_19.08 {
>
> } DPDK_19.05;
>
> +DPDK_19.05 {
> + global:
> +
> + rte_eal_manual_probe
Syntax error - needs a ';' at the end of this symbol.
> +
> +} DPDK_19.11;
> +
> EXPERIMENTAL {
> global:
On Mon, 30 Sep 2019 14:51:03 +0200
Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> Add a new EAL option enabling manual probing in the EAL.
> This command line option will configure the EAL so that buses
> will not trigger their probe step on their own.
>
> Applications are then expected to hotplug devices as they see fit.
>
> Devices declared on the command line by the user (using -w and --vdev),
> will be probed using the hotplug API, in the order they are declared.
>
> This has the effect of offering a way for users to control probe order
> of their devices, for drivers requiring it.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
I have no problems with the patch, but it would help if there was better
way to handle device naming policy in DPDK. Applications that depend on
particular port number are prone to get broken by changes in surrounding
OS or hardware environment. Just like Linux applications that are built
to depend on "eth0"; which is unfortunately all too common.
On Mon, Sep 30, 2019 at 01:51:35PM -0400, Aaron Conole wrote:
> Gaetan Rivet <gaetan.rivet@6wind.com> writes:
>
> > Add a new EAL option enabling manual probing in the EAL.
> > This command line option will configure the EAL so that buses
> > will not trigger their probe step on their own.
> >
> > Applications are then expected to hotplug devices as they see fit.
> >
> > Devices declared on the command line by the user (using -w and --vdev),
> > will be probed using the hotplug API, in the order they are declared.
> >
> > This has the effect of offering a way for users to control probe order
> > of their devices, for drivers requiring it.
> >
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >
> > I haven't heard many opinions on the matter, please shout if you see an issue
> > with this approach.
> >
> > @Slava: I have tested rather quickly that it does not break anything,
> > and that it works as intended for basic cases.
> > Can you test it further for your use-case and tell me if it works fine?
> >
> > Beyond the obvious difference between both probe mode, something to keep in mind:
> > while using -w on invalid devices would not block (PCI) bus probing, it will stop manual
> > probing in its track. All devices need to exist and be valid device IDs.
> >
> > doc/guides/rel_notes/release_19_11.rst | 9 +++++++
> > lib/librte_eal/common/eal_common_bus.c | 6 +++++
> > lib/librte_eal/common/eal_common_dev.c | 41 ++++++++++++++++++++++++++++++
> > lib/librte_eal/common/eal_common_options.c | 8 ++++++
> > lib/librte_eal/common/eal_internal_cfg.h | 1 +
> > lib/librte_eal/common/eal_options.h | 2 ++
> > lib/librte_eal/common/eal_private.h | 9 +++++++
> > lib/librte_eal/common/include/rte_eal.h | 17 +++++++++++++
> > lib/librte_eal/freebsd/eal/eal.c | 5 ++++
> > lib/librte_eal/linux/eal/eal.c | 5 ++++
> > lib/librte_eal/rte_eal_version.map | 7 +++++
> > 11 files changed, 110 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> > index 27cfbd9e3..3996e429d 100644
> > --- a/doc/guides/rel_notes/release_19_11.rst
> > +++ b/doc/guides/rel_notes/release_19_11.rst
> > @@ -56,6 +56,15 @@ New Features
> > Also, make sure to start the actual text at the margin.
> > =========================================================
> >
> > +* **EAL wil now propose to enforce manual probing of devices.**
> > +
> > + Previously, a user could not force an order when probing declared devices.
> > + This could cause issues for drivers depending on another device being present.
> > + A new option ``--manual-probe`` is now available to do just that.
> > + This new option relies on the device bus supporting hotplug. It can
> > + also be used to disable automatic probing from the ``PCI`` bus without
> > + having to disable the whole bus.
> > +
> >
> > Removed Items
> > -------------
> > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> > index baa5b532a..145a96812 100644
> > --- a/lib/librte_eal/common/eal_common_bus.c
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -6,6 +6,7 @@
> > #include <string.h>
> > #include <sys/queue.h>
> >
> > +#include <rte_eal.h>
> > #include <rte_bus.h>
> > #include <rte_debug.h>
> > #include <rte_string_fns.h>
> > @@ -63,6 +64,11 @@ rte_bus_probe(void)
> > int ret;
> > struct rte_bus *bus, *vbus = NULL;
> >
> > + if (rte_eal_manual_probe()) {
> > + RTE_LOG(DEBUG, EAL, "Manual probing enabled.\n");
> > + return rte_dev_probe_devargs_list();
> > + }
> > +
> > TAILQ_FOREACH(bus, &rte_bus_list, next) {
> > if (!strcmp(bus->name, "vdev")) {
> > vbus = bus;
> > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> > index 9e4f09d83..3b7769a3c 100644
> > --- a/lib/librte_eal/common/eal_common_dev.c
> > +++ b/lib/librte_eal/common/eal_common_dev.c
> > @@ -109,6 +109,47 @@ build_devargs(const char *busname, const char *devname,
> > }
> >
> > int
> > +rte_dev_probe_devargs_list(void)
> > +{
> > + struct rte_device *dev;
> > + struct rte_devargs *da;
> > + int ret;
> > +
> > + RTE_EAL_DEVARGS_FOREACH(NULL, da) {
> > + dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
> > + if (dev == NULL) {
> > + RTE_LOG(ERR, EAL, "Unable to find device %s on bus %s\n",
> > + da->name, da->bus->name);
> > + continue;
> > + }
> > +
> > + if (rte_dev_is_probed(dev))
> > + continue;
> > +
> > + if (dev->bus->plug == NULL) {
> > + RTE_LOG(ERR, EAL, "Manual probing (hotplug) not supported by bus %s, "
> > + "required by device %s\n",
> > + dev->bus->name, dev->name);
> > + continue;
> > + }
> > +
> > + ret = dev->bus->plug(dev);
> > + /* Ignore positive return values, they are possibly
> > + * triggered by blacklisted devices on the PCI bus. Probing
> > + * should then continue.
> > + */
> > + if (ret < 0 || !rte_dev_is_probed(dev)) {
> > + RTE_LOG(ERR, EAL, "Driver cannot attach device %s\n",
> > + dev->name);
> > + /* Fail on first real probe error. */
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int
> > rte_eal_hotplug_add(const char *busname, const char *devname,
> > const char *drvargs)
> > {
> > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> > index 05cae5f75..66c232b14 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -81,6 +81,7 @@ eal_long_options[] = {
> > {OPT_LEGACY_MEM, 0, NULL, OPT_LEGACY_MEM_NUM },
> > {OPT_SINGLE_FILE_SEGMENTS, 0, NULL, OPT_SINGLE_FILE_SEGMENTS_NUM},
> > {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> > + {OPT_MANUAL_PROBE, 0, NULL, OPT_MANUAL_PROBE_NUM },
> > {0, 0, NULL, 0 }
> > };
> >
> > @@ -1408,6 +1409,9 @@ eal_parse_common_option(int opt, const char *optarg,
> > return -1;
> > }
> > break;
> > + case OPT_MANUAL_PROBE_NUM:
> > + conf->manual_probe = 1;
> > + break;
> >
> > /* don't know what to do, leave this to caller */
> > default:
> > @@ -1634,6 +1638,10 @@ eal_common_usage(void)
> > " --"OPT_VDEV" Add a virtual device.\n"
> > " The argument format is <driver><id>[,key=val,...]\n"
> > " (ex: --vdev=net_pcap0,iface=eth2).\n"
> > + " --"OPT_MANUAL_PROBE" Enable manual probing.\n"
> > + " Disable probe step for all buses.\n"
> > + " Devices will need to be probed using the hotplug API.\n"
> > + " PCI and vdev declarations will be treated in order as hotplug commands.\n"
> > " --"OPT_IOVA_MODE" Set IOVA mode. 'pa' for IOVA_PA\n"
> > " 'va' for IOVA_VA\n"
> > " -d LIB.so|DIR Add a driver or driver directory\n"
> > diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> > index a42f34923..0006f903f 100644
> > --- a/lib/librte_eal/common/eal_internal_cfg.h
> > +++ b/lib/librte_eal/common/eal_internal_cfg.h
> > @@ -44,6 +44,7 @@ struct internal_config {
> > unsigned hugepage_unlink; /**< true to unlink backing files */
> > volatile unsigned no_pci; /**< true to disable PCI */
> > volatile unsigned no_hpet; /**< true to disable HPET */
> > + volatile unsigned manual_probe; /**< true to enable manual device probing. */
> > volatile unsigned vmware_tsc_map; /**< true to use VMware TSC mapping
> > * instead of native TSC */
> > volatile unsigned no_shconf; /**< true if there is no shared config */
> > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> > index 9855429e5..588fa32a6 100644
> > --- a/lib/librte_eal/common/eal_options.h
> > +++ b/lib/librte_eal/common/eal_options.h
> > @@ -69,6 +69,8 @@ enum {
> > OPT_IOVA_MODE_NUM,
> > #define OPT_MATCH_ALLOCATIONS "match-allocations"
> > OPT_MATCH_ALLOCATIONS_NUM,
> > +#define OPT_MANUAL_PROBE "manual-probe"
> > + OPT_MANUAL_PROBE_NUM,
> > OPT_LONG_MAX_NUM
> > };
> >
> > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> > index 798ede553..fd7ac8e37 100644
> > --- a/lib/librte_eal/common/eal_private.h
> > +++ b/lib/librte_eal/common/eal_private.h
> > @@ -381,4 +381,13 @@ rte_option_init(void);
> > void
> > rte_option_usage(void);
> >
> > +/**
> > + * Go through the devargs list and probe everything in order.
> > + *
> > + * @return
> > + * 0 on success, negative on error.
> > + */
> > +int
> > +rte_dev_probe_devargs_list(void);
> > +
> > #endif /* _EAL_PRIVATE_H_ */
> > diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
> > index b7cf91214..bd9087bcc 100644
> > --- a/lib/librte_eal/common/include/rte_eal.h
> > +++ b/lib/librte_eal/common/include/rte_eal.h
> > @@ -465,6 +465,23 @@ int rte_eal_has_hugepages(void);
> > int rte_eal_has_pci(void);
> >
> > /**
> > + * Whether EAL probe is manual.
> > + * Enabled by the --manual-probe option.
> > + *
> > + * When manual probing is enabled, batched bus probe of
> > + * their devices is disabled. All devices need to be probed
> > + * using the proper rte_dev API.
> > + *
> > + * In this mode, devices declared on the command line will
> > + * be probed using the bus hotplug API. It is used to enforce
> > + * a specific probe order.
> > + *
> > + * @return
> > + * Nonzero if manual device probing is enabled.
> > + */
> > +int rte_eal_manual_probe(void);
> > +
> > +/**
> > * Whether the EAL was asked to create UIO device.
> > *
> > * @return
> > diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> > index d53f0fe69..2daa63b23 100644
> > --- a/lib/librte_eal/freebsd/eal/eal.c
> > +++ b/lib/librte_eal/freebsd/eal/eal.c
> > @@ -961,6 +961,11 @@ int rte_eal_has_pci(void)
> > return !internal_config.no_pci;
> > }
> >
> > +int rte_eal_manual_probe(void)
> > +{
> > + return internal_config.manual_probe;
> > +}
> > +
> > int rte_eal_create_uio_dev(void)
> > {
> > return internal_config.create_uio_dev;
> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> > index 946222ccd..476fc2e4a 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -1376,6 +1376,11 @@ int rte_eal_create_uio_dev(void)
> > return internal_config.create_uio_dev;
> > }
> >
> > +int rte_eal_manual_probe(void)
> > +{
> > + return internal_config.manual_probe;
> > +}
> > +
> > enum rte_intr_mode
> > rte_eal_vfio_intr_mode(void)
> > {
> > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> > index 7cbf82d37..fe5395c24 100644
> > --- a/lib/librte_eal/rte_eal_version.map
> > +++ b/lib/librte_eal/rte_eal_version.map
> > @@ -312,6 +312,13 @@ DPDK_19.08 {
> >
> > } DPDK_19.05;
> >
> > +DPDK_19.05 {
> > + global:
> > +
> > + rte_eal_manual_probe
>
> Syntax error - needs a ';' at the end of this symbol.
>
Ah yes, sorry.
I'm thinking there is a script or a validation tool using this, I should try it.
> > +
> > +} DPDK_19.11;
> > +
> > EXPERIMENTAL {
> > global:
On Mon, Sep 30, 2019 at 11:53:33AM -0700, Stephen Hemminger wrote:
> On Mon, 30 Sep 2019 14:51:03 +0200
> Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
>
> > Add a new EAL option enabling manual probing in the EAL.
> > This command line option will configure the EAL so that buses
> > will not trigger their probe step on their own.
> >
> > Applications are then expected to hotplug devices as they see fit.
> >
> > Devices declared on the command line by the user (using -w and --vdev),
> > will be probed using the hotplug API, in the order they are declared.
> >
> > This has the effect of offering a way for users to control probe order
> > of their devices, for drivers requiring it.
> >
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>
> I have no problems with the patch, but it would help if there was better
> way to handle device naming policy in DPDK. Applications that depend on
> particular port number are prone to get broken by changes in surrounding
> OS or hardware environment. Just like Linux applications that are built
> to depend on "eth0"; which is unfortunately all too common.
Hello Stephen,
This patch is a way to avoid having the PCI bus defining the probe order
with the current hardware environment. It seems to be a step in the
right direction for the issue you identify.
There is a tight coupling between device names and driver matches for
the vdev bus, but that seems difficult to avoid.
Do you see other EAL APIs fostering an over reliance of downstream
systems on device names?
I pushed a few months back a way to iterate / match devices by their
properties. If you identify other pain points, this could certainly be
improved as well.
On Tue, Oct 1, 2019 at 2:40 PM Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
>
> On Mon, Sep 30, 2019 at 11:53:33AM -0700, Stephen Hemminger wrote:
> > On Mon, 30 Sep 2019 14:51:03 +0200
> > Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> >
> > > Add a new EAL option enabling manual probing in the EAL.
> > > This command line option will configure the EAL so that buses
> > > will not trigger their probe step on their own.
> > >
> > > Applications are then expected to hotplug devices as they see fit.
> > >
> > > Devices declared on the command line by the user (using -w and --vdev),
> > > will be probed using the hotplug API, in the order they are declared.
> > >
> > > This has the effect of offering a way for users to control probe order
> > > of their devices, for drivers requiring it.
> > >
> > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> >
> > I have no problems with the patch, but it would help if there was better
> > way to handle device naming policy in DPDK. Applications that depend on
> > particular port number are prone to get broken by changes in surrounding
> > OS or hardware environment. Just like Linux applications that are built
> > to depend on "eth0"; which is unfortunately all too common.
>
> Hello Stephen,
>
> This patch is a way to avoid having the PCI bus defining the probe order
> with the current hardware environment. It seems to be a step in the
> right direction for the issue you identify.
>
> There is a tight coupling between device names and driver matches for
> the vdev bus, but that seems difficult to avoid.
>
> Do you see other EAL APIs fostering an over reliance of downstream
> systems on device names?
>
> I pushed a few months back a way to iterate / match devices by their
> properties. If you identify other pain points, this could certainly be
> improved as well.
And this mode will be kicked in only when "--manual-probe" selected on
eal arguments.
So it won't change the behavior of the existing applications.
>
> --
> Gaėtan Rivet
> 6WIND
Gaëtan Rivet <gaetan.rivet@6wind.com> writes:
> On Mon, Sep 30, 2019 at 01:51:35PM -0400, Aaron Conole wrote:
>> Gaetan Rivet <gaetan.rivet@6wind.com> writes:
>>
>> > Add a new EAL option enabling manual probing in the EAL.
>> > This command line option will configure the EAL so that buses
>> > will not trigger their probe step on their own.
>> >
>> > Applications are then expected to hotplug devices as they see fit.
>> >
>> > Devices declared on the command line by the user (using -w and --vdev),
>> > will be probed using the hotplug API, in the order they are declared.
>> >
>> > This has the effect of offering a way for users to control probe order
>> > of their devices, for drivers requiring it.
>> >
>> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>> > ---
>> >
>> > I haven't heard many opinions on the matter, please shout if you see an issue
>> > with this approach.
>> >
>> > @Slava: I have tested rather quickly that it does not break anything,
>> > and that it works as intended for basic cases.
>> > Can you test it further for your use-case and tell me if it works fine?
>> >
>> > Beyond the obvious difference between both probe mode, something to keep in mind:
>> > while using -w on invalid devices would not block (PCI) bus probing, it will stop manual
>> > probing in its track. All devices need to exist and be valid device IDs.
>> >
>> > doc/guides/rel_notes/release_19_11.rst | 9 +++++++
>> > lib/librte_eal/common/eal_common_bus.c | 6 +++++
>> > lib/librte_eal/common/eal_common_dev.c | 41 ++++++++++++++++++++++++++++++
>> > lib/librte_eal/common/eal_common_options.c | 8 ++++++
>> > lib/librte_eal/common/eal_internal_cfg.h | 1 +
>> > lib/librte_eal/common/eal_options.h | 2 ++
>> > lib/librte_eal/common/eal_private.h | 9 +++++++
>> > lib/librte_eal/common/include/rte_eal.h | 17 +++++++++++++
>> > lib/librte_eal/freebsd/eal/eal.c | 5 ++++
>> > lib/librte_eal/linux/eal/eal.c | 5 ++++
>> > lib/librte_eal/rte_eal_version.map | 7 +++++
>> > 11 files changed, 110 insertions(+)
>> >
>> > diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
>> > index 27cfbd9e3..3996e429d 100644
>> > --- a/doc/guides/rel_notes/release_19_11.rst
>> > +++ b/doc/guides/rel_notes/release_19_11.rst
>> > @@ -56,6 +56,15 @@ New Features
>> > Also, make sure to start the actual text at the margin.
>> > =========================================================
>> >
>> > +* **EAL wil now propose to enforce manual probing of devices.**
>> > +
>> > + Previously, a user could not force an order when probing declared devices.
>> > + This could cause issues for drivers depending on another device being present.
>> > + A new option ``--manual-probe`` is now available to do just that.
>> > + This new option relies on the device bus supporting hotplug. It can
>> > + also be used to disable automatic probing from the ``PCI`` bus without
>> > + having to disable the whole bus.
>> > +
>> >
>> > Removed Items
>> > -------------
>> > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
>> > index baa5b532a..145a96812 100644
>> > --- a/lib/librte_eal/common/eal_common_bus.c
>> > +++ b/lib/librte_eal/common/eal_common_bus.c
>> > @@ -6,6 +6,7 @@
>> > #include <string.h>
>> > #include <sys/queue.h>
>> >
>> > +#include <rte_eal.h>
>> > #include <rte_bus.h>
>> > #include <rte_debug.h>
>> > #include <rte_string_fns.h>
>> > @@ -63,6 +64,11 @@ rte_bus_probe(void)
>> > int ret;
>> > struct rte_bus *bus, *vbus = NULL;
>> >
>> > + if (rte_eal_manual_probe()) {
>> > + RTE_LOG(DEBUG, EAL, "Manual probing enabled.\n");
>> > + return rte_dev_probe_devargs_list();
>> > + }
>> > +
>> > TAILQ_FOREACH(bus, &rte_bus_list, next) {
>> > if (!strcmp(bus->name, "vdev")) {
>> > vbus = bus;
>> > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
>> > index 9e4f09d83..3b7769a3c 100644
>> > --- a/lib/librte_eal/common/eal_common_dev.c
>> > +++ b/lib/librte_eal/common/eal_common_dev.c
>> > @@ -109,6 +109,47 @@ build_devargs(const char *busname, const char *devname,
>> > }
>> >
>> > int
>> > +rte_dev_probe_devargs_list(void)
>> > +{
>> > + struct rte_device *dev;
>> > + struct rte_devargs *da;
>> > + int ret;
>> > +
>> > + RTE_EAL_DEVARGS_FOREACH(NULL, da) {
>> > + dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
>> > + if (dev == NULL) {
>> > + RTE_LOG(ERR, EAL, "Unable to find device %s on bus %s\n",
>> > + da->name, da->bus->name);
>> > + continue;
>> > + }
>> > +
>> > + if (rte_dev_is_probed(dev))
>> > + continue;
>> > +
>> > + if (dev->bus->plug == NULL) {
>> > + RTE_LOG(ERR, EAL, "Manual probing (hotplug) not supported by bus %s, "
>> > + "required by device %s\n",
>> > + dev->bus->name, dev->name);
>> > + continue;
>> > + }
>> > +
>> > + ret = dev->bus->plug(dev);
>> > + /* Ignore positive return values, they are possibly
>> > + * triggered by blacklisted devices on the PCI bus. Probing
>> > + * should then continue.
>> > + */
>> > + if (ret < 0 || !rte_dev_is_probed(dev)) {
>> > + RTE_LOG(ERR, EAL, "Driver cannot attach device %s\n",
>> > + dev->name);
>> > + /* Fail on first real probe error. */
>> > + return ret;
>> > + }
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +int
>> > rte_eal_hotplug_add(const char *busname, const char *devname,
>> > const char *drvargs)
>> > {
>> > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
>> > index 05cae5f75..66c232b14 100644
>> > --- a/lib/librte_eal/common/eal_common_options.c
>> > +++ b/lib/librte_eal/common/eal_common_options.c
>> > @@ -81,6 +81,7 @@ eal_long_options[] = {
>> > {OPT_LEGACY_MEM, 0, NULL, OPT_LEGACY_MEM_NUM },
>> > {OPT_SINGLE_FILE_SEGMENTS, 0, NULL, OPT_SINGLE_FILE_SEGMENTS_NUM},
>> > {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
>> > + {OPT_MANUAL_PROBE, 0, NULL, OPT_MANUAL_PROBE_NUM },
>> > {0, 0, NULL, 0 }
>> > };
>> >
>> > @@ -1408,6 +1409,9 @@ eal_parse_common_option(int opt, const char *optarg,
>> > return -1;
>> > }
>> > break;
>> > + case OPT_MANUAL_PROBE_NUM:
>> > + conf->manual_probe = 1;
>> > + break;
>> >
>> > /* don't know what to do, leave this to caller */
>> > default:
>> > @@ -1634,6 +1638,10 @@ eal_common_usage(void)
>> > " --"OPT_VDEV" Add a virtual device.\n"
>> > " The argument format is <driver><id>[,key=val,...]\n"
>> > " (ex: --vdev=net_pcap0,iface=eth2).\n"
>> > + " --"OPT_MANUAL_PROBE" Enable manual probing.\n"
>> > + " Disable probe step for all buses.\n"
>> > + " Devices will need to be probed using the hotplug API.\n"
>> > + " PCI and vdev declarations will be treated in order as hotplug commands.\n"
>> > " --"OPT_IOVA_MODE" Set IOVA mode. 'pa' for IOVA_PA\n"
>> > " 'va' for IOVA_VA\n"
>> > " -d LIB.so|DIR Add a driver or driver directory\n"
>> > diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
>> > index a42f34923..0006f903f 100644
>> > --- a/lib/librte_eal/common/eal_internal_cfg.h
>> > +++ b/lib/librte_eal/common/eal_internal_cfg.h
>> > @@ -44,6 +44,7 @@ struct internal_config {
>> > unsigned hugepage_unlink; /**< true to unlink backing files */
>> > volatile unsigned no_pci; /**< true to disable PCI */
>> > volatile unsigned no_hpet; /**< true to disable HPET */
>> > + volatile unsigned manual_probe; /**< true to enable manual device probing. */
>> > volatile unsigned vmware_tsc_map; /**< true to use VMware TSC mapping
>> > * instead of native TSC */
>> > volatile unsigned no_shconf; /**< true if there is no shared config */
>> > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
>> > index 9855429e5..588fa32a6 100644
>> > --- a/lib/librte_eal/common/eal_options.h
>> > +++ b/lib/librte_eal/common/eal_options.h
>> > @@ -69,6 +69,8 @@ enum {
>> > OPT_IOVA_MODE_NUM,
>> > #define OPT_MATCH_ALLOCATIONS "match-allocations"
>> > OPT_MATCH_ALLOCATIONS_NUM,
>> > +#define OPT_MANUAL_PROBE "manual-probe"
>> > + OPT_MANUAL_PROBE_NUM,
>> > OPT_LONG_MAX_NUM
>> > };
>> >
>> > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
>> > index 798ede553..fd7ac8e37 100644
>> > --- a/lib/librte_eal/common/eal_private.h
>> > +++ b/lib/librte_eal/common/eal_private.h
>> > @@ -381,4 +381,13 @@ rte_option_init(void);
>> > void
>> > rte_option_usage(void);
>> >
>> > +/**
>> > + * Go through the devargs list and probe everything in order.
>> > + *
>> > + * @return
>> > + * 0 on success, negative on error.
>> > + */
>> > +int
>> > +rte_dev_probe_devargs_list(void);
>> > +
>> > #endif /* _EAL_PRIVATE_H_ */
>> > diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
>> > index b7cf91214..bd9087bcc 100644
>> > --- a/lib/librte_eal/common/include/rte_eal.h
>> > +++ b/lib/librte_eal/common/include/rte_eal.h
>> > @@ -465,6 +465,23 @@ int rte_eal_has_hugepages(void);
>> > int rte_eal_has_pci(void);
>> >
>> > /**
>> > + * Whether EAL probe is manual.
>> > + * Enabled by the --manual-probe option.
>> > + *
>> > + * When manual probing is enabled, batched bus probe of
>> > + * their devices is disabled. All devices need to be probed
>> > + * using the proper rte_dev API.
>> > + *
>> > + * In this mode, devices declared on the command line will
>> > + * be probed using the bus hotplug API. It is used to enforce
>> > + * a specific probe order.
>> > + *
>> > + * @return
>> > + * Nonzero if manual device probing is enabled.
>> > + */
>> > +int rte_eal_manual_probe(void);
>> > +
>> > +/**
>> > * Whether the EAL was asked to create UIO device.
>> > *
>> > * @return
>> > diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
>> > index d53f0fe69..2daa63b23 100644
>> > --- a/lib/librte_eal/freebsd/eal/eal.c
>> > +++ b/lib/librte_eal/freebsd/eal/eal.c
>> > @@ -961,6 +961,11 @@ int rte_eal_has_pci(void)
>> > return !internal_config.no_pci;
>> > }
>> >
>> > +int rte_eal_manual_probe(void)
>> > +{
>> > + return internal_config.manual_probe;
>> > +}
>> > +
>> > int rte_eal_create_uio_dev(void)
>> > {
>> > return internal_config.create_uio_dev;
>> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
>> > index 946222ccd..476fc2e4a 100644
>> > --- a/lib/librte_eal/linux/eal/eal.c
>> > +++ b/lib/librte_eal/linux/eal/eal.c
>> > @@ -1376,6 +1376,11 @@ int rte_eal_create_uio_dev(void)
>> > return internal_config.create_uio_dev;
>> > }
>> >
>> > +int rte_eal_manual_probe(void)
>> > +{
>> > + return internal_config.manual_probe;
>> > +}
>> > +
>> > enum rte_intr_mode
>> > rte_eal_vfio_intr_mode(void)
>> > {
>> > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
>> > index 7cbf82d37..fe5395c24 100644
>> > --- a/lib/librte_eal/rte_eal_version.map
>> > +++ b/lib/librte_eal/rte_eal_version.map
>> > @@ -312,6 +312,13 @@ DPDK_19.08 {
>> >
>> > } DPDK_19.05;
>> >
>> > +DPDK_19.05 {
>> > + global:
>> > +
>> > + rte_eal_manual_probe
>>
>> Syntax error - needs a ';' at the end of this symbol.
>>
>
> Ah yes, sorry.
> I'm thinking there is a script or a validation tool using this, I should try it.
One method I use is to push to my personal github account, linked to
Travis-CI - then I get emails when it fails ;)
>> > +
>> > +} DPDK_19.11;
>> > +
>> > EXPERIMENTAL {
>> > global:
On Tue, Oct 01, 2019 at 03:19:42PM +0530, Jerin Jacob wrote:
> On Tue, Oct 1, 2019 at 2:40 PM Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
> >
> > On Mon, Sep 30, 2019 at 11:53:33AM -0700, Stephen Hemminger wrote:
> > > On Mon, 30 Sep 2019 14:51:03 +0200
> > > Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > >
> > > > Add a new EAL option enabling manual probing in the EAL.
> > > > This command line option will configure the EAL so that buses
> > > > will not trigger their probe step on their own.
> > > >
> > > > Applications are then expected to hotplug devices as they see fit.
> > > >
> > > > Devices declared on the command line by the user (using -w and --vdev),
> > > > will be probed using the hotplug API, in the order they are declared.
> > > >
> > > > This has the effect of offering a way for users to control probe order
> > > > of their devices, for drivers requiring it.
> > > >
> > > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > >
> > > I have no problems with the patch, but it would help if there was better
> > > way to handle device naming policy in DPDK. Applications that depend on
> > > particular port number are prone to get broken by changes in surrounding
> > > OS or hardware environment. Just like Linux applications that are built
> > > to depend on "eth0"; which is unfortunately all too common.
> >
> > Hello Stephen,
> >
> > This patch is a way to avoid having the PCI bus defining the probe order
> > with the current hardware environment. It seems to be a step in the
> > right direction for the issue you identify.
> >
> > There is a tight coupling between device names and driver matches for
> > the vdev bus, but that seems difficult to avoid.
> >
> > Do you see other EAL APIs fostering an over reliance of downstream
> > systems on device names?
> >
> > I pushed a few months back a way to iterate / match devices by their
> > properties. If you identify other pain points, this could certainly be
> > improved as well.
>
>
> And this mode will be kicked in only when "--manual-probe" selected on
> eal arguments.
> So it won't change the behavior of the existing applications.
If I read you correctly, if hardware independence is the proper way to function,
we should switch entirely to it.
I agree, but that means rewriting entirely the probe step of rte_bus.
This patch is a incremental step, I preferred to keep risks low.
It is not clear from your remark whether you are considering this
limitation a good or a bad thing however :)
Can you be a bit more explicit?
On Tue, Oct 1, 2019 at 7:39 PM Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
>
> On Tue, Oct 01, 2019 at 03:19:42PM +0530, Jerin Jacob wrote:
> > On Tue, Oct 1, 2019 at 2:40 PM Gaėtan Rivet <gaetan.rivet@6wind.com> wrote:
> > >
> > > On Mon, Sep 30, 2019 at 11:53:33AM -0700, Stephen Hemminger wrote:
> > > > On Mon, 30 Sep 2019 14:51:03 +0200
> > > > Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > > >
> > > > > Add a new EAL option enabling manual probing in the EAL.
> > > > > This command line option will configure the EAL so that buses
> > > > > will not trigger their probe step on their own.
> > > > >
> > > > > Applications are then expected to hotplug devices as they see fit.
> > > > >
> > > > > Devices declared on the command line by the user (using -w and --vdev),
> > > > > will be probed using the hotplug API, in the order they are declared.
> > > > >
> > > > > This has the effect of offering a way for users to control probe order
> > > > > of their devices, for drivers requiring it.
> > > > >
> > > > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > >
> > > > I have no problems with the patch, but it would help if there was better
> > > > way to handle device naming policy in DPDK. Applications that depend on
> > > > particular port number are prone to get broken by changes in surrounding
> > > > OS or hardware environment. Just like Linux applications that are built
> > > > to depend on "eth0"; which is unfortunately all too common.
> > >
> > > Hello Stephen,
> > >
> > > This patch is a way to avoid having the PCI bus defining the probe order
> > > with the current hardware environment. It seems to be a step in the
> > > right direction for the issue you identify.
> > >
> > > There is a tight coupling between device names and driver matches for
> > > the vdev bus, but that seems difficult to avoid.
> > >
> > > Do you see other EAL APIs fostering an over reliance of downstream
> > > systems on device names?
> > >
> > > I pushed a few months back a way to iterate / match devices by their
> > > properties. If you identify other pain points, this could certainly be
> > > improved as well.
> >
> >
> > And this mode will be kicked in only when "--manual-probe" selected on
> > eal arguments.
> > So it won't change the behavior of the existing applications.
>
> If I read you correctly, if hardware independence is the proper way to function,
> we should switch entirely to it.
>
> I agree, but that means rewriting entirely the probe step of rte_bus.
> This patch is a incremental step, I preferred to keep risks low.
+1
>
> It is not clear from your remark whether you are considering this
> limitation a good or a bad thing however :)
I am considering it as a good step.
> Can you be a bit more explicit?
>
> --
> Gaėtan Rivet
> 6WIND
@@ -56,6 +56,15 @@ New Features
Also, make sure to start the actual text at the margin.
=========================================================
+* **EAL wil now propose to enforce manual probing of devices.**
+
+ Previously, a user could not force an order when probing declared devices.
+ This could cause issues for drivers depending on another device being present.
+ A new option ``--manual-probe`` is now available to do just that.
+ This new option relies on the device bus supporting hotplug. It can
+ also be used to disable automatic probing from the ``PCI`` bus without
+ having to disable the whole bus.
+
Removed Items
-------------
@@ -6,6 +6,7 @@
#include <string.h>
#include <sys/queue.h>
+#include <rte_eal.h>
#include <rte_bus.h>
#include <rte_debug.h>
#include <rte_string_fns.h>
@@ -63,6 +64,11 @@ rte_bus_probe(void)
int ret;
struct rte_bus *bus, *vbus = NULL;
+ if (rte_eal_manual_probe()) {
+ RTE_LOG(DEBUG, EAL, "Manual probing enabled.\n");
+ return rte_dev_probe_devargs_list();
+ }
+
TAILQ_FOREACH(bus, &rte_bus_list, next) {
if (!strcmp(bus->name, "vdev")) {
vbus = bus;
@@ -109,6 +109,47 @@ build_devargs(const char *busname, const char *devname,
}
int
+rte_dev_probe_devargs_list(void)
+{
+ struct rte_device *dev;
+ struct rte_devargs *da;
+ int ret;
+
+ RTE_EAL_DEVARGS_FOREACH(NULL, da) {
+ dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
+ if (dev == NULL) {
+ RTE_LOG(ERR, EAL, "Unable to find device %s on bus %s\n",
+ da->name, da->bus->name);
+ continue;
+ }
+
+ if (rte_dev_is_probed(dev))
+ continue;
+
+ if (dev->bus->plug == NULL) {
+ RTE_LOG(ERR, EAL, "Manual probing (hotplug) not supported by bus %s, "
+ "required by device %s\n",
+ dev->bus->name, dev->name);
+ continue;
+ }
+
+ ret = dev->bus->plug(dev);
+ /* Ignore positive return values, they are possibly
+ * triggered by blacklisted devices on the PCI bus. Probing
+ * should then continue.
+ */
+ if (ret < 0 || !rte_dev_is_probed(dev)) {
+ RTE_LOG(ERR, EAL, "Driver cannot attach device %s\n",
+ dev->name);
+ /* Fail on first real probe error. */
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int
rte_eal_hotplug_add(const char *busname, const char *devname,
const char *drvargs)
{
@@ -81,6 +81,7 @@ eal_long_options[] = {
{OPT_LEGACY_MEM, 0, NULL, OPT_LEGACY_MEM_NUM },
{OPT_SINGLE_FILE_SEGMENTS, 0, NULL, OPT_SINGLE_FILE_SEGMENTS_NUM},
{OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
+ {OPT_MANUAL_PROBE, 0, NULL, OPT_MANUAL_PROBE_NUM },
{0, 0, NULL, 0 }
};
@@ -1408,6 +1409,9 @@ eal_parse_common_option(int opt, const char *optarg,
return -1;
}
break;
+ case OPT_MANUAL_PROBE_NUM:
+ conf->manual_probe = 1;
+ break;
/* don't know what to do, leave this to caller */
default:
@@ -1634,6 +1638,10 @@ eal_common_usage(void)
" --"OPT_VDEV" Add a virtual device.\n"
" The argument format is <driver><id>[,key=val,...]\n"
" (ex: --vdev=net_pcap0,iface=eth2).\n"
+ " --"OPT_MANUAL_PROBE" Enable manual probing.\n"
+ " Disable probe step for all buses.\n"
+ " Devices will need to be probed using the hotplug API.\n"
+ " PCI and vdev declarations will be treated in order as hotplug commands.\n"
" --"OPT_IOVA_MODE" Set IOVA mode. 'pa' for IOVA_PA\n"
" 'va' for IOVA_VA\n"
" -d LIB.so|DIR Add a driver or driver directory\n"
@@ -44,6 +44,7 @@ struct internal_config {
unsigned hugepage_unlink; /**< true to unlink backing files */
volatile unsigned no_pci; /**< true to disable PCI */
volatile unsigned no_hpet; /**< true to disable HPET */
+ volatile unsigned manual_probe; /**< true to enable manual device probing. */
volatile unsigned vmware_tsc_map; /**< true to use VMware TSC mapping
* instead of native TSC */
volatile unsigned no_shconf; /**< true if there is no shared config */
@@ -69,6 +69,8 @@ enum {
OPT_IOVA_MODE_NUM,
#define OPT_MATCH_ALLOCATIONS "match-allocations"
OPT_MATCH_ALLOCATIONS_NUM,
+#define OPT_MANUAL_PROBE "manual-probe"
+ OPT_MANUAL_PROBE_NUM,
OPT_LONG_MAX_NUM
};
@@ -381,4 +381,13 @@ rte_option_init(void);
void
rte_option_usage(void);
+/**
+ * Go through the devargs list and probe everything in order.
+ *
+ * @return
+ * 0 on success, negative on error.
+ */
+int
+rte_dev_probe_devargs_list(void);
+
#endif /* _EAL_PRIVATE_H_ */
@@ -465,6 +465,23 @@ int rte_eal_has_hugepages(void);
int rte_eal_has_pci(void);
/**
+ * Whether EAL probe is manual.
+ * Enabled by the --manual-probe option.
+ *
+ * When manual probing is enabled, batched bus probe of
+ * their devices is disabled. All devices need to be probed
+ * using the proper rte_dev API.
+ *
+ * In this mode, devices declared on the command line will
+ * be probed using the bus hotplug API. It is used to enforce
+ * a specific probe order.
+ *
+ * @return
+ * Nonzero if manual device probing is enabled.
+ */
+int rte_eal_manual_probe(void);
+
+/**
* Whether the EAL was asked to create UIO device.
*
* @return
@@ -961,6 +961,11 @@ int rte_eal_has_pci(void)
return !internal_config.no_pci;
}
+int rte_eal_manual_probe(void)
+{
+ return internal_config.manual_probe;
+}
+
int rte_eal_create_uio_dev(void)
{
return internal_config.create_uio_dev;
@@ -1376,6 +1376,11 @@ int rte_eal_create_uio_dev(void)
return internal_config.create_uio_dev;
}
+int rte_eal_manual_probe(void)
+{
+ return internal_config.manual_probe;
+}
+
enum rte_intr_mode
rte_eal_vfio_intr_mode(void)
{
@@ -312,6 +312,13 @@ DPDK_19.08 {
} DPDK_19.05;
+DPDK_19.05 {
+ global:
+
+ rte_eal_manual_probe
+
+} DPDK_19.11;
+
EXPERIMENTAL {
global: