From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by dpdk.org (Postfix) with ESMTP id 4F9EF2BA3 for ; Fri, 31 Aug 2018 12:22:29 +0200 (CEST) Received: by mail-wm0-f66.google.com with SMTP id j192-v6so4848632wmj.1 for ; Fri, 31 Aug 2018 03:22:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=XddZM4KKPt5YiVCEsUVV6FAOmm03qi1eycrs0R2qfMQ=; b=Vx4SDerwWvpeFQfRw5aGk9KB58UFdytg1pErV9nt2Eqw9h4wQhmMtJnimgZgq5zZT+ r1pLKdscF9OH5KzjC/FOVptxNfXQKJSkOqnpgu8zIKkdv6/97CWVbxEFA5rXa/nq+5q8 D/p62vY2YWudcLpmQ9tJCWytHFDhKZ3ST9xXx2xF62DcoQ8pS4OrueU/5Jxo/Yyy5jua 6WaFXgsc/g2dWU+8fxENTrjJK2fsNttYQWPv6AFpn/k7HvUS4dGPMDejCHATVcJnez0G 2bD2pup7ZNcg/M2WiAdgze5ISm8hO+QrtFV69S0O1+pgPuo408jF1pXVwFl2LuswctVY DOzw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=XddZM4KKPt5YiVCEsUVV6FAOmm03qi1eycrs0R2qfMQ=; b=FzsoG74M5f3Vx5jgup+tFUnfSGEe3FwWULyKHCZ/WdWNOeGq8hD/gNNxYwlPoMbVu+ 58Mr47FLKWz8czzy+yam0KozBroFt3yx7GAW2EzZSfJfobRtumDz3Ms34Egbfk/9+J7f EslN21WaNv0rIfFy0LFUDd8GP17fFljHuoiAjaTY76Ww9UR8kDvL6WEIq9pSHPdW4/iX f1Mn3ynUWxG+ypQLZHNIiwozcpporB3cH9cQlGosV3lBmKSt2KOYSzSjnRGOjWybHhg+ x5kTvcMVVbQU7ZdheGWP6TJcmjGMudXD5XokgMujRRwQ0cUrTw41zr7OWRTqvLcgl3lu UJ5A== X-Gm-Message-State: APzg51COxkLq+R3/RuAbniC1rk+vns9Q3UFX7FG7SUQ2MCKspywKTim6 QfjPaXpylALnd0laZZt6Wj6uxQ== X-Google-Smtp-Source: ANB0VdZ0u1n4rBQidtKrSQOExd9Pe95H2V+zIvTKdtvIhL3uRxIsMjiSQfQZTaplHGwIIVquBcM94Q== X-Received: by 2002:a1c:91cd:: with SMTP id t196-v6mr3550077wmd.100.1535710948733; Fri, 31 Aug 2018 03:22:28 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a9-v6sm4687019wmf.28.2018.08.31.03.22.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 31 Aug 2018 03:22:27 -0700 (PDT) Date: Fri, 31 Aug 2018 12:22:11 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Andrew Rybchenko Cc: dev@dpdk.org Message-ID: <20180831102211.pazx7fd3rvikmohi@bidouze.vm.6wind.com> References: <378e2116-4682-1ff8-8dba-1562bc1e2530@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <378e2116-4682-1ff8-8dba-1562bc1e2530@solarflare.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v1 05/13] ethdev: add private generic device iterator X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Aug 2018 10:22:29 -0000 On Fri, Aug 31, 2018 at 01:09:34PM +0300, Andrew Rybchenko wrote: > 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. > Agreed. > > @@ -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. > I'm undecided about it, I wanted eth_find_device to be and internal operator in the most generic way, allowing ethdev layer to move from manually iterating to using this, to introduce the probable future evolution of not using an array of rte_eth_devices. So I thought of this operator as something stateless, only looking at currently available memory and letting the comparison select what is useful. Maybe for example this operator would be used to find the next unused device, etc. My doubts is that this is kind of future-proofing the design, something that I don't think is good practice. So, if you prefer something that takes care of state, as far as devargs parsing is concerned, I'm ok with that. Otherwise it can stay that way, UNUSED devices are taken care of afterward. > > + 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 sure. > > <...> -- Gaëtan Rivet 6WIND