From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from mail-wm0-f53.google.com (mail-wm0-f53.google.com [74.125.82.53])
 by dpdk.org (Postfix) with ESMTP id 691282C27
 for <dev@dpdk.org>; Wed, 12 Apr 2017 21:55:24 +0200 (CEST)
Received: by mail-wm0-f53.google.com with SMTP id u2so32037384wmu.0
 for <dev@dpdk.org>; Wed, 12 Apr 2017 12:55:24 -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:in-reply-to:references
 :mime-version:content-transfer-encoding;
 bh=qkwEShJHTwRLbmSNPyaySiocr178GzK4c6mPaaFfSGM=;
 b=nDhHWlGc02uD5L1buHj800VEkb4C29fY9XvFD7ndPNRkMgthwpNiOcORYyvBZiWjh+
 W2/HnYXt3QKwL4puvBPhOwPSKKuhxiKGWbAiRFpYIl7pcaRoaIAxFBpzHmv35EEC0OzF
 S5ntq409dNhvSS0vTBYMqki1zuFmASR6zLER2uax6tLf71f8VfzzLQf2HU37xjdmd1Dc
 ubtEl/egGiLNbyOrnQskkuKASJqIXz01591FGfGEmYLzV7psbxrZqz2PuQhWfUtAjKJK
 YSq7nXIqO7NTjYM148JmsPftMG8X+G/nItimHjy6dbqIPuuA7YJBUyFW5ueAqK5wENVY
 GKgg==
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:in-reply-to
 :references:mime-version:content-transfer-encoding;
 bh=qkwEShJHTwRLbmSNPyaySiocr178GzK4c6mPaaFfSGM=;
 b=O0XkXQY/I6RTOJ39R+qpgSoHBngNBh3ArqkAnwL//u/XraZgkrMvsNNadFhmoknkCg
 D5suLlqaVdRA0PqTrltFwpXIZp0GXZ1BF3zGagr3M54Hig5R2G+FYtvMNK4DPHu+vgEi
 tQbWY2LOATr8owL+2awHyfkyCoUXFMSC5+HXWyK3aZmgmFntXJ8KChjtgsJvCZBnqIXs
 6hfIqxZJbK6JzTpd/Eg8e+skJtEaV94Bg1HrcU7dLpCnpgKYgskCaftjV5HCLwqAGxom
 uNAyS3gfh6aQgqeTrFNL9bD0Es/2XUTjv+ieMJB63GS+bbg6cX+4oo0oNJOOOm0cPriO
 LoYA==
X-Gm-Message-State: AN3rC/5j3yUYz490g1XpNrKiWkh+ySp5MAsnJyJaitA9eTQJkYaQsMJg
 b/GVfKWZdnfnR6Mb
X-Received: by 10.28.90.2 with SMTP id o2mr20765238wmb.53.1492026923972;
 Wed, 12 Apr 2017 12:55:23 -0700 (PDT)
Received: from neon (bc535-h01-176-146-114-125.dsl.sta.abo.bbox.fr.
 [176.146.114.125])
 by smtp.gmail.com with ESMTPSA id 94sm26826797wrp.34.2017.04.12.12.55.23
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Wed, 12 Apr 2017 12:55:23 -0700 (PDT)
Date: Wed, 12 Apr 2017 21:55:20 +0200
From: Olivier MATZ <olivier.matz@6wind.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>, Shreyansh Jain
 <shreyansh.jain@nxp.com>, john miller <john.miller@atomicrules.com>,
 dev@dpdk.org
Message-ID: <20170412215520.44c485a2@neon>
In-Reply-To: <8154073.47RD1mYHR3@xps13>
References: <357262EF-AA7B-4064-963E-FF130E4607E6@atomicrules.com>
 <VI1PR0401MB24644620D86665DC93E8073F90030@VI1PR0401MB2464.eurprd04.prod.outlook.com>
 <59AF69C657FD0841A61C55336867B5B06675640D@IRSMSX103.ger.corp.intel.com>
 <8154073.47RD1mYHR3@xps13>
X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] error in testpmd when CONFIG_RTE_BUILD_SHARED_LIB=y
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://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: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 12 Apr 2017 19:55:24 -0000

Hi,

On Wed, 12 Apr 2017 14:31:56 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2017-04-12 11:31, Richardson, Bruce:
> >   
> > > -----Original Message-----
> > > From: Shreyansh Jain [mailto:shreyansh.jain@nxp.com]
> > > Sent: Wednesday, April 12, 2017 12:02 PM
> > > To: Richardson, Bruce <bruce.richardson@intel.com>
> > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; john miller
> > > <john.miller@atomicrules.com>; dev@dpdk.org; olivier.matz@6wind.com
> > > Subject: RE: [dpdk-dev] error in testpmd when
> > > CONFIG_RTE_BUILD_SHARED_LIB=y
> > >   
> > > > -----Original Message-----
> > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > Sent: Wednesday, April 12, 2017 4:12 PM
> > > > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; john miller
> > > > <john.miller@atomicrules.com>; dev@dpdk.org; olivier.matz@6wind.com
> > > > Subject: Re: [dpdk-dev] error in testpmd when
> > > > CONFIG_RTE_BUILD_SHARED_LIB=y
> > > >
> > > > On Wed, Apr 12, 2017 at 11:38:55AM +0100, Bruce Richardson wrote:  
> > > > > On Wed, Apr 12, 2017 at 10:33:10AM +0000, Shreyansh Jain wrote:  
> > > > > > My bad - I was too quick in replying - some clarification beneath.
> > > > > >  
> > > > > > > -----Original Message-----
> > > > > > > From: Shreyansh Jain
> > > > > > > Sent: Wednesday, April 12, 2017 3:55 PM
> > > > > > > To: 'Bruce Richardson' <bruce.richardson@intel.com>
> > > > > > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; john miller
> > > > > > > <john.miller@atomicrules.com>; dev@dpdk.org;
> > > > > > > olivier.matz@6wind.com
> > > > > > > Subject: RE: [dpdk-dev] error in testpmd when  
> > > > CONFIG_RTE_BUILD_SHARED_LIB=y  
> > > > > > >  
> > > > > > > > -----Original Message-----
> > > > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > > > > > Sent: Wednesday, April 12, 2017 3:35 PM
> > > > > > > > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > > > > > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; john miller
> > > > > > > > <john.miller@atomicrules.com>; dev@dpdk.org;
> > > > > > > > olivier.matz@6wind.com
> > > > > > > > Subject: Re: [dpdk-dev] error in testpmd when  
> > > > CONFIG_RTE_BUILD_SHARED_LIB=y  
> > > > > > > >
> > > > > > > > On Wed, Apr 12, 2017 at 04:52:47AM +0000, Shreyansh Jain wrote:  
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > > > > Sent: Wednesday, April 12, 2017 12:58 AM
> > > > > > > > > > To: john miller <john.miller@atomicrules.com>
> > > > > > > > > > Cc: dev@dpdk.org; olivier.matz@6wind.com; Shreyansh Jain
> > > > > > > > > > <shreyansh.jain@nxp.com>
> > > > > > > > > > Subject: Re: [dpdk-dev] error in testpmd when  
> > > > > > > > CONFIG_RTE_BUILD_SHARED_LIB=y  
> > > > > > > > > >
> > > > > > > > > > 2017-04-11 14:02, john miller:  
> > > > > > > > > > >
> > > > > > > > > > > We are seeing an issue when running from the head of the
> > > > > > > > > > > master  
> > > > > > > branch  
> > > > > > > > in  
> > > > > > > > > > dpdk-next-net and building with  
> > > CONFIG_RTE_BUILD_SHARED_LIB=y.  
> > > > When  
> > > > > > > we  
> > > > > > > > run  
> > > > > > > > > > testpmd using  -d to point to our PMD we get this error  
> > > > > > > > > > >
> > > > > > > > > > > EAL: Error - exiting with code: 1
> > > > > > > > > > >   Cause: Creation of mbuf pool for socket 0 failed:
> > > > > > > > > > > Invalid  
> > > > argument  
> > > > > > > > > > >
> > > > > > > > > > > This error occurs as a result of the rte mempool ops
> > > > > > > > > > > table  
> > > > having 0  
> > > > > > > > > > entries.  This table is populated from a call to  
> > > > > > > > rte_mempool_register_ops().  
> > > > > > > > > > This function gets called in rte_mempool_ring.c via the
> > > > > > > > > > static  
> > > > > > > > initialization  
> > > > > > > > > > MACRO MEMPOOL_REGISTER_OPS and exists in  
> > > librte_mempool_ring.so.  
> > > > > > > However  
> > > > > > > > > > this library is not loaded when the rte_eal_init() gets
> > > > > > > > > > called so  
> > > > the  
> > > > > > > > static  
> > > > > > > > > > initializers are not yet loaded.  
> > > > > > > > > > >
> > > > > > > > > > > I am requesting advice on the proper way to repair this.  
> > > > > > > > >
> > > > > > > > > "-d" the ring library (rte_mempool_ring) - just like any
> > > > > > > > > other  
> > > > shared  
> > > > > > > lib.  
> > > > > > > > >  
> > > > > > > >
> > > > > > > > I think this is a bug that should be fixed. The user should
> > > > > > > > not need  
> > > > to  
> > > > > > > > have to specify a mempool driver just to get testpmd working,
> > > > > > > > so I  
> > > > think  
> > > > > > > > the ring handler as default should be compiled in
> > > > > > > > automatically so as  
> > > > to  
> > > > > > > > allow regular mempools to just work as before.  
> > > > > > >
> > > > > > > For Ring Mempool as default enabled, +1  
> > > > > >
> > > > > > Actually, Ring mempool is enabled by default. But, obviously for
> > > > > > shared  
> > > > library case, this still means "-d".  
> > > > > >  
> > > > >
> > > > > Not necessarily. That only applies if we don't explicitly link it
> > > > > like the other shared libraries. We "special-case" our drivers in
> > > > > that we don't add them with a -l flag, but expect the user to
> > > > > dynamically load them at runtime. This is one case where I think all
> > > > > apps should explicitly link against the ring mempool driver. There
> > > > > is no reason we can't make some drivers mandatory.
> > > > >  
> > > > > > >  
> > > > > > > >  
> > > > > > > > > This change was done recently to move ring handler into its  
> > > > separate  
> > > > > > > > drivers/mempool/ring directory. That also means it no longer
> > > > > > > > is  
> > > > compiled  
> > > > > > > into  
> > > > > > > > the librte_mempool.  
> > > > > > > > >  
> > > > > > > > > >
> > > > > > > > > > We should just add a better error message if no mempool
> > > > > > > > > > driver is  
> > > > > > > > available.  
> > > > > > > > >
> > > > > > > > > Yes, that is something to be improved.  
> > > > > > > >
> > > > > > > > This should be fixed by always having a mempool driver  
> > > installed.  
> > > > > > >
> > > > > > > Agree.  
> > > > > >
> > > > > > Probably, as ring mempool is a driver and compiled in shared mode,  
> > > > enabled by default will not fix this.  
> > > > >
> > > > > But linked in by default will fix it.
> > > > >  
> > > > And as follow-up to my own mail, I think we can actually go further
> > > > here. Mempool is a core library, and very little can be done in DPDK
> > > > without it. It's also not what most people would think as needing a
> > > > driver loaded, so from a usability point of view, I don't see why we
> > > > shouldn't link in all mempool drivers by default, like we do other libs.
> > > > It will make users life easier, and I can't see any downside to doing
> > > > so - they are just .so's after all!  
> > > 
> > > I don't have a particularly strong opinion against this.
> > > For static build, we are already 'there' - mempool would be linked in with
> > > testpmd.
> > > For Shared library, the idea is to have small footprint and leave it to
> > > user to link what is required, and what is not.
> > > 
> > > Still, for usability sakes, we have three options:
> > > 1. Link all library - which might be more than just ring (stack, more to
> > > be added soon...) 2. Only link ring by default - because that is also
> > > being used as default option when creating the mempool (ring_mp_mc) 3.
> > > Don't link any
> > > 
> > > (3) is a cleaner approach, but may not be a good usecase. But, going by
> > > (1) would mean linking in unused mempool handler by default (yes, user
> > > could always say 'n' in config file).
> > > 
> > > So, if we are going to select the mempool as inbuild, we might as well
> > > have it only for Ring (2).
> > > 
> > > It's more important to make DPDK useful than to make it idealistic. :)
> > >   
> > 
> > I'm ok with option 2 or 3.  
> 
> I think the default mempool could be linked.
> I don't know how easy it is to transform
> 	CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS="ring_mp_mc"
> into
> 	-lrte_mempool_ring
> 
> Anyone for a patch?

This is the default mempool for the mbufs, selected when using the
function rte_pktmbuf_pool_create().

For non-mbuf mempools, the default is to use the ring handler (see
in rte_mempool_create()).

So if we want option 2-, it looks that moving this line in rte.app.mk
above, outside the if (shared build), would do the job:

_LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING)   += -lrte_mempool_ring

I think both 2- and 3- are acceptable. People using the shared library
mode should already provide all the .so in a known location, so that
they are loaded by default. This is needed for the PMDs for instance.


Olivier