[dpdk-dev,v5,02/12] bus: add device iterator method

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

Checks

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

Commit Message

Gaëtan Rivet June 26, 2017, 12:22 a.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/common/include/rte_bus.h | 19 +++++++++++++++++++
 lib/librte_eal/common/include/rte_dev.h | 21 +++++++++++++++++++++
 2 files changed, 40 insertions(+)
  

Comments

Bruce Richardson June 26, 2017, 4:20 p.m. UTC | #1
On Mon, Jun 26, 2017 at 02:22:00AM +0200, Gaetan Rivet 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/common/include/rte_bus.h | 19 +++++++++++++++++++
>  lib/librte_eal/common/include/rte_dev.h | 21 +++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index ecf839b..5efb76e 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -82,6 +82,24 @@ typedef int (*rte_bus_scan_t)(void);
>  typedef int (*rte_bus_probe_t)(void);
>  
>  /**
> + * Device iterator to find a device on a bus.
> + *
> + * This function returns an rte_device if one of those held by the bus
> + * matches the data passed as parameter.
> + *
> + * @param cmp
> + *	Comparison function.
> + *
> + * @param data
> + *	Data to compare each device against.
> + *
> + * @return
> + *	The first device matching the data, NULL if none exists.
> + */
> +typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t cmp,
> +						     const void *data);
> +
> +/**

The bus find function takes a third, start, parameter. Is it worthwhile
including such a parameter here, for consistency sake if nothing else?

Otherwise:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  
Gaëtan Rivet June 26, 2017, 9:13 p.m. UTC | #2
On Mon, Jun 26, 2017 at 05:20:37PM +0100, Bruce Richardson wrote:
> On Mon, Jun 26, 2017 at 02:22:00AM +0200, Gaetan Rivet 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/common/include/rte_bus.h | 19 +++++++++++++++++++
> >  lib/librte_eal/common/include/rte_dev.h | 21 +++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > index ecf839b..5efb76e 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -82,6 +82,24 @@ typedef int (*rte_bus_scan_t)(void);
> >  typedef int (*rte_bus_probe_t)(void);
> >  
> >  /**
> > + * Device iterator to find a device on a bus.
> > + *
> > + * This function returns an rte_device if one of those held by the bus
> > + * matches the data passed as parameter.
> > + *
> > + * @param cmp
> > + *	Comparison function.
> > + *
> > + * @param data
> > + *	Data to compare each device against.
> > + *
> > + * @return
> > + *	The first device matching the data, NULL if none exists.
> > + */
> > +typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t cmp,
> > +						     const void *data);
> > +
> > +/**
> 
> The bus find function takes a third, start, parameter. Is it worthwhile
> including such a parameter here, for consistency sake if nothing else?
> 

I was wondering this actually.

Doing so would deport the added complexity, to be repeated in
each bus implementation. It would in turn simplify my own calls,
ever so slightly. That means a few small-ish additional spots of
complexity instead of a single one.

I'm also thinking that maybe a few buses could avoid having linked lists
for their containers (I know it's frowned upon to roll your own, but
some might have good reasons for doing so), allowing random access, bypassing
cycling through each.

Well, I will do as you suggest, if anyone objects, please shout before I send the
next version.

> Otherwise:
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

Patch

diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index ecf839b..5efb76e 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -82,6 +82,24 @@  typedef int (*rte_bus_scan_t)(void);
 typedef int (*rte_bus_probe_t)(void);
 
 /**
+ * Device iterator to find a device on a bus.
+ *
+ * This function returns an rte_device if one of those held by the bus
+ * matches the data passed as parameter.
+ *
+ * @param cmp
+ *	Comparison function.
+ *
+ * @param data
+ *	Data to compare each device against.
+ *
+ * @return
+ *	The first device matching the data, NULL if none exists.
+ */
+typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t cmp,
+						     const void *data);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -89,6 +107,7 @@  struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
+	rte_bus_find_device_t find_device; /**< Find a device on bus */
 };
 
 /**
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..04d9c28 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -191,6 +191,27 @@  int rte_eal_dev_attach(const char *name, const char *devargs);
  */
 int rte_eal_dev_detach(const char *name);
 
+/**
+ * Device comparison function.
+ *
+ * This type of function is used to compare an rte_device with arbitrary
+ * data.
+ *
+ * @param dev
+ *   Device handle.
+ *
+ * @param data
+ *   Data to compare against. The type of this parameter is determined by
+ *   the kind of comparison performed by the function.
+ *
+ * @return
+ *   0 if the device matches the data.
+ *   !0 if the device does not match.
+ *   <0 if ordering is possible and the device is lower than the data.
+ *   >0 if ordering is possible and the device is greater than the data.
+ */
+typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);
+
 #define RTE_PMD_EXPORT_NAME_ARRAY(n, idx) n##idx[]
 
 #define RTE_PMD_EXPORT_NAME(name, idx) \