[1/5] bus/vdev: add iteration filter on name
Checks
Commit Message
A virtual device can be matched with following syntax:
bus=vdev,name=X
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
drivers/bus/vdev/vdev_params.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
Comments
On 10/8/18 1:25 AM, Thomas Monjalon wrote:
> A virtual device can be matched with following syntax:
> bus=vdev,name=X
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> drivers/bus/vdev/vdev_params.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
> index da270f2ec..133998c3e 100644
> --- a/drivers/bus/vdev/vdev_params.c
> +++ b/drivers/bus/vdev/vdev_params.c
> @@ -2,6 +2,8 @@
> * Copyright 2018 Gaëtan Rivet
> */
>
> +#include <string.h>
> +
> #include <rte_dev.h>
> #include <rte_bus.h>
> #include <rte_kvargs.h>
> @@ -11,10 +13,12 @@
> #include "vdev_private.h"
>
> enum vdev_params {
> + RTE_VDEV_PARAM_NAME,
> RTE_VDEV_PARAM_MAX,
> };
>
> static const char * const vdev_params_keys[] = {
> + [RTE_VDEV_PARAM_NAME] = "name",
> [RTE_VDEV_PARAM_MAX] = NULL,
> };
>
> @@ -22,11 +26,22 @@ static int
> vdev_dev_match(const struct rte_device *dev,
> const void *_kvlist)
> {
> + int ret;
> const struct rte_kvargs *kvlist = _kvlist;
> + char *name;
> +
> + /* cannot pass const dev->name to rte_kvargs_process() */
> + name = strdup(dev->name);
> + if (name == NULL)
> + return -ENOMEM; /* interpreted as no match */
It is strange to see -ENOMEM and -1 returned from the same function.
rte_dev_cmp_t does not return negative errno. It just says match /
no match (greater / smaller than 0 if ordering is possible).
So, -ENOMEM is really confusing here. I think just -1 should be used.
> + ret = rte_kvargs_process(kvlist,
> + vdev_params_keys[RTE_VDEV_PARAM_NAME],
> + rte_kvargs_strcmp, name);
> + free(name);
> + if (ret != 0)
> + return -1;
>
> - (void) kvlist;
> - (void) dev;
> - return 0;
> + return ret;
I'm not sure that I understand why 'ret' is returned here
instead of 0. Above check guarantees that ret==0.
If you change it, it should be a good reason.
> }
>
> void *
08/10/2018 08:46, Andrew Rybchenko:
> On 10/8/18 1:25 AM, Thomas Monjalon wrote:
> > A virtual device can be matched with following syntax:
> > bus=vdev,name=X
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > drivers/bus/vdev/vdev_params.c | 21 ++++++++++++++++++---
> > 1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
> > index da270f2ec..133998c3e 100644
> > --- a/drivers/bus/vdev/vdev_params.c
> > +++ b/drivers/bus/vdev/vdev_params.c
> > @@ -2,6 +2,8 @@
> > * Copyright 2018 Gaëtan Rivet
> > */
> >
> > +#include <string.h>
> > +
> > #include <rte_dev.h>
> > #include <rte_bus.h>
> > #include <rte_kvargs.h>
> > @@ -11,10 +13,12 @@
> > #include "vdev_private.h"
> >
> > enum vdev_params {
> > + RTE_VDEV_PARAM_NAME,
> > RTE_VDEV_PARAM_MAX,
> > };
> >
> > static const char * const vdev_params_keys[] = {
> > + [RTE_VDEV_PARAM_NAME] = "name",
> > [RTE_VDEV_PARAM_MAX] = NULL,
> > };
> >
> > @@ -22,11 +26,22 @@ static int
> > vdev_dev_match(const struct rte_device *dev,
> > const void *_kvlist)
> > {
> > + int ret;
> > const struct rte_kvargs *kvlist = _kvlist;
> > + char *name;
> > +
> > + /* cannot pass const dev->name to rte_kvargs_process() */
> > + name = strdup(dev->name);
> > + if (name == NULL)
> > + return -ENOMEM; /* interpreted as no match */
>
> It is strange to see -ENOMEM and -1 returned from the same function.
> rte_dev_cmp_t does not return negative errno. It just says match /
> no match (greater / smaller than 0 if ordering is possible).
> So, -ENOMEM is really confusing here. I think just -1 should be used.
Yes, OK.
> > + ret = rte_kvargs_process(kvlist,
> > + vdev_params_keys[RTE_VDEV_PARAM_NAME],
> > + rte_kvargs_strcmp, name);
> > + free(name);
> > + if (ret != 0)
> > + return -1;
> >
> > - (void) kvlist;
> > - (void) dev;
> > - return 0;
> > + return ret;
>
> I'm not sure that I understand why 'ret' is returned here
> instead of 0. Above check guarantees that ret==0.
> If you change it, it should be a good reason.
Right
Thanks for the review!
@@ -2,6 +2,8 @@
* Copyright 2018 Gaëtan Rivet
*/
+#include <string.h>
+
#include <rte_dev.h>
#include <rte_bus.h>
#include <rte_kvargs.h>
@@ -11,10 +13,12 @@
#include "vdev_private.h"
enum vdev_params {
+ RTE_VDEV_PARAM_NAME,
RTE_VDEV_PARAM_MAX,
};
static const char * const vdev_params_keys[] = {
+ [RTE_VDEV_PARAM_NAME] = "name",
[RTE_VDEV_PARAM_MAX] = NULL,
};
@@ -22,11 +26,22 @@ static int
vdev_dev_match(const struct rte_device *dev,
const void *_kvlist)
{
+ int ret;
const struct rte_kvargs *kvlist = _kvlist;
+ char *name;
+
+ /* cannot pass const dev->name to rte_kvargs_process() */
+ name = strdup(dev->name);
+ if (name == NULL)
+ return -ENOMEM; /* interpreted as no match */
+ ret = rte_kvargs_process(kvlist,
+ vdev_params_keys[RTE_VDEV_PARAM_NAME],
+ rte_kvargs_strcmp, name);
+ free(name);
+ if (ret != 0)
+ return -1;
- (void) kvlist;
- (void) dev;
- return 0;
+ return ret;
}
void *