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.ri...@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

Reply via email to