[dpdk-dev,v2,04/11] bus: add bus helper iterator to find a particular device

Message ID e68718487dad2944f3108de8c93a616d8608a4b1.1496235017.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gaëtan Rivet May 31, 2017, 1:17 p.m. UTC
  From: Jan Blunck <jblunck@infradead.org>

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          | 25 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 50 insertions(+)
  

Comments

Jan Blunck June 7, 2017, 4:41 p.m. UTC | #1
On Wed, May 31, 2017 at 3:17 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> From: Jan Blunck <jblunck@infradead.org>
>
> 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          | 25 +++++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>  4 files changed, 50 insertions(+)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index f1a0765..21640d6 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -164,6 +164,7 @@ DPDK_17.05 {
>
>         rte_bus_find;
>         rte_bus_find_by_device;
> +       rte_bus_find_device;
>         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 811aff3..632a6e7 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -182,3 +182,28 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
>  {
>         return rte_bus_find(bus_find_device, (const void *)dev);
>  }
> +
> +struct rte_device *
> +rte_bus_find_device(const struct rte_device *start,
> +                   rte_dev_match_t match, const void *data)
> +{
> +       struct rte_bus *bus;
> +       struct rte_device *dev = NULL;
> +       int start_seen = 0;
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next) {

I don't think that this should use the TAILQ_FOREACH() macro since it
makes the code nesting deeper unnecessarily.


> +               if (!bus->find_device)
> +                       continue;
> +               if (!!start ^ start_seen) {
> +                       dev = bus->find_device(cmp_rte_device, start);
> +                       if (dev)
> +                               start_seen = 1;
> +                       else
> +                               continue;
> +               }
> +               dev = bus->find_device(match, data);
> +               if (dev)
> +                       break;
> +       }
> +       return dev;
> +}
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index 0664662..b56018f 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -180,6 +180,29 @@ typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
>  struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
>
>  /**
> + * Bus iterator to find a particular device.
> + *
> + * If the callback returns non-zero this function will stop iterating over any
> + * more buses and devices. To continue a search the device of a previous search
> + * is passed via the start parameters.
> + *
> + * @param start
> + *      Start device of the iteration
> + *
> + * @param match
> + *      Callback function to check device
> + *
> + * @param data
> + *      Data to pass to match callback
> + *
> + * @return
> + *      A pointer to a rte_bus structure or NULL in case no bus matches
> + */
> +struct rte_device *
> +rte_bus_find_device(const struct rte_device *start,
> +                   rte_dev_match_t match, const void *data);
> +
> +/**
>   * Find the registered bus for a particular device.
>   */
>  struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 6f77222..e0a056d 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -168,6 +168,7 @@ DPDK_17.05 {
>
>         rte_bus_find;
>         rte_bus_find_by_device;
> +       rte_bus_find_device;
>         rte_cpu_is_supported;
>         rte_intr_free_epoll_fd;
>         rte_log_dump;
> --
> 2.1.4
>
  
Gaëtan Rivet June 7, 2017, 8:12 p.m. UTC | #2
On Wed, Jun 07, 2017 at 06:41:58PM +0200, Jan Blunck wrote:
> On Wed, May 31, 2017 at 3:17 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > From: Jan Blunck <jblunck@infradead.org>
> >
> > 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          | 25 +++++++++++++++++++++++++
> >  lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> >  4 files changed, 50 insertions(+)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > index f1a0765..21640d6 100644
> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > @@ -164,6 +164,7 @@ DPDK_17.05 {
> >
> >         rte_bus_find;
> >         rte_bus_find_by_device;
> > +       rte_bus_find_device;
> >         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 811aff3..632a6e7 100644
> > --- a/lib/librte_eal/common/eal_common_bus.c
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -182,3 +182,28 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
> >  {
> >         return rte_bus_find(bus_find_device, (const void *)dev);
> >  }
> > +
> > +struct rte_device *
> > +rte_bus_find_device(const struct rte_device *start,
> > +                   rte_dev_match_t match, const void *data)
> > +{
> > +       struct rte_bus *bus;
> > +       struct rte_device *dev = NULL;
> > +       int start_seen = 0;
> > +
> > +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> 
> I don't think that this should use the TAILQ_FOREACH() macro since it
> makes the code nesting deeper unnecessarily.
> 
> 

While debugging the previous version, I found rather difficult to follow
the nesting of iterators / callback calls.

I preferred laying all of it down and simplifying the flow. The nesting
is either within the same scope or divided among several scopes. I
proposed the former as I found it easier to write and read, but that's
subjective.

> > +               if (!bus->find_device)
> > +                       continue;
> > +               if (!!start ^ start_seen) {
> > +                       dev = bus->find_device(cmp_rte_device, start);
> > +                       if (dev)
> > +                               start_seen = 1;
> > +                       else
> > +                               continue;
> > +               }
> > +               dev = bus->find_device(match, data);
> > +               if (dev)
> > +                       break;
> > +       }
> > +       return dev;
> > +}
> > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > index 0664662..b56018f 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -180,6 +180,29 @@ typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
> >  struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
> >
> >  /**
> > + * Bus iterator to find a particular device.
> > + *
> > + * If the callback returns non-zero this function will stop iterating over any
> > + * more buses and devices. To continue a search the device of a previous search
> > + * is passed via the start parameters.
> > + *
> > + * @param start
> > + *      Start device of the iteration
> > + *
> > + * @param match
> > + *      Callback function to check device
> > + *
> > + * @param data
> > + *      Data to pass to match callback
> > + *
> > + * @return
> > + *      A pointer to a rte_bus structure or NULL in case no bus matches
> > + */
> > +struct rte_device *
> > +rte_bus_find_device(const struct rte_device *start,
> > +                   rte_dev_match_t match, const void *data);
> > +
> > +/**
> >   * Find the registered bus for a particular device.
> >   */
> >  struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > index 6f77222..e0a056d 100644
> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > @@ -168,6 +168,7 @@ DPDK_17.05 {
> >
> >         rte_bus_find;
> >         rte_bus_find_by_device;
> > +       rte_bus_find_device;
> >         rte_cpu_is_supported;
> >         rte_intr_free_epoll_fd;
> >         rte_log_dump;
> > --
> > 2.1.4
> >
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f1a0765..21640d6 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -164,6 +164,7 @@  DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	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 811aff3..632a6e7 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -182,3 +182,28 @@  struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
 {
 	return rte_bus_find(bus_find_device, (const void *)dev);
 }
+
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_match_t match, const void *data)
+{
+	struct rte_bus *bus;
+	struct rte_device *dev = NULL;
+	int start_seen = 0;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!bus->find_device)
+			continue;
+		if (!!start ^ start_seen) {
+			dev = bus->find_device(cmp_rte_device, start);
+			if (dev)
+				start_seen = 1;
+			else
+				continue;
+		}
+		dev = bus->find_device(match, data);
+		if (dev)
+			break;
+	}
+	return dev;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 0664662..b56018f 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -180,6 +180,29 @@  typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
 struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
 
 /**
+ * Bus iterator to find a particular device.
+ *
+ * If the callback returns non-zero this function will stop iterating over any
+ * more buses and devices. To continue a search the device of a previous search
+ * is passed via the start parameters.
+ *
+ * @param start
+ *	 Start device of the iteration
+ *
+ * @param match
+ *	 Callback function to check device
+ *
+ * @param data
+ *	 Data to pass to match callback
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_match_t match, const void *data);
+
+/**
  * Find the registered bus for a particular device.
  */
 struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6f77222..e0a056d 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -168,6 +168,7 @@  DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;