[v3] 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.
v2: fixed a few typos, map file (and used Travis to validate).
Slava, are you able to test this patch?
v3: properly fixed the map file (herited 19.08 instead of 19.05).
Added a function to set the probe manual from the application,
without having the user do it from the command line.
Stopped spamming Slava about it, Vamsi was actually the one interested in it!
Standing issue worth chiming in:
Currently manual-probe will cut off probing from all buses.
It could be interesting to be able to only cut buses supporting hotplug,
given that they are the one able to probe devices afterward.
No real use-case for this right now, so leaving as-is. Might be worth
considering in the future.
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 | 34 +++++++++++++++++++++++++
lib/librte_eal/freebsd/eal/eal.c | 10 ++++++++
lib/librte_eal/linux/eal/eal.c | 10 ++++++++
lib/librte_eal/rte_eal_version.map | 8 ++++++
11 files changed, 138 insertions(+)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Gaetan Rivet
> Sent: Friday, October 4, 2019 6:25 PM
> To: dev@dpdk.org; Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Cc: Gaetan Rivet <gaetan.rivet@6wind.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Anatoly Burakov <anatoly.burakov@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; David Marchand <david.marchand@redhat.com>
> Subject: [dpdk-dev] [PATCH v3] eal: add manual probing option
>
> 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.
>
> v2: fixed a few typos, map file (and used Travis to validate).
>
> Slava, are you able to test this patch?
>
> v3: properly fixed the map file (herited 19.08 instead of 19.05).
>
> Added a function to set the probe manual from the application,
> without having the user do it from the command line.
>
> Stopped spamming Slava about it, Vamsi was actually the one interested in
> it!
@Gaetan: Thanks a lot for providing this patch. I have tested it in our use cases
and it works as expected, also verified multiple combinations by passing
whitelisted pdevs, vdevs and mix of both. Probing sequence happens as required.
Acked-by : Vamsi Attunuru < vattunuru@marvell.com >
Tested-by: Vamsi Attunuru < vattunuru@marvell.com >
>
> Standing issue worth chiming in:
>
> Currently manual-probe will cut off probing from all buses.
> It could be interesting to be able to only cut buses supporting hotplug,
> given that they are the one able to probe devices afterward.
>
> No real use-case for this right now, so leaving as-is. Might be worth
> considering in the future.
>
> 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 | 34
> +++++++++++++++++++++++++
> lib/librte_eal/freebsd/eal/eal.c | 10 ++++++++
> lib/librte_eal/linux/eal/eal.c | 10 ++++++++
> lib/librte_eal/rte_eal_version.map | 8 ++++++
> 11 files changed, 138 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_19_11.rst
> b/doc/guides/rel_notes/release_19_11.rst
> index 27cfbd9e3..700f9a726 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 will now allow manual probing 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..f4ce1c56c 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_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..da86fdb6b 100644
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -465,6 +465,40 @@ int rte_eal_has_hugepages(void); int
> rte_eal_has_pci(void);
>
> /**
> + * Whether EAL probe is manual.
> + * Enabled by the --manual-probe option or by
> + * using rte_eal_manual_probe_set().
> + *
> + * 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.
> + *
> + * @see rte_eal_manual_probe_set
> + */
> +int rte_eal_manual_probe(void);
> +
> +/**
> + * Configure EAL probe mode -- manual or automatic.
> + *
> + * Enable or disable manual probe mode in EAL.
> + * This function can be called at any time, but must be used
> + * before calling rte_eal_init() to have any effect.
> + *
> + * @param enabled
> + * zero to disable manual probe, non-zero to enable it.
> + *
> + * @see rte_eal_manual_probe
> + */
> +void rte_eal_manual_probe_set(int enabled);
> +
> +/**
> * 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..cc6d846bd 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -972,6 +972,16 @@ rte_eal_vfio_intr_mode(void)
> return RTE_INTR_MODE_NONE;
> }
>
> +int rte_eal_manual_probe(void)
> +{
> + return internal_config.manual_probe;
> +}
> +
> +void rte_eal_manual_probe_set(int enabled) {
> + internal_config.manual_probe = !!enabled; }
> +
> int rte_vfio_setup_device(__rte_unused const char *sysfs_base,
> __rte_unused const char *dev_addr,
> __rte_unused int *vfio_dev_fd,
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c index
> 946222ccd..da00eb14d 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -1382,6 +1382,16 @@ rte_eal_vfio_intr_mode(void)
> return internal_config.vfio_intr_mode; }
>
> +int rte_eal_manual_probe(void)
> +{
> + return internal_config.manual_probe;
> +}
> +
> +void rte_eal_manual_probe_set(int enabled) {
> + internal_config.manual_probe = !!enabled; }
> +
> int
> rte_eal_check_module(const char *module_name) { diff --git
> a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 7cbf82d37..ccc4ffb21 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -312,6 +312,14 @@ DPDK_19.08 {
>
> } DPDK_19.05;
>
> +DPDK_19.11 {
> + global:
> +
> + rte_eal_manual_probe;
> + rte_eal_manual_probe_set;
> +
> +} DPDK_19.08;
> +
> EXPERIMENTAL {
> global:
>
> --
> 2.11.0
Hi,
On Mon, Oct 07, 2019 at 01:27:48AM +0000, Vamsi Krishna Attunuru wrote:
>
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Gaetan Rivet
> > Sent: Friday, October 4, 2019 6:25 PM
> > To: dev@dpdk.org; Vamsi Krishna Attunuru <vattunuru@marvell.com>
> > Cc: Gaetan Rivet <gaetan.rivet@6wind.com>; Ferruh Yigit
> > <ferruh.yigit@intel.com>; Anatoly Burakov <anatoly.burakov@intel.com>;
> > Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; David Marchand <david.marchand@redhat.com>
> > Subject: [dpdk-dev] [PATCH v3] eal: add manual probing option
> >
> > 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.
> >
> > v2: fixed a few typos, map file (and used Travis to validate).
> >
> > Slava, are you able to test this patch?
> >
> > v3: properly fixed the map file (herited 19.08 instead of 19.05).
> >
> > Added a function to set the probe manual from the application,
> > without having the user do it from the command line.
> >
> > Stopped spamming Slava about it, Vamsi was actually the one interested in
> > it!
>
> @Gaetan: Thanks a lot for providing this patch. I have tested it in our use cases
> and it works as expected, also verified multiple combinations by passing
> whitelisted pdevs, vdevs and mix of both. Probing sequence happens as required.
>
> Acked-by : Vamsi Attunuru < vattunuru@marvell.com >
> Tested-by: Vamsi Attunuru < vattunuru@marvell.com >
>
> >
> > Standing issue worth chiming in:
> >
> > Currently manual-probe will cut off probing from all buses.
> > It could be interesting to be able to only cut buses supporting hotplug,
> > given that they are the one able to probe devices afterward.
> >
> > No real use-case for this right now, so leaving as-is. Might be worth
> > considering in the future.
> >
Any further comment for this patch? Can someone review it?
On Fri, Oct 4, 2019 at 6:25 PM 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 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.
>
> v2: fixed a few typos, map file (and used Travis to validate).
>
> Slava, are you able to test this patch?
>
> v3: properly fixed the map file (herited 19.08 instead of 19.05).
>
> Added a function to set the probe manual from the application,
> without having the user do it from the command line.
>
> Stopped spamming Slava about it, Vamsi was actually the one interested in it!
>
> Standing issue worth chiming in:
>
> Currently manual-probe will cut off probing from all buses.
> It could be interesting to be able to only cut buses supporting hotplug,
> given that they are the one able to probe devices afterward.
>
> No real use-case for this right now, so leaving as-is. Might be worth
> considering in the future.
>
> 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 | 34 +++++++++++++++++++++++++
> lib/librte_eal/freebsd/eal/eal.c | 10 ++++++++
> lib/librte_eal/linux/eal/eal.c | 10 ++++++++
> lib/librte_eal/rte_eal_version.map | 8 ++++++
> 11 files changed, 138 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index 27cfbd9e3..700f9a726 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 will now allow manual probing 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()) {
See below,
> int rte_vfio_setup_device(__rte_unused const char *sysfs_base,
> __rte_unused const char *dev_addr,
> __rte_unused int *vfio_dev_fd,
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 946222ccd..da00eb14d 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -1382,6 +1382,16 @@ rte_eal_vfio_intr_mode(void)
> return internal_config.vfio_intr_mode;
> }
>
> +int rte_eal_manual_probe(void)
> +{
> + return internal_config.manual_probe;
> +}
> +
> +void rte_eal_manual_probe_set(int enabled)
> +{
> + internal_config.manual_probe = !!enabled;
> +}
I don't think FreeBSD and Linux specific implementation is NOT required
as internal_config accessible in eal/common. example diff.
[master][dpdk.org] $ git diff
diff --git a/lib/librte_eal/common/eal_common_bus.c
b/lib/librte_eal/common/eal_common_bus.c
index 145a96812..db4257cf1 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -13,6 +13,7 @@
#include <rte_errno.h>
#include "eal_private.h"
+#include "eal_internal_cfg.h"
static struct rte_bus_list rte_bus_list =
TAILQ_HEAD_INITIALIZER(rte_bus_list);
@@ -64,7 +65,7 @@ rte_bus_probe(void)
int ret;
struct rte_bus *bus, *vbus = NULL;
- if (rte_eal_manual_probe()) {
+ if (internal_config.manual_probe) {
RTE_LOG(DEBUG, EAL, "Manual probing enabled.\n");
return rte_dev_probe_devargs_list();
}
> int
> rte_eal_check_module(const char *module_name)
> {
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 7cbf82d37..ccc4ffb21 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -312,6 +312,14 @@ DPDK_19.08 {
>
> } DPDK_19.05;
>
> +DPDK_19.11 {
> + global:
> +
> + rte_eal_manual_probe;
> + rte_eal_manual_probe_set;
Do we need public API for this? it is only used by eal lib.
checkpatch complains the following too.
ERROR: symbol rte_eal_manual_probe_set is added in the DPDK_19.11
section, but is expected to be added in the EXPERIMENTAL section of
the version map
On Fri, Oct 25, 2019 at 05:29:12PM +0530, Jerin Jacob wrote:
> On Fri, Oct 4, 2019 at 6:25 PM 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 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.
> >
> > v2: fixed a few typos, map file (and used Travis to validate).
> >
> > Slava, are you able to test this patch?
> >
> > v3: properly fixed the map file (herited 19.08 instead of 19.05).
> >
> > Added a function to set the probe manual from the application,
> > without having the user do it from the command line.
> >
> > Stopped spamming Slava about it, Vamsi was actually the one interested in it!
> >
> > Standing issue worth chiming in:
> >
> > Currently manual-probe will cut off probing from all buses.
> > It could be interesting to be able to only cut buses supporting hotplug,
> > given that they are the one able to probe devices afterward.
> >
> > No real use-case for this right now, so leaving as-is. Might be worth
> > considering in the future.
> >
> > 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 | 34 +++++++++++++++++++++++++
> > lib/librte_eal/freebsd/eal/eal.c | 10 ++++++++
> > lib/librte_eal/linux/eal/eal.c | 10 ++++++++
> > lib/librte_eal/rte_eal_version.map | 8 ++++++
> > 11 files changed, 138 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> > index 27cfbd9e3..700f9a726 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 will now allow manual probing 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()) {
>
> See below,
>
>
>
> > int rte_vfio_setup_device(__rte_unused const char *sysfs_base,
> > __rte_unused const char *dev_addr,
> > __rte_unused int *vfio_dev_fd,
> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> > index 946222ccd..da00eb14d 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -1382,6 +1382,16 @@ rte_eal_vfio_intr_mode(void)
> > return internal_config.vfio_intr_mode;
> > }
> >
> > +int rte_eal_manual_probe(void)
> > +{
> > + return internal_config.manual_probe;
> > +}
> > +
> > +void rte_eal_manual_probe_set(int enabled)
> > +{
> > + internal_config.manual_probe = !!enabled;
> > +}
>
> I don't think FreeBSD and Linux specific implementation is NOT required
> as internal_config accessible in eal/common. example diff.
>
> [master][dpdk.org] $ git diff
> diff --git a/lib/librte_eal/common/eal_common_bus.c
> b/lib/librte_eal/common/eal_common_bus.c
> index 145a96812..db4257cf1 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -13,6 +13,7 @@
> #include <rte_errno.h>
>
> #include "eal_private.h"
> +#include "eal_internal_cfg.h"
>
> static struct rte_bus_list rte_bus_list =
> TAILQ_HEAD_INITIALIZER(rte_bus_list);
> @@ -64,7 +65,7 @@ rte_bus_probe(void)
> int ret;
> struct rte_bus *bus, *vbus = NULL;
>
> - if (rte_eal_manual_probe()) {
> + if (internal_config.manual_probe) {
> RTE_LOG(DEBUG, EAL, "Manual probing enabled.\n");
> return rte_dev_probe_devargs_list();
> }
>
> > int
> > rte_eal_check_module(const char *module_name)
> > {
> > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> > index 7cbf82d37..ccc4ffb21 100644
> > --- a/lib/librte_eal/rte_eal_version.map
> > +++ b/lib/librte_eal/rte_eal_version.map
> > @@ -312,6 +312,14 @@ DPDK_19.08 {
> >
> > } DPDK_19.05;
> >
> > +DPDK_19.11 {
> > + global:
> > +
> > + rte_eal_manual_probe;
> > + rte_eal_manual_probe_set;
>
> Do we need public API for this? it is only used by eal lib.
>
I think some PMDs and some applications would need to issue warnings to
the user if manual probing is not enabled (i.e. a port representor
parameter was used in a PMD, but probing is automatic --> on some
platform it would work and others it would fail with the same config).
Some applications would also prefer forcing it enabled without having to
use the option on the command line.
This motivates the public API. Given that there is a public API, I think
it's better to use it even from within the EAL.
However I agree that it does not need two separate implementations. I
could move it to some eal_common_* part probably, maybe bus or dev?
> checkpatch complains the following too.
> ERROR: symbol rte_eal_manual_probe_set is added in the DPDK_19.11
> section, but is expected to be added in the EXPERIMENTAL section of
> the version map
The API is simple enough IMO to make it immediately stable: it will
allow applications forbidding the use of experimental API to make use of
it.
This is only for the sake of simplicity for some users. I'm not
opposed to making it experimental first if it's expected for all new
APIs.
> > > rte_eal_check_module(const char *module_name)
> > > {
> > > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> > > index 7cbf82d37..ccc4ffb21 100644
> > > --- a/lib/librte_eal/rte_eal_version.map
> > > +++ b/lib/librte_eal/rte_eal_version.map
> > > @@ -312,6 +312,14 @@ DPDK_19.08 {
> > >
> > > } DPDK_19.05;
> > >
> > > +DPDK_19.11 {
> > > + global:
> > > +
> > > + rte_eal_manual_probe;
> > > + rte_eal_manual_probe_set;
> >
> > Do we need public API for this? it is only used by eal lib.
> >
>
> I think some PMDs and some applications would need to issue warnings to
> the user if manual probing is not enabled (i.e. a port representor
> parameter was used in a PMD, but probing is automatic --> on some
> platform it would work and others it would fail with the same config).
>
> Some applications would also prefer forcing it enabled without having to
> use the option on the command line.
>
> This motivates the public API. Given that there is a public API, I think
it's better to use it even from within the EAL.
No issue from my side to expose through public API as well.
Please rte_eal_manual_probe_set() when setting in flags in eal as well.
Currently rte_eal_manual_probe_set() not used anywhere.
>
> However I agree that it does not need two separate implementations. I
> could move it to some eal_common_* part probably, maybe bus or dev?
eal should be fine. No strong opinion on the place.
>
> > checkpatch complains the following too.
> > ERROR: symbol rte_eal_manual_probe_set is added in the DPDK_19.11
> > section, but is expected to be added in the EXPERIMENTAL section of
> > the version map
>
> The API is simple enough IMO to make it immediately stable: it will
> allow applications forbidding the use of experimental API to make use of
> it.
>
> This is only for the sake of simplicity for some users. I'm not
> opposed to making it experimental first if it's expected for all new
> APIs.
All new APIs are treated as experimental. I don't think, we need to
make an exception here.
Other these nits, This patch looks good.
>
> --
> Gaėtan Rivet
> 6WIND
@@ -56,6 +56,15 @@ New Features
Also, make sure to start the actual text at the margin.
=========================================================
+* **EAL will now allow manual probing 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_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,40 @@ int rte_eal_has_hugepages(void);
int rte_eal_has_pci(void);
/**
+ * Whether EAL probe is manual.
+ * Enabled by the --manual-probe option or by
+ * using rte_eal_manual_probe_set().
+ *
+ * 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.
+ *
+ * @see rte_eal_manual_probe_set
+ */
+int rte_eal_manual_probe(void);
+
+/**
+ * Configure EAL probe mode -- manual or automatic.
+ *
+ * Enable or disable manual probe mode in EAL.
+ * This function can be called at any time, but must be used
+ * before calling rte_eal_init() to have any effect.
+ *
+ * @param enabled
+ * zero to disable manual probe, non-zero to enable it.
+ *
+ * @see rte_eal_manual_probe
+ */
+void rte_eal_manual_probe_set(int enabled);
+
+/**
* Whether the EAL was asked to create UIO device.
*
* @return
@@ -972,6 +972,16 @@ rte_eal_vfio_intr_mode(void)
return RTE_INTR_MODE_NONE;
}
+int rte_eal_manual_probe(void)
+{
+ return internal_config.manual_probe;
+}
+
+void rte_eal_manual_probe_set(int enabled)
+{
+ internal_config.manual_probe = !!enabled;
+}
+
int rte_vfio_setup_device(__rte_unused const char *sysfs_base,
__rte_unused const char *dev_addr,
__rte_unused int *vfio_dev_fd,
@@ -1382,6 +1382,16 @@ rte_eal_vfio_intr_mode(void)
return internal_config.vfio_intr_mode;
}
+int rte_eal_manual_probe(void)
+{
+ return internal_config.manual_probe;
+}
+
+void rte_eal_manual_probe_set(int enabled)
+{
+ internal_config.manual_probe = !!enabled;
+}
+
int
rte_eal_check_module(const char *module_name)
{
@@ -312,6 +312,14 @@ DPDK_19.08 {
} DPDK_19.05;
+DPDK_19.11 {
+ global:
+
+ rte_eal_manual_probe;
+ rte_eal_manual_probe_set;
+
+} DPDK_19.08;
+
EXPERIMENTAL {
global: