diff mbox series

[v3] bus: fix device iterator match from arguments

Message ID 20211124110244.26464-1-olivier.matz@6wind.com (mailing list archive)
State Superseded, archived
Headers
Series [v3] bus: fix device iterator match from arguments |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS

Commit Message

Olivier Matz Nov. 24, 2021, 11:02 a.m. UTC 
  From: Xueming Li <xuemingl@nvidia.com>

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")

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
bug is specific to 21.11, no need to cc stable@dpdk.org
---
 app/test/meson.build                     |   3 +
 app/test/test_vdev.c                     | 168 +++++++++++++++++++++++
 drivers/bus/auxiliary/auxiliary_params.c |   9 +-
 drivers/bus/vdev/vdev_params.c           |   9 +-
 4 files changed, 187 insertions(+), 2 deletions(-)
 create mode 100644 app/test/test_vdev.c

Comments

Xueming Li Nov. 24, 2021, 11:31 a.m. UTC | #1 
On Wed, 2021-11-24 at 12:02 +0100, Olivier Matz wrote:
> From: Xueming Li <xuemingl@nvidia.com>
> 
> 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")
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> bug is specific to 21.11, no need to cc stable@dpdk.org
> ---
>  app/test/meson.build                     |   3 +
>  app/test/test_vdev.c                     | 168 +++++++++++++++++++++++
>  drivers/bus/auxiliary/auxiliary_params.c |   9 +-
>  drivers/bus/vdev/vdev_params.c           |   9 +-
>  4 files changed, 187 insertions(+), 2 deletions(-)
>  create mode 100644 app/test/test_vdev.c
> 
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 961bebc5cb..19009da52e 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -153,6 +153,7 @@ test_sources = files(
>          'test_trace.c',
>          'test_trace_register.c',
>          'test_trace_perf.c',
> +        'test_vdev.c',
>          'test_version.c',
>          'virtual_pmd.c',
>  )
> @@ -177,6 +178,7 @@ test_deps = [
>          'ipsec',
>          'lpm',
>          'member',
> +        'net_null',

From my experience, CI will fail if build not configured with net_null,
even we add it here. Let's monitor CI result. 

>          'node',
>          'pipeline',
>          'port',
> @@ -283,6 +285,7 @@ fast_tests = [
>          ['service_autotest', true],
>          ['thash_autotest', true],
>          ['trace_autotest', true],
> +        ['vdev_autotest', true],
>  ]
>  
>  # Tests known to have issues or which don't belong in other tests lists.
> diff --git a/app/test/test_vdev.c b/app/test/test_vdev.c
> new file mode 100644
> index 0000000000..720722c363
> --- /dev/null
> +++ b/app/test/test_vdev.c
> @@ -0,0 +1,168 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2021 6WIND S.A.
> + */
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <rte_common.h>
> +#include <rte_kvargs.h>
> +#include <rte_bus_vdev.h>
> +
> +#include "test.h"
> +
> +#define TEST_VDEV_KEY_NAME "name"
> +
> +static const char * const valid_keys[] = {
> +	TEST_VDEV_KEY_NAME,
> +	NULL,
> +};
> +
> +static int
> +cmp_dev_name(const struct rte_device *dev, const void *name)
> +{
> +	return strcmp(dev->name, name);
> +}
> +
> +static int
> +cmp_dev_match(const struct rte_device *dev, const void *_kvlist)
> +{
> +	const struct rte_kvargs *kvlist = _kvlist;
> +	const char *key = TEST_VDEV_KEY_NAME;
> +	const char *name;
> +
> +	/* no kvlist arg, all devices match */
> +	if (kvlist == NULL)
> +		return 0;
> +
> +	/* if key is present in kvlist and does not match, filter device */
> +	name = rte_kvargs_get(kvlist, key);
> +	if (name != NULL && strcmp(name, dev->name))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static struct rte_device *
> +get_matching_vdev(const char *match_str)
> +{
> +	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
> +	struct rte_kvargs *kvargs = NULL;
> +	struct rte_device *dev;
> +
> +	if (match_str != NULL) {
> +		kvargs = rte_kvargs_parse(match_str, valid_keys);
> +		if (kvargs == NULL) {
> +			printf("Failed to parse match string\n");
> +			return NULL;
> +		}
> +	}
> +
> +	dev = vdev_bus->find_device(NULL, cmp_dev_match, kvargs);
> +	rte_kvargs_free(kvargs);
> +
> +	return dev;
> +}
> +
> +static int
> +test_vdev_bus(void)
> +{
> +	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
> +	struct rte_dev_iterator dev_iter = { 0 };
> +	struct rte_device *dev, *dev0, *dev1;
> +
> +	/* not supported */
> +	if (vdev_bus == NULL)
> +		return 0;
> +
> +	/* create first vdev */
> +	if (rte_vdev_init("net_null_test0", "") < 0) {
> +		printf("Failed to create vdev net_null_test0\n");
> +		goto fail;
> +	}
> +	dev0 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test0");
> +	if (dev0 == NULL) {
> +		printf("Cannot find net_null_test0 vdev\n");
> +		goto fail;
> +	}
> +
> +	/* create second vdev */
> +	if (rte_vdev_init("net_null_test1", "") < 0) {
> +		printf("Failed to create vdev net_null_test1\n");
> +		goto fail;
> +	}
> +	dev1 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test1");
> +	if (dev1 == NULL) {
> +		printf("Cannot find net_null_test1 vdev\n");
> +		goto fail;
> +	}
> +
> +	/* try to match vdevs */
> +	dev = get_matching_vdev("name=net_null_test0");
> +	if (dev != dev0) {
> +		printf("Cannot match net_null_test0 vdev\n");
> +		goto fail;
> +	}
> +
> +	dev = get_matching_vdev("name=net_null_test1");
> +	if (dev != dev1) {
> +		printf("Cannot match net_null_test1 vdev\n");
> +		goto fail;
> +	}
> +
> +	dev = get_matching_vdev("name=unexistant");
> +	if (dev != NULL) {
> +		printf("Unexistant vdev should not match\n");
> +		goto fail;
> +	}
> +
> +	dev = get_matching_vdev("");
> +	if (dev == NULL || dev == dev1) {
> +		printf("Cannot match any vdev with empty match string\n");
> +		goto fail;
> +	}
> +
> +	dev = get_matching_vdev(NULL);
> +	if (dev == NULL || dev == dev1) {
> +		printf("Cannot match any vdev with NULL match string\n");
> +		goto fail;
> +	}
> +
> +	/* iterate all vdevs, and ensure we find vdev0 and vdev1 */
> +	RTE_DEV_FOREACH(dev, "bus=vdev", &dev_iter) {
> +		if (dev == dev0)
> +			dev0 = NULL;
> +		else if (dev == dev1)
> +			dev1 = NULL;
> +	}
> +	if (dev0 != NULL) {
> +		printf("dev0 was not iterated\n");
> +		goto fail;
> +	}
> +	if (dev1 != NULL) {
> +		printf("dev1 was not iterated\n");
> +		goto fail;
> +	}
> +
> +	rte_vdev_uninit("net_null_test0");
> +	rte_vdev_uninit("net_null_test1");
> +
> +	return 0;
> +
> +fail:
> +	rte_vdev_uninit("net_null_test0");
> +	rte_vdev_uninit("net_null_test1");
> +	return -1;
> +}
> +
> +static int
> +test_vdev(void)
> +{
> +	printf("== test vdev bus ==\n");
> +	if (test_vdev_bus() < 0)
> +		return -1;
> +	return 0;
> +}
> +
> +REGISTER_TEST_COMMAND(vdev_autotest, test_vdev);
> diff --git a/drivers/bus/auxiliary/auxiliary_params.c b/drivers/bus/auxiliary/auxiliary_params.c
> index 8dd8813611..9c08ccdd1b 100644
> --- a/drivers/bus/auxiliary/auxiliary_params.c
> +++ b/drivers/bus/auxiliary/auxiliary_params.c
> @@ -28,8 +28,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];
> +	const char *name;
>  
> -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> +	/* no kvlist arg, all devices match */
> +	if (kvlist == NULL)
> +		return 0;
> +
> +	/* if key is present in kvlist and does not match, filter device */
> +	name = rte_kvargs_get(kvlist, key);
> +	if (name != NULL && strcmp(name, dev->name))
>  		return -1;
>  
>  	return 0;
> diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
> index 37d95395e7..3969faf16d 100644
> --- a/drivers/bus/vdev/vdev_params.c
> +++ b/drivers/bus/vdev/vdev_params.c
> @@ -28,8 +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];
> +	const char *name;
>  
> -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> +	/* no kvlist arg, all devices match */
> +	if (kvlist == NULL)
> +		return 0;
> +
> +	/* if key is present in kvlist and does not match, filter device */
> +	name = rte_kvargs_get(kvlist, key);
> +	if (name != NULL && strcmp(name, dev->name))
>  		return -1;
>  
>  	return 0;

Nice unit test!

Reviewed-by: Xueming Li <xuemingl@nvidia.com>
Xueming Li Nov. 24, 2021, 11:43 a.m. UTC | #2 
On Wed, 2021-11-24 at 19:30 +0800, Xueming Li wrote:
> On Wed, 2021-11-24 at 12:02 +0100, Olivier Matz wrote:
> > From: Xueming Li <xuemingl@nvidia.com>
> > 
> > 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")
> > 
> > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> > bug is specific to 21.11, no need to cc stable@dpdk.org
> > ---
> >  app/test/meson.build                     |   3 +
> >  app/test/test_vdev.c                     | 168 +++++++++++++++++++++++
> >  drivers/bus/auxiliary/auxiliary_params.c |   9 +-
> >  drivers/bus/vdev/vdev_params.c           |   9 +-
> >  4 files changed, 187 insertions(+), 2 deletions(-)
> >  create mode 100644 app/test/test_vdev.c
> > 
> > diff --git a/app/test/meson.build b/app/test/meson.build
> > index 961bebc5cb..19009da52e 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -153,6 +153,7 @@ test_sources = files(
> >          'test_trace.c',
> >          'test_trace_register.c',
> >          'test_trace_perf.c',
> > +        'test_vdev.c',
> >          'test_version.c',
> >          'virtual_pmd.c',
> >  )
> > @@ -177,6 +178,7 @@ test_deps = [
> >          'ipsec',
> >          'lpm',
> >          'member',
> > +        'net_null',
> 
> From my experience, CI will fail if build not configured with net_null,
> even we add it here. Let's monitor CI result. 

Sorry, from CI scripts, net_null was there already, no concern.

> 
> >          'node',
> >          'pipeline',
> >          'port',
> > @@ -283,6 +285,7 @@ fast_tests = [
> >          ['service_autotest', true],
> >          ['thash_autotest', true],
> >          ['trace_autotest', true],
> > +        ['vdev_autotest', true],
> >  ]
> >  
> >  # Tests known to have issues or which don't belong in other tests lists.
> > diff --git a/app/test/test_vdev.c b/app/test/test_vdev.c
> > new file mode 100644
> > index 0000000000..720722c363
> > --- /dev/null
> > +++ b/app/test/test_vdev.c
> > @@ -0,0 +1,168 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2021 6WIND S.A.
> > + */
> > +
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +
> > +#include <rte_common.h>
> > +#include <rte_kvargs.h>
> > +#include <rte_bus_vdev.h>
> > +
> > +#include "test.h"
> > +
> > +#define TEST_VDEV_KEY_NAME "name"
> > +
> > +static const char * const valid_keys[] = {
> > +	TEST_VDEV_KEY_NAME,
> > +	NULL,
> > +};
> > +
> > +static int
> > +cmp_dev_name(const struct rte_device *dev, const void *name)
> > +{
> > +	return strcmp(dev->name, name);
> > +}
> > +
> > +static int
> > +cmp_dev_match(const struct rte_device *dev, const void *_kvlist)
> > +{
> > +	const struct rte_kvargs *kvlist = _kvlist;
> > +	const char *key = TEST_VDEV_KEY_NAME;
> > +	const char *name;
> > +
> > +	/* no kvlist arg, all devices match */
> > +	if (kvlist == NULL)
> > +		return 0;
> > +
> > +	/* if key is present in kvlist and does not match, filter device */
> > +	name = rte_kvargs_get(kvlist, key);
> > +	if (name != NULL && strcmp(name, dev->name))
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> > +static struct rte_device *
> > +get_matching_vdev(const char *match_str)
> > +{
> > +	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
> > +	struct rte_kvargs *kvargs = NULL;
> > +	struct rte_device *dev;
> > +
> > +	if (match_str != NULL) {
> > +		kvargs = rte_kvargs_parse(match_str, valid_keys);
> > +		if (kvargs == NULL) {
> > +			printf("Failed to parse match string\n");
> > +			return NULL;
> > +		}
> > +	}
> > +
> > +	dev = vdev_bus->find_device(NULL, cmp_dev_match, kvargs);
> > +	rte_kvargs_free(kvargs);
> > +
> > +	return dev;
> > +}
> > +
> > +static int
> > +test_vdev_bus(void)
> > +{
> > +	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
> > +	struct rte_dev_iterator dev_iter = { 0 };
> > +	struct rte_device *dev, *dev0, *dev1;
> > +
> > +	/* not supported */
> > +	if (vdev_bus == NULL)
> > +		return 0;
> > +
> > +	/* create first vdev */
> > +	if (rte_vdev_init("net_null_test0", "") < 0) {
> > +		printf("Failed to create vdev net_null_test0\n");
> > +		goto fail;
> > +	}
> > +	dev0 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test0");
> > +	if (dev0 == NULL) {
> > +		printf("Cannot find net_null_test0 vdev\n");
> > +		goto fail;
> > +	}
> > +
> > +	/* create second vdev */
> > +	if (rte_vdev_init("net_null_test1", "") < 0) {
> > +		printf("Failed to create vdev net_null_test1\n");
> > +		goto fail;
> > +	}
> > +	dev1 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test1");
> > +	if (dev1 == NULL) {
> > +		printf("Cannot find net_null_test1 vdev\n");
> > +		goto fail;
> > +	}
> > +
> > +	/* try to match vdevs */
> > +	dev = get_matching_vdev("name=net_null_test0");
> > +	if (dev != dev0) {
> > +		printf("Cannot match net_null_test0 vdev\n");
> > +		goto fail;
> > +	}
> > +
> > +	dev = get_matching_vdev("name=net_null_test1");
> > +	if (dev != dev1) {
> > +		printf("Cannot match net_null_test1 vdev\n");
> > +		goto fail;
> > +	}
> > +
> > +	dev = get_matching_vdev("name=unexistant");
> > +	if (dev != NULL) {
> > +		printf("Unexistant vdev should not match\n");
> > +		goto fail;
> > +	}
> > +
> > +	dev = get_matching_vdev("");
> > +	if (dev == NULL || dev == dev1) {
> > +		printf("Cannot match any vdev with empty match string\n");
> > +		goto fail;
> > +	}
> > +
> > +	dev = get_matching_vdev(NULL);
> > +	if (dev == NULL || dev == dev1) {
> > +		printf("Cannot match any vdev with NULL match string\n");
> > +		goto fail;
> > +	}
> > +
> > +	/* iterate all vdevs, and ensure we find vdev0 and vdev1 */
> > +	RTE_DEV_FOREACH(dev, "bus=vdev", &dev_iter) {
> > +		if (dev == dev0)
> > +			dev0 = NULL;
> > +		else if (dev == dev1)
> > +			dev1 = NULL;
> > +	}
> > +	if (dev0 != NULL) {
> > +		printf("dev0 was not iterated\n");
> > +		goto fail;
> > +	}
> > +	if (dev1 != NULL) {
> > +		printf("dev1 was not iterated\n");
> > +		goto fail;
> > +	}
> > +
> > +	rte_vdev_uninit("net_null_test0");
> > +	rte_vdev_uninit("net_null_test1");
> > +
> > +	return 0;
> > +
> > +fail:
> > +	rte_vdev_uninit("net_null_test0");
> > +	rte_vdev_uninit("net_null_test1");
> > +	return -1;
> > +}
> > +
> > +static int
> > +test_vdev(void)
> > +{
> > +	printf("== test vdev bus ==\n");
> > +	if (test_vdev_bus() < 0)
> > +		return -1;
> > +	return 0;
> > +}
> > +
> > +REGISTER_TEST_COMMAND(vdev_autotest, test_vdev);
> > diff --git a/drivers/bus/auxiliary/auxiliary_params.c b/drivers/bus/auxiliary/auxiliary_params.c
> > index 8dd8813611..9c08ccdd1b 100644
> > --- a/drivers/bus/auxiliary/auxiliary_params.c
> > +++ b/drivers/bus/auxiliary/auxiliary_params.c
> > @@ -28,8 +28,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];
> > +	const char *name;
> >  
> > -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> > +	/* no kvlist arg, all devices match */
> > +	if (kvlist == NULL)
> > +		return 0;
> > +
> > +	/* if key is present in kvlist and does not match, filter device */
> > +	name = rte_kvargs_get(kvlist, key);
> > +	if (name != NULL && strcmp(name, dev->name))
> >  		return -1;
> >  
> >  	return 0;
> > diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
> > index 37d95395e7..3969faf16d 100644
> > --- a/drivers/bus/vdev/vdev_params.c
> > +++ b/drivers/bus/vdev/vdev_params.c
> > @@ -28,8 +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];
> > +	const char *name;
> >  
> > -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> > +	/* no kvlist arg, all devices match */
> > +	if (kvlist == NULL)
> > +		return 0;
> > +
> > +	/* if key is present in kvlist and does not match, filter device */
> > +	name = rte_kvargs_get(kvlist, key);
> > +	if (name != NULL && strcmp(name, dev->name))
> >  		return -1;
> >  
> >  	return 0;
> 
> Nice unit test!
> 
> Reviewed-by: Xueming Li <xuemingl@nvidia.com>
>
Olivier Matz Nov. 24, 2021, 12:14 p.m. UTC | #3 
On Wed, Nov 24, 2021 at 11:43:29AM +0000, Xueming(Steven) Li wrote:
> On Wed, 2021-11-24 at 19:30 +0800, Xueming Li wrote:
> > On Wed, 2021-11-24 at 12:02 +0100, Olivier Matz wrote:
> > > From: Xueming Li <xuemingl@nvidia.com>
> > > 
> > > 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")
> > > 
> > > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > ---
> > > bug is specific to 21.11, no need to cc stable@dpdk.org
> > > ---
> > >  app/test/meson.build                     |   3 +
> > >  app/test/test_vdev.c                     | 168 +++++++++++++++++++++++
> > >  drivers/bus/auxiliary/auxiliary_params.c |   9 +-
> > >  drivers/bus/vdev/vdev_params.c           |   9 +-
> > >  4 files changed, 187 insertions(+), 2 deletions(-)
> > >  create mode 100644 app/test/test_vdev.c
> > > 
> > > diff --git a/app/test/meson.build b/app/test/meson.build
> > > index 961bebc5cb..19009da52e 100644
> > > --- a/app/test/meson.build
> > > +++ b/app/test/meson.build
> > > @@ -153,6 +153,7 @@ test_sources = files(
> > >          'test_trace.c',
> > >          'test_trace_register.c',
> > >          'test_trace_perf.c',
> > > +        'test_vdev.c',
> > >          'test_version.c',
> > >          'virtual_pmd.c',
> > >  )
> > > @@ -177,6 +178,7 @@ test_deps = [
> > >          'ipsec',
> > >          'lpm',
> > >          'member',
> > > +        'net_null',
> > 
> > From my experience, CI will fail if build not configured with net_null,
> > even we add it here. Let's monitor CI result. 
> 
> Sorry, from CI scripts, net_null was there already, no concern.

No you are right! I had almost the same comment from David offline.
I'm fixing the meson.build to add the test only when RTE_NET_NULL is set.

Thanks

> > 
> > >          'node',
> > >          'pipeline',
> > >          'port',
> > > @@ -283,6 +285,7 @@ fast_tests = [
> > >          ['service_autotest', true],
> > >          ['thash_autotest', true],
> > >          ['trace_autotest', true],
> > > +        ['vdev_autotest', true],
> > >  ]
> > >  
> > >  # Tests known to have issues or which don't belong in other tests lists.
> > > diff --git a/app/test/test_vdev.c b/app/test/test_vdev.c
> > > new file mode 100644
> > > index 0000000000..720722c363
> > > --- /dev/null
> > > +++ b/app/test/test_vdev.c
> > > @@ -0,0 +1,168 @@
> > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > + * Copyright 2021 6WIND S.A.
> > > + */
> > > +
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +#include <string.h>
> > > +
> > > +#include <rte_common.h>
> > > +#include <rte_kvargs.h>
> > > +#include <rte_bus_vdev.h>
> > > +
> > > +#include "test.h"
> > > +
> > > +#define TEST_VDEV_KEY_NAME "name"
> > > +
> > > +static const char * const valid_keys[] = {
> > > +	TEST_VDEV_KEY_NAME,
> > > +	NULL,
> > > +};
> > > +
> > > +static int
> > > +cmp_dev_name(const struct rte_device *dev, const void *name)
> > > +{
> > > +	return strcmp(dev->name, name);
> > > +}
> > > +
> > > +static int
> > > +cmp_dev_match(const struct rte_device *dev, const void *_kvlist)
> > > +{
> > > +	const struct rte_kvargs *kvlist = _kvlist;
> > > +	const char *key = TEST_VDEV_KEY_NAME;
> > > +	const char *name;
> > > +
> > > +	/* no kvlist arg, all devices match */
> > > +	if (kvlist == NULL)
> > > +		return 0;
> > > +
> > > +	/* if key is present in kvlist and does not match, filter device */
> > > +	name = rte_kvargs_get(kvlist, key);
> > > +	if (name != NULL && strcmp(name, dev->name))
> > > +		return -1;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static struct rte_device *
> > > +get_matching_vdev(const char *match_str)
> > > +{
> > > +	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
> > > +	struct rte_kvargs *kvargs = NULL;
> > > +	struct rte_device *dev;
> > > +
> > > +	if (match_str != NULL) {
> > > +		kvargs = rte_kvargs_parse(match_str, valid_keys);
> > > +		if (kvargs == NULL) {
> > > +			printf("Failed to parse match string\n");
> > > +			return NULL;
> > > +		}
> > > +	}
> > > +
> > > +	dev = vdev_bus->find_device(NULL, cmp_dev_match, kvargs);
> > > +	rte_kvargs_free(kvargs);
> > > +
> > > +	return dev;
> > > +}
> > > +
> > > +static int
> > > +test_vdev_bus(void)
> > > +{
> > > +	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
> > > +	struct rte_dev_iterator dev_iter = { 0 };
> > > +	struct rte_device *dev, *dev0, *dev1;
> > > +
> > > +	/* not supported */
> > > +	if (vdev_bus == NULL)
> > > +		return 0;
> > > +
> > > +	/* create first vdev */
> > > +	if (rte_vdev_init("net_null_test0", "") < 0) {
> > > +		printf("Failed to create vdev net_null_test0\n");
> > > +		goto fail;
> > > +	}
> > > +	dev0 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test0");
> > > +	if (dev0 == NULL) {
> > > +		printf("Cannot find net_null_test0 vdev\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	/* create second vdev */
> > > +	if (rte_vdev_init("net_null_test1", "") < 0) {
> > > +		printf("Failed to create vdev net_null_test1\n");
> > > +		goto fail;
> > > +	}
> > > +	dev1 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test1");
> > > +	if (dev1 == NULL) {
> > > +		printf("Cannot find net_null_test1 vdev\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	/* try to match vdevs */
> > > +	dev = get_matching_vdev("name=net_null_test0");
> > > +	if (dev != dev0) {
> > > +		printf("Cannot match net_null_test0 vdev\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	dev = get_matching_vdev("name=net_null_test1");
> > > +	if (dev != dev1) {
> > > +		printf("Cannot match net_null_test1 vdev\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	dev = get_matching_vdev("name=unexistant");
> > > +	if (dev != NULL) {
> > > +		printf("Unexistant vdev should not match\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	dev = get_matching_vdev("");
> > > +	if (dev == NULL || dev == dev1) {
> > > +		printf("Cannot match any vdev with empty match string\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	dev = get_matching_vdev(NULL);
> > > +	if (dev == NULL || dev == dev1) {
> > > +		printf("Cannot match any vdev with NULL match string\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	/* iterate all vdevs, and ensure we find vdev0 and vdev1 */
> > > +	RTE_DEV_FOREACH(dev, "bus=vdev", &dev_iter) {
> > > +		if (dev == dev0)
> > > +			dev0 = NULL;
> > > +		else if (dev == dev1)
> > > +			dev1 = NULL;
> > > +	}
> > > +	if (dev0 != NULL) {
> > > +		printf("dev0 was not iterated\n");
> > > +		goto fail;
> > > +	}
> > > +	if (dev1 != NULL) {
> > > +		printf("dev1 was not iterated\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	rte_vdev_uninit("net_null_test0");
> > > +	rte_vdev_uninit("net_null_test1");
> > > +
> > > +	return 0;
> > > +
> > > +fail:
> > > +	rte_vdev_uninit("net_null_test0");
> > > +	rte_vdev_uninit("net_null_test1");
> > > +	return -1;
> > > +}
> > > +
> > > +static int
> > > +test_vdev(void)
> > > +{
> > > +	printf("== test vdev bus ==\n");
> > > +	if (test_vdev_bus() < 0)
> > > +		return -1;
> > > +	return 0;
> > > +}
> > > +
> > > +REGISTER_TEST_COMMAND(vdev_autotest, test_vdev);
> > > diff --git a/drivers/bus/auxiliary/auxiliary_params.c b/drivers/bus/auxiliary/auxiliary_params.c
> > > index 8dd8813611..9c08ccdd1b 100644
> > > --- a/drivers/bus/auxiliary/auxiliary_params.c
> > > +++ b/drivers/bus/auxiliary/auxiliary_params.c
> > > @@ -28,8 +28,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];
> > > +	const char *name;
> > >  
> > > -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> > > +	/* no kvlist arg, all devices match */
> > > +	if (kvlist == NULL)
> > > +		return 0;
> > > +
> > > +	/* if key is present in kvlist and does not match, filter device */
> > > +	name = rte_kvargs_get(kvlist, key);
> > > +	if (name != NULL && strcmp(name, dev->name))
> > >  		return -1;
> > >  
> > >  	return 0;
> > > diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
> > > index 37d95395e7..3969faf16d 100644
> > > --- a/drivers/bus/vdev/vdev_params.c
> > > +++ b/drivers/bus/vdev/vdev_params.c
> > > @@ -28,8 +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];
> > > +	const char *name;
> > >  
> > > -	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
> > > +	/* no kvlist arg, all devices match */
> > > +	if (kvlist == NULL)
> > > +		return 0;
> > > +
> > > +	/* if key is present in kvlist and does not match, filter device */
> > > +	name = rte_kvargs_get(kvlist, key);
> > > +	if (name != NULL && strcmp(name, dev->name))
> > >  		return -1;
> > >  
> > >  	return 0;
> > 
> > Nice unit test!
> > 
> > Reviewed-by: Xueming Li <xuemingl@nvidia.com>
> > 
>
diff mbox series

Patch

diff --git a/app/test/meson.build b/app/test/meson.build
index 961bebc5cb..19009da52e 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -153,6 +153,7 @@  test_sources = files(
         'test_trace.c',
         'test_trace_register.c',
         'test_trace_perf.c',
+        'test_vdev.c',
         'test_version.c',
         'virtual_pmd.c',
 )
@@ -177,6 +178,7 @@  test_deps = [
         'ipsec',
         'lpm',
         'member',
+        'net_null',
         'node',
         'pipeline',
         'port',
@@ -283,6 +285,7 @@  fast_tests = [
         ['service_autotest', true],
         ['thash_autotest', true],
         ['trace_autotest', true],
+        ['vdev_autotest', true],
 ]
 
 # Tests known to have issues or which don't belong in other tests lists.
diff --git a/app/test/test_vdev.c b/app/test/test_vdev.c
new file mode 100644
index 0000000000..720722c363
--- /dev/null
+++ b/app/test/test_vdev.c
@@ -0,0 +1,168 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 6WIND S.A.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_kvargs.h>
+#include <rte_bus_vdev.h>
+
+#include "test.h"
+
+#define TEST_VDEV_KEY_NAME "name"
+
+static const char * const valid_keys[] = {
+	TEST_VDEV_KEY_NAME,
+	NULL,
+};
+
+static int
+cmp_dev_name(const struct rte_device *dev, const void *name)
+{
+	return strcmp(dev->name, name);
+}
+
+static int
+cmp_dev_match(const struct rte_device *dev, const void *_kvlist)
+{
+	const struct rte_kvargs *kvlist = _kvlist;
+	const char *key = TEST_VDEV_KEY_NAME;
+	const char *name;
+
+	/* no kvlist arg, all devices match */
+	if (kvlist == NULL)
+		return 0;
+
+	/* if key is present in kvlist and does not match, filter device */
+	name = rte_kvargs_get(kvlist, key);
+	if (name != NULL && strcmp(name, dev->name))
+		return -1;
+
+	return 0;
+}
+
+static struct rte_device *
+get_matching_vdev(const char *match_str)
+{
+	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
+	struct rte_kvargs *kvargs = NULL;
+	struct rte_device *dev;
+
+	if (match_str != NULL) {
+		kvargs = rte_kvargs_parse(match_str, valid_keys);
+		if (kvargs == NULL) {
+			printf("Failed to parse match string\n");
+			return NULL;
+		}
+	}
+
+	dev = vdev_bus->find_device(NULL, cmp_dev_match, kvargs);
+	rte_kvargs_free(kvargs);
+
+	return dev;
+}
+
+static int
+test_vdev_bus(void)
+{
+	struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev");
+	struct rte_dev_iterator dev_iter = { 0 };
+	struct rte_device *dev, *dev0, *dev1;
+
+	/* not supported */
+	if (vdev_bus == NULL)
+		return 0;
+
+	/* create first vdev */
+	if (rte_vdev_init("net_null_test0", "") < 0) {
+		printf("Failed to create vdev net_null_test0\n");
+		goto fail;
+	}
+	dev0 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test0");
+	if (dev0 == NULL) {
+		printf("Cannot find net_null_test0 vdev\n");
+		goto fail;
+	}
+
+	/* create second vdev */
+	if (rte_vdev_init("net_null_test1", "") < 0) {
+		printf("Failed to create vdev net_null_test1\n");
+		goto fail;
+	}
+	dev1 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test1");
+	if (dev1 == NULL) {
+		printf("Cannot find net_null_test1 vdev\n");
+		goto fail;
+	}
+
+	/* try to match vdevs */
+	dev = get_matching_vdev("name=net_null_test0");
+	if (dev != dev0) {
+		printf("Cannot match net_null_test0 vdev\n");
+		goto fail;
+	}
+
+	dev = get_matching_vdev("name=net_null_test1");
+	if (dev != dev1) {
+		printf("Cannot match net_null_test1 vdev\n");
+		goto fail;
+	}
+
+	dev = get_matching_vdev("name=unexistant");
+	if (dev != NULL) {
+		printf("Unexistant vdev should not match\n");
+		goto fail;
+	}
+
+	dev = get_matching_vdev("");
+	if (dev == NULL || dev == dev1) {
+		printf("Cannot match any vdev with empty match string\n");
+		goto fail;
+	}
+
+	dev = get_matching_vdev(NULL);
+	if (dev == NULL || dev == dev1) {
+		printf("Cannot match any vdev with NULL match string\n");
+		goto fail;
+	}
+
+	/* iterate all vdevs, and ensure we find vdev0 and vdev1 */
+	RTE_DEV_FOREACH(dev, "bus=vdev", &dev_iter) {
+		if (dev == dev0)
+			dev0 = NULL;
+		else if (dev == dev1)
+			dev1 = NULL;
+	}
+	if (dev0 != NULL) {
+		printf("dev0 was not iterated\n");
+		goto fail;
+	}
+	if (dev1 != NULL) {
+		printf("dev1 was not iterated\n");
+		goto fail;
+	}
+
+	rte_vdev_uninit("net_null_test0");
+	rte_vdev_uninit("net_null_test1");
+
+	return 0;
+
+fail:
+	rte_vdev_uninit("net_null_test0");
+	rte_vdev_uninit("net_null_test1");
+	return -1;
+}
+
+static int
+test_vdev(void)
+{
+	printf("== test vdev bus ==\n");
+	if (test_vdev_bus() < 0)
+		return -1;
+	return 0;
+}
+
+REGISTER_TEST_COMMAND(vdev_autotest, test_vdev);
diff --git a/drivers/bus/auxiliary/auxiliary_params.c b/drivers/bus/auxiliary/auxiliary_params.c
index 8dd8813611..9c08ccdd1b 100644
--- a/drivers/bus/auxiliary/auxiliary_params.c
+++ b/drivers/bus/auxiliary/auxiliary_params.c
@@ -28,8 +28,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];
+	const char *name;
 
-	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
+	/* no kvlist arg, all devices match */
+	if (kvlist == NULL)
+		return 0;
+
+	/* if key is present in kvlist and does not match, filter device */
+	name = rte_kvargs_get(kvlist, key);
+	if (name != NULL && strcmp(name, dev->name))
 		return -1;
 
 	return 0;
diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
index 37d95395e7..3969faf16d 100644
--- a/drivers/bus/vdev/vdev_params.c
+++ b/drivers/bus/vdev/vdev_params.c
@@ -28,8 +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];
+	const char *name;
 
-	if (rte_kvargs_get_with_value(kvlist, key, dev->name) == NULL)
+	/* no kvlist arg, all devices match */
+	if (kvlist == NULL)
+		return 0;
+
+	/* if key is present in kvlist and does not match, filter device */
+	name = rte_kvargs_get(kvlist, key);
+	if (name != NULL && strcmp(name, dev->name))
 		return -1;
 
 	return 0;