[dpdk-dev,v3,01/10] devargs: introduce iterator

Message ID 5d49fe469f56c443c8a95688db1b96f13f169652.1521587199.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 fail Compilation issues

Commit Message

Gaëtan Rivet March 20, 2018, 11:20 p.m. UTC
  In preparation to making devargs_list private.

Bus drivers generally need to access rte_devargs pertaining to their
operations. This match is a common operation for bus drivers.

Add a new accessor for the rte_devargs list.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_devargs.c  | 20 ++++++++++++++++++++
 lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++
 lib/librte_eal/rte_eal_version.map          |  1 +
 3 files changed, 41 insertions(+)
  

Comments

Jianfeng Tan March 21, 2018, 5:43 a.m. UTC | #1
Hi Gaetan,


On 3/21/2018 7:20 AM, Gaetan Rivet wrote:
> In preparation to making devargs_list private.
>
> Bus drivers generally need to access rte_devargs pertaining to their
> operations. This match is a common operation for bus drivers.
>
> Add a new accessor for the rte_devargs list.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>   lib/librte_eal/common/eal_common_devargs.c  | 20 ++++++++++++++++++++
>   lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++
>   lib/librte_eal/rte_eal_version.map          |  1 +
>   3 files changed, 41 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index 810b3e18f..c6c5eabcf 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f)
>   			devargs->name, devargs->args);
>   	}
>   }
> +
> +/* bus-aware rte_devargs iterator. */
> +__rte_experimental
> +struct rte_devargs *
> +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
> +{
> +	struct rte_devargs *da;
> +
> +	if (start != NULL)
> +		da = TAILQ_NEXT(start, next);
> +	else
> +		da = TAILQ_FIRST(&devargs_list);
> +	while (da != NULL) {
> +		if (busname == NULL ||
> +		    (strcmp(busname, da->bus->name) == 0))
> +			return da;
> +		da = TAILQ_NEXT(da, next);
> +	}
> +	return NULL;
> +}
> diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> index 84e5e23c4..969a10449 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
>    */
>   void rte_eal_devargs_dump(FILE *f);
>   
> +/**
> + * Find next rte_devargs matching the provided bus name.
> + *
> + * @param busname
> + *   Limit the iteration to devargs related to buses
> + *   matching this name.
> + *   Will return any next rte_devargs if NULL.
> + *
> + * @param start
> + *   Starting iteration point. The iteration will start at
> + *   the first rte_devargs if NULL.
> + *
> + * @return
> + *   Next rte_devargs entry matching the requested bus,
> + *   NULL if there is none.
> + */
> +__rte_experimental
> +struct rte_devargs *
> +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index d12360235..02a040a8b 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -216,6 +216,7 @@ EXPERIMENTAL {
>   
>   	rte_eal_cleanup;
>   	rte_eal_devargs_insert;
> +	rte_eal_devargs_next;
>   	rte_eal_devargs_parse;
>   	rte_eal_devargs_remove;
>   	rte_eal_hotplug_add;

Shall we change these APIs to be thread-safe?

Thanks,
Jianfeng
  
Gaëtan Rivet March 21, 2018, 8:50 a.m. UTC | #2
Hello Jianfeng,

On Wed, Mar 21, 2018 at 01:43:49PM +0800, Tan, Jianfeng wrote:
> Hi Gaetan,
> 
> 
> On 3/21/2018 7:20 AM, Gaetan Rivet wrote:
> > In preparation to making devargs_list private.
> > 
> > Bus drivers generally need to access rte_devargs pertaining to their
> > operations. This match is a common operation for bus drivers.
> > 
> > Add a new accessor for the rte_devargs list.
> > 
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >   lib/librte_eal/common/eal_common_devargs.c  | 20 ++++++++++++++++++++
> >   lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++
> >   lib/librte_eal/rte_eal_version.map          |  1 +
> >   3 files changed, 41 insertions(+)
> > 
> > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> > index 810b3e18f..c6c5eabcf 100644
> > --- a/lib/librte_eal/common/eal_common_devargs.c
> > +++ b/lib/librte_eal/common/eal_common_devargs.c
> > @@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f)
> >   			devargs->name, devargs->args);
> >   	}
> >   }
> > +
> > +/* bus-aware rte_devargs iterator. */
> > +__rte_experimental
> > +struct rte_devargs *
> > +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
> > +{
> > +	struct rte_devargs *da;
> > +
> > +	if (start != NULL)
> > +		da = TAILQ_NEXT(start, next);
> > +	else
> > +		da = TAILQ_FIRST(&devargs_list);
> > +	while (da != NULL) {
> > +		if (busname == NULL ||
> > +		    (strcmp(busname, da->bus->name) == 0))
> > +			return da;
> > +		da = TAILQ_NEXT(da, next);
> > +	}
> > +	return NULL;
> > +}
> > diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> > index 84e5e23c4..969a10449 100644
> > --- a/lib/librte_eal/common/include/rte_devargs.h
> > +++ b/lib/librte_eal/common/include/rte_devargs.h
> > @@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
> >    */
> >   void rte_eal_devargs_dump(FILE *f);
> > +/**
> > + * Find next rte_devargs matching the provided bus name.
> > + *
> > + * @param busname
> > + *   Limit the iteration to devargs related to buses
> > + *   matching this name.
> > + *   Will return any next rte_devargs if NULL.
> > + *
> > + * @param start
> > + *   Starting iteration point. The iteration will start at
> > + *   the first rte_devargs if NULL.
> > + *
> > + * @return
> > + *   Next rte_devargs entry matching the requested bus,
> > + *   NULL if there is none.
> > + */
> > +__rte_experimental
> > +struct rte_devargs *
> > +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
> > +
> >   #ifdef __cplusplus
> >   }
> >   #endif
> > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> > index d12360235..02a040a8b 100644
> > --- a/lib/librte_eal/rte_eal_version.map
> > +++ b/lib/librte_eal/rte_eal_version.map
> > @@ -216,6 +216,7 @@ EXPERIMENTAL {
> >   	rte_eal_cleanup;
> >   	rte_eal_devargs_insert;
> > +	rte_eal_devargs_next;
> >   	rte_eal_devargs_parse;
> >   	rte_eal_devargs_remove;
> >   	rte_eal_hotplug_add;
> 
> Shall we change these APIs to be thread-safe?

It was not planned, but I think you raise an interesting point.

At the moment, most devargs operations are happening when a single
thread is started (eal init).

However, hotplug means having insertion and deletion possibly happening
in additional threads. This API exists and could help making these
operations thread-safe.

So this is low-priority I think for now (unless shown otherwise), but
definitely something to look into.
  

Patch

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 810b3e18f..c6c5eabcf 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -207,3 +207,23 @@  rte_eal_devargs_dump(FILE *f)
 			devargs->name, devargs->args);
 	}
 }
+
+/* bus-aware rte_devargs iterator. */
+__rte_experimental
+struct rte_devargs *
+rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
+{
+	struct rte_devargs *da;
+
+	if (start != NULL)
+		da = TAILQ_NEXT(start, next);
+	else
+		da = TAILQ_FIRST(&devargs_list);
+	while (da != NULL) {
+		if (busname == NULL ||
+		    (strcmp(busname, da->bus->name) == 0))
+			return da;
+		da = TAILQ_NEXT(da, next);
+	}
+	return NULL;
+}
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 84e5e23c4..969a10449 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -189,6 +189,26 @@  rte_eal_devargs_type_count(enum rte_devtype devtype);
  */
 void rte_eal_devargs_dump(FILE *f);
 
+/**
+ * Find next rte_devargs matching the provided bus name.
+ *
+ * @param busname
+ *   Limit the iteration to devargs related to buses
+ *   matching this name.
+ *   Will return any next rte_devargs if NULL.
+ *
+ * @param start
+ *   Starting iteration point. The iteration will start at
+ *   the first rte_devargs if NULL.
+ *
+ * @return
+ *   Next rte_devargs entry matching the requested bus,
+ *   NULL if there is none.
+ */
+__rte_experimental
+struct rte_devargs *
+rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d12360235..02a040a8b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -216,6 +216,7 @@  EXPERIMENTAL {
 
 	rte_eal_cleanup;
 	rte_eal_devargs_insert;
+	rte_eal_devargs_next;
 	rte_eal_devargs_parse;
 	rte_eal_devargs_remove;
 	rte_eal_hotplug_add;