From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gaetan.rivet@6wind.com>
Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68])
 by dpdk.org (Postfix) with ESMTP id 1DA3F34F0
 for <dev@dpdk.org>; Tue, 27 Mar 2018 14:40:16 +0200 (CEST)
Received: by mail-wm0-f68.google.com with SMTP id f125so21382842wme.4
 for <dev@dpdk.org>; Tue, 27 Mar 2018 05:40:16 -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=R9idJa/usi+4cTPBkcQ2M84jD06e42YJMuRxroVxqNQ=;
 b=oKu26Vlj+A+pAw7CX8y7rGO41y/iUidge8TzOiy+78m6DeXgRiFJUgheV1sOMGU+/5
 kxAxBE4rjtQB9WbomJbnRMJsTW3IpUF/2iLnEm0IiKq+m04DE7FVHWCSCBgS7rTpJh+z
 7r324uoPUR3/jM4P0RqW636sz66gWPbbu03H1iNo1AIBQLiiGntNQLVNiApuYL8rNGW8
 OF2Gi259XChNMRHSBZTdqpDyutSL7qr1r2WkjcOIiFGCwLw5YMBYi+i0leKFBb8WfmBC
 cR6fSfRNNeq5V19rci6al1qSSMjOZCLIfuy0aUR8PPex6kKlBXX29RwTWi3F9AYyG5nr
 ajEw==
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=R9idJa/usi+4cTPBkcQ2M84jD06e42YJMuRxroVxqNQ=;
 b=qCHmnUV5rXHT7jXAhl+utCqbbL+LeivJPCXEHN0+DFl5Uw3msy3qLiPCG6CZYZJ6K2
 ZaMrbXW5P4v11l40CQtpA4OhAfItb8A5pDM2lFx/5qpjo89Tc8tOgZ7aNaBVFASYLEDC
 8vusJpak5srw4EqgmQbwlWehe30PoKS2UkdR0y+im9Xr8Vpwx0wSGaIgmAyQpnJ5wXoA
 WGHqDn5cRgECLW//rRmy5vxVPvIdkPatgxCsv6fciideTjq5x/vBX5A2FRobYl8bgs4L
 o6QVIV+7so5oVfKydjQdkTVJQTcEoP4uaa41PQPhJR8GD70WvxQzqjRmvSn5YpS3058g
 7QxA==
X-Gm-Message-State: AElRT7HzfvV/aUTbGL/BmEg8pNNGY/shOaOL/+2NnU6SYd/l+PUy3FvE
 /WmcGvbaKTU0RXqY5LQIJ75rYg==
X-Google-Smtp-Source: AG47ELuE2MbxSE3JVWKU1ox/dVWEinKe8/Gr7bHpgU5w7CGTBuIM4VVEqdgAJJ/Hz36KFDLrlAeYgQ==
X-Received: by 10.28.62.16 with SMTP id l16mr19703384wma.54.1522154415447;
 Tue, 27 Mar 2018 05:40:15 -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 b34sm709441wra.21.2018.03.27.05.40.13
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Tue, 27 Mar 2018 05:40:14 -0700 (PDT)
Date: Tue, 27 Mar 2018 14:40:00 +0200
From: =?iso-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: dev@dpdk.org, "Wiles, Keith" <keith.wiles@intel.com>
Message-ID: <20180327124000.6n63hpng53tm3bil@bidouze.vm.6wind.com>
References: <cover.1522105876.git.gaetan.rivet@6wind.com>
 <f7f7115526378e8d83e03cf5e6bc3bead6571f24.1522105876.git.gaetan.rivet@6wind.com>
 <20180327114750.GA30585@hmswarspite.think-freely.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180327114750.GA30585@hmswarspite.think-freely.org>
User-Agent: NeoMutt/20170113 (1.7.2)
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 <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 27 Mar 2018 12:40:16 -0000

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 <nhorman@tuxdriver.com>
> > Cc: "Wiles, Keith" <keith.wiles@intel.com>
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> > 
> > 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 <rte_compat.h>
> >  #include <rte_bus.h>
> > +#include <rte_class.h>
> >  #include <rte_dev.h>
> >  #include <rte_devargs.h>
> >  #include <rte_debug.h>
> > +#include <rte_errno.h>
> > +#include <rte_kvargs.h>
> >  #include <rte_log.h>
> >  
> >  #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.

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