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 7C091A32A3 for ; Fri, 25 Oct 2019 14:50:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C05ED1C1C5; Fri, 25 Oct 2019 14:50:54 +0200 (CEST) Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by dpdk.org (Postfix) with ESMTP id 4E7401C195 for ; Fri, 25 Oct 2019 14:50:53 +0200 (CEST) Received: by mail-wm1-f67.google.com with SMTP id r141so1894366wme.4 for ; Fri, 25 Oct 2019 05:50:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=P60lhqpNx3FdQBP/KL9D820VNu2u0GFicZ8y6EvI/2w=; b=D2F/ziN7tXgFe6xeaHDjQwP50xuXi8feB7xS9hZCO0cMyZ2gu6BIN8gRoiumt+wU3G VWel7bPjqlLQy7qFdJkY/TG74nBt5b68H2HAkbX6SSHDAbOJt2zDTg5r/90AiJaWLnjs n8P/Gu51vC/09051LmoqH3Ii2Ul+sykxULVip9Vwi60z+/cwa+1O8tlJ7Fo1qXKaMaOZ RpRhllk3fEGRiS9SQtD359dDVyYFkmMkAKCudWH2Ju8JFKHwAUYrlmnpXXPS2LxY3M8t ZT5DbY4bFembvbt3rc2RG0W2BI1EkXXdRw5C/XDG+oRnfl+s789K5bq/U69oRofz9++x DXrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=P60lhqpNx3FdQBP/KL9D820VNu2u0GFicZ8y6EvI/2w=; b=TKXI0LzlRsP3ZXLZkypdfjfOGzFqQLp2kazVJbS1hxgxMNCVS3Rv6PDE37GHeVFo2s hxYV+H4wNv/rZ797Xw8DejTJ20F2NjKc9jOh0CzltIDEMr0LGGFOtTEMTGRAUsDIJ+U7 14LiW41IIt0W6b73u8CCLWSg+kmYm7VkC4JdnN1HXzv1xvvnwf9bg4onzG5uKjm2hzLV fGr2XdbqBKTYS/eB3QvuPQAi54D7Dps+pOm+f/m2lGjwL1hYebn7TpDAKzY88iLdsYJ3 rQ6XCkktEGdVQKdBmATyDdHHgvK3PYvurE75hey8HdgfWrySnqWdZ7wvcNilBqEDzmkd r54A== X-Gm-Message-State: APjAAAWm1FFdmqtM/+Bn9lKZzTQ67L8agd8Pl6j2DA+7e2pQeNG7i53N 4Hf22wrULQsgH1DOm/wBYSD9Sw== X-Google-Smtp-Source: APXvYqzp1B+q63Hv93mHh9JV5To3loHsubvbc6trWotAdOztZOOONvEQy6Pmb0iRWYFiH18QXEiqfQ== X-Received: by 2002:a05:600c:2056:: with SMTP id p22mr3396237wmg.44.1572007852929; Fri, 25 Oct 2019 05:50:52 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a11sm2552356wmh.40.2019.10.25.05.50.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 25 Oct 2019 05:50:52 -0700 (PDT) Date: Fri, 25 Oct 2019 14:50:50 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Jerin Jacob Cc: dpdk-dev , Vamsi Krishna Attunuru , Ferruh Yigit , Anatoly Burakov , Thomas Monjalon , Jerin Jacob Kollanukkaran , David Marchand Message-ID: <20191025125050.m2zpdhmnakoeq43s@bidouze.vm.6wind.com> References: <6fff088fbca661b574b5cf27ebb924a690ed7d51.1570089310.git.gaetan.rivet@6wind.com> <5b8ee6827c26356cc009b0b3ff5b8d142b009fa7.1570193349.git.gaetan.rivet@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v3] 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 Fri, Oct 25, 2019 at 05:29:12PM +0530, Jerin Jacob wrote: > On Fri, Oct 4, 2019 at 6:25 PM Gaetan Rivet 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 > > --- > > > > 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 > > #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()) { > > 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 > > #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. -- Gaëtan Rivet 6WIND