[dpdk-dev,v5,01/12] bus: add bus iterator to find a bus
Checks
Commit Message
From: Jan Blunck <jblunck@infradead.org>
This helper allows to iterate over all registered buses and find one
matching data used as parameter.
Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
lib/librte_eal/common/eal_common_bus.c | 20 ++++++++++++
lib/librte_eal/common/include/rte_bus.h | 43 +++++++++++++++++++++++++
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
4 files changed, 65 insertions(+)
Comments
On Mon, Jun 26, 2017 at 02:21:59AM +0200, Gaetan Rivet wrote:
> From: Jan Blunck <jblunck@infradead.org>
>
> This helper allows to iterate over all registered buses and find one
> matching data used as parameter.
>
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> lib/librte_eal/common/eal_common_bus.c | 20 ++++++++++++
> lib/librte_eal/common/include/rte_bus.h | 43 +++++++++++++++++++++++++
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> 4 files changed, 65 insertions(+)
>
Two minor suggestions below. Otherwise:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2e48a73..ed09ab2 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -162,6 +162,7 @@ DPDK_17.02 {
> DPDK_17.05 {
> global:
>
> + rte_bus_find;
> rte_cpu_is_supported;
> rte_log_dump;
> rte_log_register;
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 8f9baf8..4619eb2 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -145,3 +145,23 @@ rte_bus_dump(FILE *f)
> }
> }
> }
> +
> +struct rte_bus *
> +rte_bus_find(rte_bus_cmp_t cmp,
> + const void *data,
> + const struct rte_bus *start)
> +{
> + struct rte_bus *bus = NULL;
> + int started = start == NULL;
Please put brackets around the "start == NULL" to improve readability.
My first reading of this I assumed it was doing multiple assignment of
both start and started to NULL. To make it extra clear, prefix the
comparison with "!!".
> +
> + TAILQ_FOREACH(bus, &rte_bus_list, next) {
> + if (!started) {
> + if (bus == start)
> + started = 1;
> + continue;
> + }
> + if (cmp(bus, data) == 0)
> + break;
> + }
> + return bus;
> +}
I also think the name "started" is confusing, and might be better as the
slighly longer "start_found".
Hi Bruce,
Thanks for reading.
On Mon, Jun 26, 2017 at 04:30:55PM +0100, Bruce Richardson wrote:
> On Mon, Jun 26, 2017 at 02:21:59AM +0200, Gaetan Rivet wrote:
> > From: Jan Blunck <jblunck@infradead.org>
> >
> > This helper allows to iterate over all registered buses and find one
> > matching data used as parameter.
> >
> > Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> > lib/librte_eal/common/eal_common_bus.c | 20 ++++++++++++
> > lib/librte_eal/common/include/rte_bus.h | 43 +++++++++++++++++++++++++
> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> > 4 files changed, 65 insertions(+)
> >
>
> Two minor suggestions below. Otherwise:
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > index 2e48a73..ed09ab2 100644
> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > @@ -162,6 +162,7 @@ DPDK_17.02 {
> > DPDK_17.05 {
> > global:
> >
> > + rte_bus_find;
> > rte_cpu_is_supported;
> > rte_log_dump;
> > rte_log_register;
> > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> > index 8f9baf8..4619eb2 100644
> > --- a/lib/librte_eal/common/eal_common_bus.c
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -145,3 +145,23 @@ rte_bus_dump(FILE *f)
> > }
> > }
> > }
> > +
> > +struct rte_bus *
> > +rte_bus_find(rte_bus_cmp_t cmp,
> > + const void *data,
> > + const struct rte_bus *start)
> > +{
> > + struct rte_bus *bus = NULL;
> > + int started = start == NULL;
>
> Please put brackets around the "start == NULL" to improve readability.
> My first reading of this I assumed it was doing multiple assignment of
> both start and started to NULL. To make it extra clear, prefix the
> comparison with "!!".
>
> > +
> > + TAILQ_FOREACH(bus, &rte_bus_list, next) {
> > + if (!started) {
> > + if (bus == start)
> > + started = 1;
> > + continue;
> > + }
> > + if (cmp(bus, data) == 0)
> > + break;
> > + }
> > + return bus;
> > +}
>
> I also think the name "started" is confusing, and might be better as the
> slighly longer "start_found".
>
Agreed on both account, it will be fixed in the next version.
@@ -162,6 +162,7 @@ DPDK_17.02 {
DPDK_17.05 {
global:
+ rte_bus_find;
rte_cpu_is_supported;
rte_log_dump;
rte_log_register;
@@ -145,3 +145,23 @@ rte_bus_dump(FILE *f)
}
}
}
+
+struct rte_bus *
+rte_bus_find(rte_bus_cmp_t cmp,
+ const void *data,
+ const struct rte_bus *start)
+{
+ struct rte_bus *bus = NULL;
+ int started = start == NULL;
+
+ TAILQ_FOREACH(bus, &rte_bus_list, next) {
+ if (!started) {
+ if (bus == start)
+ started = 1;
+ continue;
+ }
+ if (cmp(bus, data) == 0)
+ break;
+ }
+ return bus;
+}
@@ -141,6 +141,49 @@ int rte_bus_probe(void);
void rte_bus_dump(FILE *f);
/**
+ * Bus comparison function.
+ *
+ * @param bus
+ * Bus under test.
+ *
+ * @param data
+ * Data to compare against.
+ *
+ * @return
+ * 0 if the bus matches the data.
+ * !0 if the bus does not match.
+ * <0 if ordering is possible and the bus is lower than the data.
+ * >0 if ordering is possible and the bus is greater than the data.
+ */
+typedef int (*rte_bus_cmp_t)(const struct rte_bus *bus, const void *data);
+
+/**
+ * Bus iterator to find a particular bus.
+ *
+ * This function compares each registered bus to find one that matches
+ * the data passed as parameter.
+ *
+ * If the comparison function returns zero this function will stop iterating
+ * over any more buses. To continue a search the bus of a previous search can
+ * be passed via the start parameter.
+ *
+ * @param cmp
+ * Comparison function.
+ *
+ * @param data
+ * Data to pass to comparison function.
+ *
+ * @param start
+ * Starting point for the iteration.
+ *
+ * @return
+ * A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp,
+ const void *data,
+ const struct rte_bus *start);
+
+/**
* Helper for Bus registration.
* The constructor has higher priority than PMD constructors.
*/
@@ -166,6 +166,7 @@ DPDK_17.02 {
DPDK_17.05 {
global:
+ rte_bus_find;
rte_cpu_is_supported;
rte_intr_free_epoll_fd;
rte_log_dump;