From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Rybchenko Subject: Re: [PATCH v1 05/13] ethdev: add private generic device iterator Date: Fri, 31 Aug 2018 13:09:34 +0300 Message-ID: <378e2116-4682-1ff8-8dba-1562bc1e2530@solarflare.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit To: Gaetan Rivet , Return-path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id EF6312BF7 for ; Fri, 31 Aug 2018 12:09:41 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 08/30/2018 04:41 PM, Gaetan Rivet wrote: > This iterator can be customized with a comparison function that will > trigger a stopping condition. > > It can be leveraged to write several different iterators that have > similar but non-identical purposes. > > It is private to librte_ethdev. > > Signed-off-by: Gaetan Rivet > --- > lib/librte_ethdev/Makefile | 1 + > lib/librte_ethdev/eth_private.c | 31 +++++++++++++++++++++++++++++++ > lib/librte_ethdev/eth_private.h | 26 ++++++++++++++++++++++++++ > lib/librte_ethdev/meson.build | 3 ++- > 4 files changed, 60 insertions(+), 1 deletion(-) > create mode 100644 lib/librte_ethdev/eth_private.c > create mode 100644 lib/librte_ethdev/eth_private.h > > diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile > index 0935a275e..3c1c92cb9 100644 > --- a/lib/librte_ethdev/Makefile > +++ b/lib/librte_ethdev/Makefile > @@ -18,6 +18,7 @@ EXPORT_MAP := rte_ethdev_version.map > > LIBABIVER := 10 > > +SRCS-y += eth_private.c > SRCS-y += rte_ethdev.c > SRCS-y += rte_flow.c > SRCS-y += rte_tm.c > diff --git a/lib/librte_ethdev/eth_private.c b/lib/librte_ethdev/eth_private.c > new file mode 100644 > index 000000000..d565568a0 > --- /dev/null > +++ b/lib/librte_ethdev/eth_private.c Just a nit I think it is better to name it ethdev_private.c since we already have ethdev_profile.[ch] and it is about ethdev, not Ethernet. > @@ -0,0 +1,31 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2018 Gaëtan Rivet > + */ > + > +#include "rte_ethdev.h" > +#include "eth_private.h" > + > +struct rte_eth_dev * > +eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp, > + const void *data) > +{ > + struct rte_eth_dev *edev; > + ptrdiff_t idx; > + > + /* Avoid Undefined Behaviour */ > + if (start != NULL && > + (start < &rte_eth_devices[0] || > + start > &rte_eth_devices[RTE_MAX_ETHPORTS])) > + return NULL; > + if (start != NULL) > + idx = start - &rte_eth_devices[0] + 1; > + else > + idx = 0; > + for (; idx < RTE_MAX_ETHPORTS; idx++) { > + edev = &rte_eth_devices[idx]; Shouldn't we limit it to valid ports only? If no, I think it would be useful to highlight it in the function description that it iterates over all devices including unused. > + if (cmp(edev, data) == 0) > + return edev; > + } > + return NULL; > +} > + > diff --git a/lib/librte_ethdev/eth_private.h b/lib/librte_ethdev/eth_private.h > new file mode 100644 > index 000000000..0f5c6d5c4 > --- /dev/null > +++ b/lib/librte_ethdev/eth_private.h ethdev_private.h <...>