From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 11600A0539; Thu, 23 Jan 2020 10:20:11 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F03402956; Thu, 23 Jan 2020 10:20:10 +0100 (CET) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by dpdk.org (Postfix) with ESMTP id 8B544DE3 for ; Thu, 23 Jan 2020 10:20:09 +0100 (CET) X-Originating-IP: 37.58.153.206 Received: from [10.0.3.88] (bny206.haproxy.com [37.58.153.206]) (Authenticated sender: grive@u256.net) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 564021BF209; Thu, 23 Jan 2020 09:20:07 +0000 (UTC) To: Pavan Nikhilesh Bhagavatula , "dev@dpdk.org" Cc: Vamsi Krishna Attunuru , David Marchand , Jerin Jacob Kollanukkaran References: <84075ad163f2eddf752690fc78f1d9bd9f7b0636.1572014017.git.gaetan.rivet@6wind.com> From: Gaetan Rivet Message-ID: <62c8e695-efe4-84e3-f467-edab22ae506f@u256.net> Date: Thu, 23 Jan 2020 10:20:07 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5] eal: add manual probing option X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 22/01/2020 17:51, Pavan Nikhilesh Bhagavatula wrote: > Ping > @grive@u256.net > > Since, you weren't reachable on regular email I rebased the patch on ToT and sent a v6 > http://patches.dpdk.org/patch/64235/ > Hello Pavan, indeed sorry about this. I have not heard remarks about the patchset anyway in the meantime, not sure whether there is anything to change. Thanks for the rebase, I will follow-up if there are remarks blocking the integration. >> -----Original Message----- >> From: dev On Behalf Of Gaetan Rivet >> Sent: Friday, October 25, 2019 9:17 PM >> To: dev@dpdk.org >> Cc: Gaetan Rivet ; Vamsi Krishna Attunuru >> ; David Marchand >> ; Jerin Jacob Kollanukkaran >> >> Subject: [dpdk-dev] [PATCH v5] 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 >> --- >> >> 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 (inherited 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. >> >> v4: Rebased on master, >> Moved implementation in common EAL, >> Used public API within the EAL to set the option, >> Made the API experimental >> >> v5: added note in the Getting Started Guide. >> >> doc/guides/linux_gsg/eal_args.include.rst | 13 +++++++ >> 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 | 54 >> ++++++++++++++++++++++++++++++ >> 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 | 36 >> ++++++++++++++++++++ >> lib/librte_eal/rte_eal_version.map | 4 +++ >> 10 files changed, 142 insertions(+) >> >> diff --git a/doc/guides/linux_gsg/eal_args.include.rst >> b/doc/guides/linux_gsg/eal_args.include.rst >> index ed8b0e35b..d0717d4a0 100644 >> --- a/doc/guides/linux_gsg/eal_args.include.rst >> +++ b/doc/guides/linux_gsg/eal_args.include.rst >> @@ -69,6 +69,19 @@ Device-related options >> >> --vdev 'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap' >> >> +* ``--manual-probe`` >> + >> + Switch the ``EAL`` probe mode to manual. The main bus probe step >> + is disabled and applications are expected to manually probe >> + devices using ``rte_dev_probe()``. >> + >> + Devices declared on the command-line using ``-w`` and ``-vdev`` >> + are interpreted as hotplug commands. They are thus probed in the >> + order they are declared. >> + >> + This makes this option useful to enforce a specific device probe >> + order, instead of relying on each bus scan implementation details. >> + >> * ``-d `` >> >> Load external drivers. An argument can be a single shared object file, >> or a >> diff --git a/doc/guides/rel_notes/release_19_11.rst >> b/doc/guides/rel_notes/release_19_11.rst >> index e77d226b5..7251a17b2 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 probing devices manually.** >> + >> + 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. >> + >> * **FreeBSD now supports `--base-virtaddr` EAL option.** >> >> FreeBSD version now also supports setting base virtual address for >> mapping >> 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 >> #include >> >> +#include >> #include >> #include >> #include >> @@ -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..368afa273 100644 >> --- a/lib/librte_eal/common/eal_common_dev.c >> +++ b/lib/librte_eal/common/eal_common_dev.c >> @@ -21,6 +21,7 @@ >> #include >> #include >> >> +#include "eal_internal_cfg.h" >> #include "eal_private.h" >> #include "hotplug_mp.h" >> >> @@ -83,6 +84,59 @@ rte_dev_is_probed(const struct rte_device *dev) >> return dev->driver != NULL; >> } >> >> +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_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; >> +} >> + >> /* helper function to build devargs, caller should free the memory */ >> static int >> build_devargs(const char *busname, const char *devname, >> diff --git a/lib/librte_eal/common/eal_common_options.c >> b/lib/librte_eal/common/eal_common_options.c >> index a7f9c5f9b..de73970fe 100644 >> --- a/lib/librte_eal/common/eal_common_options.c >> +++ b/lib/librte_eal/common/eal_common_options.c >> @@ -82,6 +82,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 } >> }; >> >> @@ -1446,6 +1447,9 @@ eal_parse_common_option(int opt, const >> char *optarg, >> return -1; >> } >> break; >> + case OPT_MANUAL_PROBE_NUM: >> + rte_eal_manual_probe_set(1); >> + break; >> >> /* don't know what to do, leave this to caller */ >> default: >> @@ -1672,6 +1676,10 @@ eal_common_usage(void) >> " --"OPT_VDEV" Add a virtual device.\n" >> " The argument format is >> [,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..71d9b4ca6 100644 >> --- a/lib/librte_eal/common/include/rte_eal.h >> +++ b/lib/librte_eal/common/include/rte_eal.h >> @@ -465,6 +465,42 @@ 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 >> + */ >> +__rte_experimental >> +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 >> + */ >> +__rte_experimental >> +void rte_eal_manual_probe_set(int enabled); >> + >> +/** >> * Whether the EAL was asked to create UIO device. >> * >> * @return >> diff --git a/lib/librte_eal/rte_eal_version.map >> b/lib/librte_eal/rte_eal_version.map >> index 7cbf82d37..7d7ff6531 100644 >> --- a/lib/librte_eal/rte_eal_version.map >> +++ b/lib/librte_eal/rte_eal_version.map >> @@ -419,4 +419,8 @@ EXPERIMENTAL { >> rte_mcfg_timer_lock; >> rte_mcfg_timer_unlock; >> rte_rand_max; >> + >> + # added in 19.11 >> + rte_eal_manual_probe; >> + rte_eal_manual_probe_set; >> }; >> -- >> 2.11.0 >