diff mbox

[dpdk-dev,v4,18/20] ethdev: register ether layer as a class

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

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Gaëtan Rivet March 29, 2018, 9:23 p.m. UTC 
  Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/Makefile                     |  2 +-
 lib/librte_ether/Makefile        |  3 +-
 lib/librte_ether/rte_class_eth.c | 73 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_ether/rte_class_eth.c

Comments

Matan Azrad April 9, 2018, 7:41 a.m. UTC | #1 
Hi Gaetan

From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM

> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>

> ---

>  lib/Makefile                     |  2 +-

>  lib/librte_ether/Makefile        |  3 +-

>  lib/librte_ether/rte_class_eth.c | 73

> ++++++++++++++++++++++++++++++++++++++++

>  3 files changed, 76 insertions(+), 2 deletions(-)  create mode 100644

> lib/librte_ether/rte_class_eth.c

> 

> diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f 100644

> --- a/lib/Makefile

> +++ b/lib/Makefile

> @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=

> librte_cmdline  DEPDIRS-librte_cmdline := librte_eal

>  DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether  DEPDIRS-librte_ether

> := librte_net librte_eal librte_mempool librte_ring -DEPDIRS-librte_ether +=

> librte_mbuf

> +DEPDIRS-librte_ether += librte_mbuf librte_kvargs

>  DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev  DEPDIRS-

> librte_bbdev := librte_eal librte_mempool librte_mbuf

>  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git

> a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index

> 3ca5782bb..a1a0393de 100644

> --- a/lib/librte_ether/Makefile

> +++ b/lib/librte_ether/Makefile

> @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API  CFLAGS

> += -O3  CFLAGS += $(WERROR_FLAGS)  LDLIBS += -lrte_net -lrte_eal -

> lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf

> +LDLIBS += -lrte_mbuf -lrte_kvargs

> 

>  EXPORT_MAP := rte_ethdev_version.map

> 

>  LIBABIVER := 8

> 

>  SRCS-y += rte_ethdev.c

> +SRCS-y += rte_class_eth.c

>  SRCS-y += rte_flow.c

>  SRCS-y += rte_tm.c

>  SRCS-y += rte_mtr.c

> diff --git a/lib/librte_ether/rte_class_eth.c b/lib/librte_ether/rte_class_eth.c

> new file mode 100644

> index 000000000..97d24781d

> --- /dev/null

> +++ b/lib/librte_ether/rte_class_eth.c

> @@ -0,0 +1,73 @@

> +/* SPDX-License-Identifier: BSD-3-Clause

> + * Copyright(c) 2018 Gaëtan Rivet

> + */

> +

> +#include <string.h>

> +

> +#include <rte_class.h>

> +#include <rte_compat.h>

> +#include <rte_errno.h>

> +#include <rte_kvargs.h>

> +#include <rte_log.h>

> +

> +#include "rte_ethdev.h"

> +#include "rte_ethdev_core.h"

> +

> +enum eth_params {

> +	RTE_ETH_PARAMS_MAX,

> +};

> +

> +static const char *eth_params_keys[] = {

> +	[RTE_ETH_PARAMS_MAX] = NULL,

> +};

> +

> +static int

> +eth_dev_match(struct rte_eth_dev *edev,

> +	      struct rte_kvargs *kvlist)

> +{

> +	(void) kvlist;

> +	(void) edev;

> +	return 0;

> +}

> +

> +static void *

> +eth_dev_iterate(const void *_start,

> +		const char *str,

> +		const struct rte_dev_iterator *it)

> +{

> +	const struct rte_eth_dev *start = _start;

> +	struct rte_device *dev = it->device;

> +	struct rte_kvargs *kvargs = NULL;

> +	struct rte_eth_dev *edev = NULL;

> +	uint16_t p = 0;

> +

> +	if (str != NULL) {

> +		kvargs = rte_kvargs_parse(str, eth_params_keys);

> +		if (kvargs == NULL) {

> +			RTE_LOG(ERR, EAL, "cannot parse argument list\n");

> +			rte_errno = EINVAL;

> +			return NULL;

> +		}

> +	}

> +	if (start)

> +		p = start->data->port_id + 1;

> +	for (p = rte_eth_find_next(p);

> +	     p < RTE_MAX_ETHPORTS;

> +	     p = rte_eth_find_next(p + 1)) {


What are about differed\owned\bonded devices?
What is actually the use cases of this iterator? 
DPDK internal management or applications?

> +		edev = &rte_eth_devices[p];

> +		if (dev != edev->device)

> +			goto next_ethdev;

> +		if (eth_dev_match(edev, kvargs) == 0)

> +			break;

> +next_ethdev:

> +		edev = NULL;

> +	}

> +	rte_kvargs_free(kvargs);

> +	return edev;

> +}

> +

> +struct rte_class rte_class_eth = {

> +	.dev_iterate = eth_dev_iterate,

> +};

> +

> +RTE_REGISTER_CLASS(eth, rte_class_eth);

> --

> 2.11.0
Gaëtan Rivet April 9, 2018, 7:47 a.m. UTC | #2 
Hi Matan,

On Mon, Apr 09, 2018 at 07:41:58AM +0000, Matan Azrad wrote:
> Hi Gaetan
> 
> From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >  lib/Makefile                     |  2 +-
> >  lib/librte_ether/Makefile        |  3 +-
> >  lib/librte_ether/rte_class_eth.c | 73
> > ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 76 insertions(+), 2 deletions(-)  create mode 100644
> > lib/librte_ether/rte_class_eth.c
> > 
> > diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=
> > librte_cmdline  DEPDIRS-librte_cmdline := librte_eal
> >  DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether  DEPDIRS-librte_ether
> > := librte_net librte_eal librte_mempool librte_ring -DEPDIRS-librte_ether +=
> > librte_mbuf
> > +DEPDIRS-librte_ether += librte_mbuf librte_kvargs
> >  DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev  DEPDIRS-
> > librte_bbdev := librte_eal librte_mempool librte_mbuf
> >  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git
> > a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index
> > 3ca5782bb..a1a0393de 100644
> > --- a/lib/librte_ether/Makefile
> > +++ b/lib/librte_ether/Makefile
> > @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API  CFLAGS
> > += -O3  CFLAGS += $(WERROR_FLAGS)  LDLIBS += -lrte_net -lrte_eal -
> > lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf
> > +LDLIBS += -lrte_mbuf -lrte_kvargs
> > 
> >  EXPORT_MAP := rte_ethdev_version.map
> > 
> >  LIBABIVER := 8
> > 
> >  SRCS-y += rte_ethdev.c
> > +SRCS-y += rte_class_eth.c
> >  SRCS-y += rte_flow.c
> >  SRCS-y += rte_tm.c
> >  SRCS-y += rte_mtr.c
> > diff --git a/lib/librte_ether/rte_class_eth.c b/lib/librte_ether/rte_class_eth.c
> > new file mode 100644
> > index 000000000..97d24781d
> > --- /dev/null
> > +++ b/lib/librte_ether/rte_class_eth.c
> > @@ -0,0 +1,73 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2018 Gaëtan Rivet
> > + */
> > +
> > +#include <string.h>
> > +
> > +#include <rte_class.h>
> > +#include <rte_compat.h>
> > +#include <rte_errno.h>
> > +#include <rte_kvargs.h>
> > +#include <rte_log.h>
> > +
> > +#include "rte_ethdev.h"
> > +#include "rte_ethdev_core.h"
> > +
> > +enum eth_params {
> > +	RTE_ETH_PARAMS_MAX,
> > +};
> > +
> > +static const char *eth_params_keys[] = {
> > +	[RTE_ETH_PARAMS_MAX] = NULL,
> > +};
> > +
> > +static int
> > +eth_dev_match(struct rte_eth_dev *edev,
> > +	      struct rte_kvargs *kvlist)
> > +{
> > +	(void) kvlist;
> > +	(void) edev;
> > +	return 0;
> > +}
> > +
> > +static void *
> > +eth_dev_iterate(const void *_start,
> > +		const char *str,
> > +		const struct rte_dev_iterator *it)
> > +{
> > +	const struct rte_eth_dev *start = _start;
> > +	struct rte_device *dev = it->device;
> > +	struct rte_kvargs *kvargs = NULL;
> > +	struct rte_eth_dev *edev = NULL;
> > +	uint16_t p = 0;
> > +
> > +	if (str != NULL) {
> > +		kvargs = rte_kvargs_parse(str, eth_params_keys);
> > +		if (kvargs == NULL) {
> > +			RTE_LOG(ERR, EAL, "cannot parse argument list\n");
> > +			rte_errno = EINVAL;
> > +			return NULL;
> > +		}
> > +	}
> > +	if (start)
> > +		p = start->data->port_id + 1;
> > +	for (p = rte_eth_find_next(p);
> > +	     p < RTE_MAX_ETHPORTS;
> > +	     p = rte_eth_find_next(p + 1)) {
> 
> What are about differed\owned\bonded devices?
> What is actually the use cases of this iterator? 
> DPDK internal management or applications?
> 

This API is public, so anyone can use it.
It should be primarily used by internal systems however.
I do not think Applications would be interested in calling the
dev_iterate ops.

The EAL would provide a way to list interfaces from each layers. Then
DPDK libraries (such as librte_ether) would translate this following
their internal rules. librte_ether would thus for example here translate
an rte_eth_dev handle to a port_id, that an application could use. If
there was a need to filter by owner_id, then it would be the job of
librte_ether.

The EAL cannot keep track of layers specifics, it can only provide
generic APIs.

> > +		edev = &rte_eth_devices[p];
> > +		if (dev != edev->device)
> > +			goto next_ethdev;
> > +		if (eth_dev_match(edev, kvargs) == 0)
> > +			break;
> > +next_ethdev:
> > +		edev = NULL;
> > +	}
> > +	rte_kvargs_free(kvargs);
> > +	return edev;
> > +}
> > +
> > +struct rte_class rte_class_eth = {
> > +	.dev_iterate = eth_dev_iterate,
> > +};
> > +
> > +RTE_REGISTER_CLASS(eth, rte_class_eth);
> > --
> > 2.11.0
>
Matan Azrad April 9, 2018, 7:58 a.m. UTC | #3 
Hi Gaetan

From: Gaëtan Rivet, Monday, April 9, 2018 10:47 AM
> Hi Matan,
> 
> On Mon, Apr 09, 2018 at 07:41:58AM +0000, Matan Azrad wrote:
> > Hi Gaetan
> >
> > From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > ---
> > >  lib/Makefile                     |  2 +-
> > >  lib/librte_ether/Makefile        |  3 +-
> > >  lib/librte_ether/rte_class_eth.c | 73
> > > ++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 76 insertions(+), 2 deletions(-)  create mode
> > > 100644 lib/librte_ether/rte_class_eth.c
> > >
> > > diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f
> > > 100644
> > > --- a/lib/Makefile
> > > +++ b/lib/Makefile
> > > @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=
> > > librte_cmdline  DEPDIRS-librte_cmdline := librte_eal
> > >  DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
> > > DEPDIRS-librte_ether := librte_net librte_eal librte_mempool
> > > librte_ring -DEPDIRS-librte_ether += librte_mbuf
> > > +DEPDIRS-librte_ether += librte_mbuf librte_kvargs
> > >  DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev  DEPDIRS-
> > > librte_bbdev := librte_eal librte_mempool librte_mbuf
> > >  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git
> > > a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index
> > > 3ca5782bb..a1a0393de 100644
> > > --- a/lib/librte_ether/Makefile
> > > +++ b/lib/librte_ether/Makefile
> > > @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
> CFLAGS
> > > += -O3  CFLAGS += $(WERROR_FLAGS)  LDLIBS += -lrte_net -lrte_eal -
> > > lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf
> > > +LDLIBS += -lrte_mbuf -lrte_kvargs
> > >
> > >  EXPORT_MAP := rte_ethdev_version.map
> > >
> > >  LIBABIVER := 8
> > >
> > >  SRCS-y += rte_ethdev.c
> > > +SRCS-y += rte_class_eth.c
> > >  SRCS-y += rte_flow.c
> > >  SRCS-y += rte_tm.c
> > >  SRCS-y += rte_mtr.c
> > > diff --git a/lib/librte_ether/rte_class_eth.c
> > > b/lib/librte_ether/rte_class_eth.c
> > > new file mode 100644
> > > index 000000000..97d24781d
> > > --- /dev/null
> > > +++ b/lib/librte_ether/rte_class_eth.c
> > > @@ -0,0 +1,73 @@
> > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > + * Copyright(c) 2018 Gaëtan Rivet
> > > + */
> > > +
> > > +#include <string.h>
> > > +
> > > +#include <rte_class.h>
> > > +#include <rte_compat.h>
> > > +#include <rte_errno.h>
> > > +#include <rte_kvargs.h>
> > > +#include <rte_log.h>
> > > +
> > > +#include "rte_ethdev.h"
> > > +#include "rte_ethdev_core.h"
> > > +
> > > +enum eth_params {
> > > +	RTE_ETH_PARAMS_MAX,
> > > +};
> > > +
> > > +static const char *eth_params_keys[] = {
> > > +	[RTE_ETH_PARAMS_MAX] = NULL,
> > > +};
> > > +
> > > +static int
> > > +eth_dev_match(struct rte_eth_dev *edev,
> > > +	      struct rte_kvargs *kvlist)
> > > +{
> > > +	(void) kvlist;
> > > +	(void) edev;
> > > +	return 0;
> > > +}
> > > +
> > > +static void *
> > > +eth_dev_iterate(const void *_start,
> > > +		const char *str,
> > > +		const struct rte_dev_iterator *it) {
> > > +	const struct rte_eth_dev *start = _start;
> > > +	struct rte_device *dev = it->device;
> > > +	struct rte_kvargs *kvargs = NULL;
> > > +	struct rte_eth_dev *edev = NULL;
> > > +	uint16_t p = 0;
> > > +
> > > +	if (str != NULL) {
> > > +		kvargs = rte_kvargs_parse(str, eth_params_keys);
> > > +		if (kvargs == NULL) {
> > > +			RTE_LOG(ERR, EAL, "cannot parse argument list\n");
> > > +			rte_errno = EINVAL;
> > > +			return NULL;
> > > +		}
> > > +	}
> > > +	if (start)
> > > +		p = start->data->port_id + 1;
> > > +	for (p = rte_eth_find_next(p);
> > > +	     p < RTE_MAX_ETHPORTS;
> > > +	     p = rte_eth_find_next(p + 1)) {
> >
> > What are about differed\owned\bonded devices?
> > What is actually the use cases of this iterator?
> > DPDK internal management or applications?
> >
> 
> This API is public, so anyone can use it.
> It should be primarily used by internal systems however.
> I do not think Applications would be interested in calling the dev_iterate ops.
> 
> The EAL would provide a way to list interfaces from each layers. Then DPDK
> libraries (such as librte_ether) would translate this following their internal
> rules. librte_ether would thus for example here translate an rte_eth_dev
> handle to a port_id, that an application could use. If there was a need to filter
> by owner_id, then it would be the job of librte_ether.
> 
> The EAL cannot keep track of layers specifics, it can only provide generic APIs.

This is exactly what I'm asking, what is the right behavior here?

Now, The code is skipping DEFFERED devices but iterating over owned\bonded devices.
Looks like any USED port should be iterated...

Moreover, looks like there is chance to iterate over EAL device more than 1 time for loop, Is it OK?

> > > +		edev = &rte_eth_devices[p];
> > > +		if (dev != edev->device)
> > > +			goto next_ethdev;
> > > +		if (eth_dev_match(edev, kvargs) == 0)
> > > +			break;
> > > +next_ethdev:
> > > +		edev = NULL;
> > > +	}
> > > +	rte_kvargs_free(kvargs);
> > > +	return edev;
> > > +}
> > > +
> > > +struct rte_class rte_class_eth = {
> > > +	.dev_iterate = eth_dev_iterate,
> > > +};
> > > +
> > > +RTE_REGISTER_CLASS(eth, rte_class_eth);
> > > --
> > > 2.11.0
> >
> 
> --
> Gaëtan Rivet
> 6WIND
Gaëtan Rivet April 9, 2018, 8:12 a.m. UTC | #4 
On Mon, Apr 09, 2018 at 07:58:08AM +0000, Matan Azrad wrote:
> Hi Gaetan
> 
> From: Gaëtan Rivet, Monday, April 9, 2018 10:47 AM
> > Hi Matan,
> > 
> > On Mon, Apr 09, 2018 at 07:41:58AM +0000, Matan Azrad wrote:
> > > Hi Gaetan
> > >
> > > From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> > > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > > ---
> > > >  lib/Makefile                     |  2 +-
> > > >  lib/librte_ether/Makefile        |  3 +-
> > > >  lib/librte_ether/rte_class_eth.c | 73
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 76 insertions(+), 2 deletions(-)  create mode
> > > > 100644 lib/librte_ether/rte_class_eth.c
> > > >
> > > > diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f
> > > > 100644
> > > > --- a/lib/Makefile
> > > > +++ b/lib/Makefile
> > > > @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=
> > > > librte_cmdline  DEPDIRS-librte_cmdline := librte_eal
> > > >  DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
> > > > DEPDIRS-librte_ether := librte_net librte_eal librte_mempool
> > > > librte_ring -DEPDIRS-librte_ether += librte_mbuf
> > > > +DEPDIRS-librte_ether += librte_mbuf librte_kvargs
> > > >  DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev  DEPDIRS-
> > > > librte_bbdev := librte_eal librte_mempool librte_mbuf
> > > >  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git
> > > > a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index
> > > > 3ca5782bb..a1a0393de 100644
> > > > --- a/lib/librte_ether/Makefile
> > > > +++ b/lib/librte_ether/Makefile
> > > > @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
> > CFLAGS
> > > > += -O3  CFLAGS += $(WERROR_FLAGS)  LDLIBS += -lrte_net -lrte_eal -
> > > > lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf
> > > > +LDLIBS += -lrte_mbuf -lrte_kvargs
> > > >
> > > >  EXPORT_MAP := rte_ethdev_version.map
> > > >
> > > >  LIBABIVER := 8
> > > >
> > > >  SRCS-y += rte_ethdev.c
> > > > +SRCS-y += rte_class_eth.c
> > > >  SRCS-y += rte_flow.c
> > > >  SRCS-y += rte_tm.c
> > > >  SRCS-y += rte_mtr.c
> > > > diff --git a/lib/librte_ether/rte_class_eth.c
> > > > b/lib/librte_ether/rte_class_eth.c
> > > > new file mode 100644
> > > > index 000000000..97d24781d
> > > > --- /dev/null
> > > > +++ b/lib/librte_ether/rte_class_eth.c
> > > > @@ -0,0 +1,73 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > + * Copyright(c) 2018 Gaëtan Rivet
> > > > + */
> > > > +
> > > > +#include <string.h>
> > > > +
> > > > +#include <rte_class.h>
> > > > +#include <rte_compat.h>
> > > > +#include <rte_errno.h>
> > > > +#include <rte_kvargs.h>
> > > > +#include <rte_log.h>
> > > > +
> > > > +#include "rte_ethdev.h"
> > > > +#include "rte_ethdev_core.h"
> > > > +
> > > > +enum eth_params {
> > > > +	RTE_ETH_PARAMS_MAX,
> > > > +};
> > > > +
> > > > +static const char *eth_params_keys[] = {
> > > > +	[RTE_ETH_PARAMS_MAX] = NULL,
> > > > +};
> > > > +
> > > > +static int
> > > > +eth_dev_match(struct rte_eth_dev *edev,
> > > > +	      struct rte_kvargs *kvlist)
> > > > +{
> > > > +	(void) kvlist;
> > > > +	(void) edev;
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static void *
> > > > +eth_dev_iterate(const void *_start,
> > > > +		const char *str,
> > > > +		const struct rte_dev_iterator *it) {
> > > > +	const struct rte_eth_dev *start = _start;
> > > > +	struct rte_device *dev = it->device;
> > > > +	struct rte_kvargs *kvargs = NULL;
> > > > +	struct rte_eth_dev *edev = NULL;
> > > > +	uint16_t p = 0;
> > > > +
> > > > +	if (str != NULL) {
> > > > +		kvargs = rte_kvargs_parse(str, eth_params_keys);
> > > > +		if (kvargs == NULL) {
> > > > +			RTE_LOG(ERR, EAL, "cannot parse argument list\n");
> > > > +			rte_errno = EINVAL;
> > > > +			return NULL;
> > > > +		}
> > > > +	}
> > > > +	if (start)
> > > > +		p = start->data->port_id + 1;
> > > > +	for (p = rte_eth_find_next(p);
> > > > +	     p < RTE_MAX_ETHPORTS;
> > > > +	     p = rte_eth_find_next(p + 1)) {
> > >
> > > What are about differed\owned\bonded devices?
> > > What is actually the use cases of this iterator?
> > > DPDK internal management or applications?
> > >
> > 
> > This API is public, so anyone can use it.
> > It should be primarily used by internal systems however.
> > I do not think Applications would be interested in calling the dev_iterate ops.
> > 
> > The EAL would provide a way to list interfaces from each layers. Then DPDK
> > libraries (such as librte_ether) would translate this following their internal
> > rules. librte_ether would thus for example here translate an rte_eth_dev
> > handle to a port_id, that an application could use. If there was a need to filter
> > by owner_id, then it would be the job of librte_ether.
> > 
> > The EAL cannot keep track of layers specifics, it can only provide generic APIs.
> 
> This is exactly what I'm asking, what is the right behavior here?
> 

From the EAL PoV: The layer should be perfectly transparent: all
internal objects should be returned.

> Now, The code is skipping DEFFERED devices but iterating over owned\bonded devices.
> Looks like any USED port should be iterated...
> 

Right, actually DEFERRED devices should probably be returned as well,
good catch.

> Moreover, looks like there is chance to iterate over EAL device more than 1 time for loop, Is it OK?
> 

If by EAL device you mean rte_device, then yes, this is OK.
Due to mlx4 PMD, it is now possible for PMDs to spawn several
rte_eth_dev per rte_pci_device. A single rte_pci_device is a
specialization of an rte_device. So an rte_device can be shared by
several rte_eth_dev, and would be returned several times if need be.

Each time however, the rte_eth_dev layer should iterate over its
internal object, meaning that even if the rte_device is the same, the
overall context would still change.

> > > > +		edev = &rte_eth_devices[p];
> > > > +		if (dev != edev->device)
> > > > +			goto next_ethdev;
> > > > +		if (eth_dev_match(edev, kvargs) == 0)
> > > > +			break;
> > > > +next_ethdev:
> > > > +		edev = NULL;
> > > > +	}
> > > > +	rte_kvargs_free(kvargs);
> > > > +	return edev;
> > > > +}
> > > > +
> > > > +struct rte_class rte_class_eth = {
> > > > +	.dev_iterate = eth_dev_iterate,
> > > > +};
> > > > +
> > > > +RTE_REGISTER_CLASS(eth, rte_class_eth);
> > > > --
> > > > 2.11.0
> > >
> > 
> > --
> > Gaëtan Rivet
> > 6WIND
diff mbox

Patch

diff --git a/lib/Makefile b/lib/Makefile
index 4206485d3..47513f03f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -23,7 +23,7 @@  DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += librte_cmdline
 DEPDIRS-librte_cmdline := librte_eal
 DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
 DEPDIRS-librte_ether := librte_net librte_eal librte_mempool librte_ring
-DEPDIRS-librte_ether += librte_mbuf
+DEPDIRS-librte_ether += librte_mbuf librte_kvargs
 DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev
 DEPDIRS-librte_bbdev := librte_eal librte_mempool librte_mbuf
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev
diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
index 3ca5782bb..a1a0393de 100644
--- a/lib/librte_ether/Makefile
+++ b/lib/librte_ether/Makefile
@@ -12,13 +12,14 @@  CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 LDLIBS += -lrte_net -lrte_eal -lrte_mempool -lrte_ring
-LDLIBS += -lrte_mbuf
+LDLIBS += -lrte_mbuf -lrte_kvargs
 
 EXPORT_MAP := rte_ethdev_version.map
 
 LIBABIVER := 8
 
 SRCS-y += rte_ethdev.c
+SRCS-y += rte_class_eth.c
 SRCS-y += rte_flow.c
 SRCS-y += rte_tm.c
 SRCS-y += rte_mtr.c
diff --git a/lib/librte_ether/rte_class_eth.c b/lib/librte_ether/rte_class_eth.c
new file mode 100644
index 000000000..97d24781d
--- /dev/null
+++ b/lib/librte_ether/rte_class_eth.c
@@ -0,0 +1,73 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Gaëtan Rivet
+ */
+
+#include <string.h>
+
+#include <rte_class.h>
+#include <rte_compat.h>
+#include <rte_errno.h>
+#include <rte_kvargs.h>
+#include <rte_log.h>
+
+#include "rte_ethdev.h"
+#include "rte_ethdev_core.h"
+
+enum eth_params {
+	RTE_ETH_PARAMS_MAX,
+};
+
+static const char *eth_params_keys[] = {
+	[RTE_ETH_PARAMS_MAX] = NULL,
+};
+
+static int
+eth_dev_match(struct rte_eth_dev *edev,
+	      struct rte_kvargs *kvlist)
+{
+	(void) kvlist;
+	(void) edev;
+	return 0;
+}
+
+static void *
+eth_dev_iterate(const void *_start,
+		const char *str,
+		const struct rte_dev_iterator *it)
+{
+	const struct rte_eth_dev *start = _start;
+	struct rte_device *dev = it->device;
+	struct rte_kvargs *kvargs = NULL;
+	struct rte_eth_dev *edev = NULL;
+	uint16_t p = 0;
+
+	if (str != NULL) {
+		kvargs = rte_kvargs_parse(str, eth_params_keys);
+		if (kvargs == NULL) {
+			RTE_LOG(ERR, EAL, "cannot parse argument list\n");
+			rte_errno = EINVAL;
+			return NULL;
+		}
+	}
+	if (start)
+		p = start->data->port_id + 1;
+	for (p = rte_eth_find_next(p);
+	     p < RTE_MAX_ETHPORTS;
+	     p = rte_eth_find_next(p + 1)) {
+		edev = &rte_eth_devices[p];
+		if (dev != edev->device)
+			goto next_ethdev;
+		if (eth_dev_match(edev, kvargs) == 0)
+			break;
+next_ethdev:
+		edev = NULL;
+	}
+	rte_kvargs_free(kvargs);
+	return edev;
+}
+
+struct rte_class rte_class_eth = {
+	.dev_iterate = eth_dev_iterate,
+};
+
+RTE_REGISTER_CLASS(eth, rte_class_eth);