diff mbox series

[v2] kvargs: fix device iterator match from arguments

Message ID 20211124101740.3479061-1-xuemingl@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series [v2] kvargs: fix device iterator match from arguments |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS

Commit Message

Xueming Li Nov. 24, 2021, 10:17 a.m. UTC 
  Device iterator RTE_DEV_FOREACH() failed to return devices from
classifier like "class=vdpa", because matching name from empty kvargs
returns no result. If device name not specified in kvargs, the function
should iterate all devices.

This patch allows empty devargs or devargs without name specified.

Fixes: 6aebb942907d ("kvargs: add function to get from key and value")
Cc: olivier.matz@6wind.com

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
21.11 specific bug, no copy to stable.org
---
 drivers/bus/auxiliary/auxiliary_params.c | 14 +++++++++-----
 drivers/bus/vdev/vdev_params.c           | 14 +++++++++-----
 2 files changed, 18 insertions(+), 10 deletions(-)

Comments

Olivier Matz Nov. 24, 2021, 11:07 a.m. UTC | #1 
Hi Xueming,

On Wed, Nov 24, 2021 at 06:17:40PM +0800, Xueming Li wrote:
> Device iterator RTE_DEV_FOREACH() failed to return devices from
> classifier like "class=vdpa", because matching name from empty kvargs
> returns no result. If device name not specified in kvargs, the function
> should iterate all devices.
> 
> This patch allows empty devargs or devargs without name specified.
> 
> Fixes: 6aebb942907d ("kvargs: add function to get from key and value")
> Cc: olivier.matz@6wind.com
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>

I just sent a v3 with the unit test.

In case there is an issue with it, your v2 looks good to me:
Acked-by: Olivier Matz <olivier.matz@6wind.com>

As a side note, I was wondering if we should keep
rte_kvargs_get_with_value() in kvargs api, since there
is no more user. I think there it can still be useful, let's
keep it for now.

Thanks!
Olivier

> ---
> 21.11 specific bug, no copy to stable.org
> ---
>  drivers/bus/auxiliary/auxiliary_params.c | 14 +++++++++-----
>  drivers/bus/vdev/vdev_params.c           | 14 +++++++++-----
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/bus/auxiliary/auxiliary_params.c b/drivers/bus/auxiliary/auxiliary_params.c
> index a9c7853ed1d..9cbc1f7c777 100644
> --- a/drivers/bus/auxiliary/auxiliary_params.c
> +++ b/drivers/bus/auxiliary/auxiliary_params.c
> @@ -26,11 +26,15 @@ auxiliary_dev_match(const struct rte_device *dev,
>  {
>  	const struct rte_kvargs *kvlist = _kvlist;
>  	const char *key = auxiliary_params_keys[RTE_AUXILIARY_PARAM_NAME];
> -
> -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> -		return -1;
> -
> -	return 0;
> +	const char *name;
> +
> +	if (kvlist == NULL)
> +		return 0;
> +	name = rte_kvargs_get(kvlist, key);
> +	if (name == NULL)
> +		/* Iterate all devices if name not specified. */
> +		return 0;
> +	return strcmp(name, dev->name);
>  }
>  
>  void *
> diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
> index 37d95395e7a..b4baecb7c0d 100644
> --- a/drivers/bus/vdev/vdev_params.c
> +++ b/drivers/bus/vdev/vdev_params.c
> @@ -28,11 +28,15 @@ vdev_dev_match(const struct rte_device *dev,
>  {
>  	const struct rte_kvargs *kvlist = _kvlist;
>  	const char *key = vdev_params_keys[RTE_VDEV_PARAM_NAME];
> -
> -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> -		return -1;
> -
> -	return 0;
> +	const char *name;
> +
> +	if (kvlist == NULL)
> +		return 0;
> +	name = rte_kvargs_get(kvlist, key);
> +	if (name == NULL)
> +		/* Iterate all devices if name not specified. */
> +		return 0;
> +	return strcmp(name, dev->name);
>  }
>  
>  void *
> -- 
> 2.34.0
>
Xueming Li Nov. 24, 2021, 11:19 a.m. UTC | #2 
On Wed, 2021-11-24 at 12:07 +0100, Olivier Matz wrote:
> Hi Xueming,
> 
> On Wed, Nov 24, 2021 at 06:17:40PM +0800, Xueming Li wrote:
> > Device iterator RTE_DEV_FOREACH() failed to return devices from
> > classifier like "class=vdpa", because matching name from empty kvargs
> > returns no result. If device name not specified in kvargs, the function
> > should iterate all devices.
> > 
> > This patch allows empty devargs or devargs without name specified.
> > 
> > Fixes: 6aebb942907d ("kvargs: add function to get from key and value")
> > Cc: olivier.matz@6wind.com
> > 
> > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> 
> I just sent a v3 with the unit test.
> 
> In case there is an issue with it, your v2 looks good to me:
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> As a side note, I was wondering if we should keep
> rte_kvargs_get_with_value() in kvargs api, since there
> is no more user. I think there it can still be useful, let's
> keep it for now.

Agree, nice to keep.

> 
> Thanks!
> Olivier
> 
> > ---
> > 21.11 specific bug, no copy to stable.org
> > ---
> >  drivers/bus/auxiliary/auxiliary_params.c | 14 +++++++++-----
> >  drivers/bus/vdev/vdev_params.c           | 14 +++++++++-----
> >  2 files changed, 18 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/bus/auxiliary/auxiliary_params.c b/drivers/bus/auxiliary/auxiliary_params.c
> > index a9c7853ed1d..9cbc1f7c777 100644
> > --- a/drivers/bus/auxiliary/auxiliary_params.c
> > +++ b/drivers/bus/auxiliary/auxiliary_params.c
> > @@ -26,11 +26,15 @@ auxiliary_dev_match(const struct rte_device *dev,
> >  {
> >  	const struct rte_kvargs *kvlist = _kvlist;
> >  	const char *key = auxiliary_params_keys[RTE_AUXILIARY_PARAM_NAME];
> > -
> > -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> > -		return -1;
> > -
> > -	return 0;
> > +	const char *name;
> > +
> > +	if (kvlist == NULL)
> > +		return 0;
> > +	name = rte_kvargs_get(kvlist, key);
> > +	if (name == NULL)
> > +		/* Iterate all devices if name not specified. */
> > +		return 0;
> > +	return strcmp(name, dev->name);
> >  }
> >  
> >  void *
> > diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
> > index 37d95395e7a..b4baecb7c0d 100644
> > --- a/drivers/bus/vdev/vdev_params.c
> > +++ b/drivers/bus/vdev/vdev_params.c
> > @@ -28,11 +28,15 @@ vdev_dev_match(const struct rte_device *dev,
> >  {
> >  	const struct rte_kvargs *kvlist = _kvlist;
> >  	const char *key = vdev_params_keys[RTE_VDEV_PARAM_NAME];
> > -
> > -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> > -		return -1;
> > -
> > -	return 0;
> > +	const char *name;
> > +
> > +	if (kvlist == NULL)
> > +		return 0;
> > +	name = rte_kvargs_get(kvlist, key);
> > +	if (name == NULL)
> > +		/* Iterate all devices if name not specified. */
> > +		return 0;
> > +	return strcmp(name, dev->name);
> >  }
> >  
> >  void *
> > -- 
> > 2.34.0
> >
diff mbox series

Patch

diff --git a/drivers/bus/auxiliary/auxiliary_params.c b/drivers/bus/auxiliary/auxiliary_params.c
index a9c7853ed1d..9cbc1f7c777 100644
--- a/drivers/bus/auxiliary/auxiliary_params.c
+++ b/drivers/bus/auxiliary/auxiliary_params.c
@@ -26,11 +26,15 @@  auxiliary_dev_match(const struct rte_device *dev,
 {
 	const struct rte_kvargs *kvlist = _kvlist;
 	const char *key = auxiliary_params_keys[RTE_AUXILIARY_PARAM_NAME];
-
-	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
-		return -1;
-
-	return 0;
+	const char *name;
+
+	if (kvlist == NULL)
+		return 0;
+	name = rte_kvargs_get(kvlist, key);
+	if (name == NULL)
+		/* Iterate all devices if name not specified. */
+		return 0;
+	return strcmp(name, dev->name);
 }
 
 void *
diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
index 37d95395e7a..b4baecb7c0d 100644
--- a/drivers/bus/vdev/vdev_params.c
+++ b/drivers/bus/vdev/vdev_params.c
@@ -28,11 +28,15 @@  vdev_dev_match(const struct rte_device *dev,
 {
 	const struct rte_kvargs *kvlist = _kvlist;
 	const char *key = vdev_params_keys[RTE_VDEV_PARAM_NAME];
-
-	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
-		return -1;
-
-	return 0;
+	const char *name;
+
+	if (kvlist == NULL)
+		return 0;
+	name = rte_kvargs_get(kvlist, key);
+	if (name == NULL)
+		/* Iterate all devices if name not specified. */
+		return 0;
+	return strcmp(name, dev->name);
 }
 
 void *