From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 0E7CE4CC0 for ; Tue, 27 Mar 2018 20:27:14 +0200 (CEST) Received: from cpe-2606-a000-111b-40b7-640c-26a-4e16-9225.dyn6.twc.com ([2606:a000:111b:40b7:640c:26a:4e16:9225] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1f0tJJ-0006lT-6g; Tue, 27 Mar 2018 14:27:11 -0400 Date: Tue, 27 Mar 2018 14:26:33 -0400 From: Neil Horman To: =?iso-8859-1?Q?Ga=EBtan?= Rivet Cc: dev@dpdk.org, "Wiles, Keith" Message-ID: <20180327182633.GC30585@hmswarspite.think-freely.org> References: <20180327114750.GA30585@hmswarspite.think-freely.org> <20180327124000.6n63hpng53tm3bil@bidouze.vm.6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180327124000.6n63hpng53tm3bil@bidouze.vm.6wind.com> User-Agent: Mutt/1.9.2 (2017-12-15) X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH v3 10/20] eal/dev: implement device iteration initialization 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: Tue, 27 Mar 2018 18:27:15 -0000 On Tue, Mar 27, 2018 at 02:40:00PM +0200, Gaëtan Rivet wrote: > On Tue, Mar 27, 2018 at 07:47:50AM -0400, Neil Horman wrote: > > On Tue, Mar 27, 2018 at 01:18:34AM +0200, Gaetan Rivet wrote: > > > Parse a device description. > > > Split this description in their relevant part for each layers. > > > No dynamic allocation is performed. > > > > > > Cc: Neil Horman > > > Cc: "Wiles, Keith" > > > Signed-off-by: Gaetan Rivet > > > --- > > > > > > This version uses librte_kvargs. > > > > > > lib/Makefile | 1 + > > > lib/librte_eal/bsdapp/eal/Makefile | 1 + > > > lib/librte_eal/common/eal_common_dev.c | 147 ++++++++++++++++++++++++++++++++ > > > lib/librte_eal/common/include/rte_dev.h | 23 +++++ > > > lib/librte_eal/linuxapp/eal/Makefile | 1 + > > > lib/librte_eal/rte_eal_version.map | 1 + > > > 6 files changed, 174 insertions(+) > > > > > > diff --git a/lib/Makefile b/lib/Makefile > > > index fc7a55a37..1b17526f7 100644 > > > --- a/lib/Makefile > > > +++ b/lib/Makefile > > > @@ -6,6 +6,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > > > DIRS-y += librte_compat > > > DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs > > > DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal > > > +DEPDIRS-librte_eal := librte_kvargs > > > DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci > > > DEPDIRS-librte_pci := librte_eal > > > DIRS-$(CONFIG_RTE_LIBRTE_RING) += librte_ring > > > diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile > > > index 17ff1ac45..f6cea7fc2 100644 > > > --- a/lib/librte_eal/bsdapp/eal/Makefile > > > +++ b/lib/librte_eal/bsdapp/eal/Makefile > > > @@ -18,6 +18,7 @@ CFLAGS += $(WERROR_FLAGS) -O3 > > > LDLIBS += -lexecinfo > > > LDLIBS += -lpthread > > > LDLIBS += -lgcc_s > > > +LDLIBS += -lrte_kvargs > > > > > > EXPORT_MAP := ../../rte_eal_version.map > > > > > > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c > > > index cd071442f..1f6df2351 100644 > > > --- a/lib/librte_eal/common/eal_common_dev.c > > > +++ b/lib/librte_eal/common/eal_common_dev.c > > > @@ -10,9 +10,12 @@ > > > > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > +#include > > > +#include > > > #include > > > > > > #include "eal_private.h" > > > @@ -207,3 +210,147 @@ rte_eal_hotplug_remove(const char *busname, const char *devname) > > > rte_eal_devargs_remove(busname, devname); > > > return ret; > > > } > > > + > > > +static size_t > > > +dev_layer_count(const char *s) > > > +{ > > > + size_t i = s ? 1 : 0; > > > + > > > + while (s != NULL && s[0] != '\0') { > > > + i += s[0] == '/'; > > > + s++; > > > + } > > > + return i; > > > +} > > > + > > So the above code really just counts the number characters in the string, > > omitting the delimiter character '/', right? If thats all you want to do, you can just > > use strtok and strnlen for that, cant you? > > Will do. > > > > > Otherwise, this looks pretty good to me > > Please look into the librte_kvargs compatibility patch as well (quite > short). I'm very unhappy about the logging hack. > There is always the solution of setting a function pointer on rte_log > with the proper loglevels and so on. > Ideally rte_log could be made independent (starting skimming EAL from > all the fat), but this is much less trivial. > just posted about that. I agree with Keith, I don't think you should need that patch. RTE_LOG just calls rte_vlog which contains this code: if (f == NULL) { f = default_log_stream; if (f == NULL) { /* * Grab the current value of stderr here, rather than * just initializing default_log_stream to stderr. This * ensures that we will always use the current value * of stderr, even if the application closes and * reopens it. */ f = stderr; } } } Which I read as saying that the logging library should back off to stderr if its not initialized yet. If you've encountered a problem that made you need that logging patch, it seems like you should be able to drop it, and we need to fix the logging library. Can you elaborate on what you ran into here? Neil > This implementation relies on librte_kvargs being available. If this is > not the case, there isn't much point to it. > > -- > Gaëtan Rivet > 6WIND >