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

Gaëtan Rivet gaetan.rivet at 6wind.com
Mon Apr 9 10:12:11 CEST 2018


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 at 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

-- 
Gaëtan Rivet
6WIND


More information about the dev mailing list