From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 683D01F7 for ; Fri, 30 Jan 2015 18:38:54 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP; 30 Jan 2015 09:38:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,492,1418112000"; d="scan'208";a="520323585" Received: from irsmsx109.ger.corp.intel.com ([163.33.3.23]) by orsmga003.jf.intel.com with ESMTP; 30 Jan 2015 09:31:32 -0800 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.64]) by IRSMSX109.ger.corp.intel.com ([169.254.13.11]) with mapi id 14.03.0195.001; Fri, 30 Jan 2015 17:38:49 +0000 From: "Gonzalez Monroy, Sergio" To: Neil Horman Thread-Topic: [dpdk-dev] [PATCH 0/8] Improve build process Thread-Index: AQHQO9csAhbRLFHQkk+N8RLUPQ4JH5zXTHuAgAACWfCAADHOgIAA7BoggABHFoCAACPbsA== Date: Fri, 30 Jan 2015 17:38:49 +0000 Message-ID: <91383E96CE459D47BCE92EFBF5CE73B004F45534@IRSMSX108.ger.corp.intel.com> References: <1422544811-26385-1-git-send-email-sergio.gonzalez.monroy@intel.com> <20150129163859.GE1999@hmsreliant.think-freely.org> <91383E96CE459D47BCE92EFBF5CE73B004F43D9B@IRSMSX108.ger.corp.intel.com> <20150129194539.GG1999@hmsreliant.think-freely.org> <91383E96CE459D47BCE92EFBF5CE73B004F453D7@IRSMSX108.ger.corp.intel.com> <20150130140507.GA2664@hmsreliant.think-freely.org> In-Reply-To: <20150130140507.GA2664@hmsreliant.think-freely.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 0/8] Improve build process X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Jan 2015 17:38:55 -0000 > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Friday, January 30, 2015 2:05 PM > To: Gonzalez Monroy, Sergio > Cc: Thomas Monjalon; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 0/8] Improve build process >=20 > On Fri, Jan 30, 2015 at 01:39:28PM +0000, Gonzalez Monroy, Sergio wrote: > > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > Sent: Thursday, January 29, 2015 7:46 PM > > > To: Gonzalez Monroy, Sergio > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH 0/8] Improve build process > > > > > > On Thu, Jan 29, 2015 at 05:04:20PM +0000, Gonzalez Monroy, Sergio > wrote: > > > > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > > > Sent: Thursday, January 29, 2015 4:39 PM > > > > > To: Gonzalez Monroy, Sergio > > > > > Cc: dev@dpdk.org > > > > > Subject: Re: [dpdk-dev] [PATCH 0/8] Improve build process > > > > > > > > > > On Thu, Jan 29, 2015 at 03:20:03PM +0000, Sergio Gonzalez Monroy > > > wrote: > > > > > > This patch series improves the DPDK build system mostly for > > > > > > shared libraries (and a few nits for static libraries) with the= following > goals: > > > > > > - Create a library containing core DPDK libraries (librte_eal, > > > > > > librte_malloc, librte_mempool, librte_mbuf and librte_ring). > > > > > > The idea of core libraries is to group those libraries that = are > > > > > > always required (and have interdependencies) for any DPDK > > > application. > > > > > > - Remove config option to build a combined library. > > > > > > - For shared libraries, explicitly link against dependant > > > > > > libraries (adding entries to DT_NEEDED). > > > > > > - Update app linking flags for static/shared DPDK libs. > > > > > > > > > > > > Sergio Gonzalez Monroy (8): > > > > > > mk: remove combined library and related options > > > > > > core: create new librte_core > > > > > > mk: new corelib makefile > > > > > > lib: update DEPDIRS variable > > > > > > lib: set LDLIBS for each library > > > > > > mk: use LDLIBS when linking shared libraries > > > > > > mk: update LDLIBS for app building > > > > > > mk: add -lpthread to linuxapp EXECENV_LDLIBS > > > > > > > > > > > > config/common_bsdapp | 6 -- > > > > > > config/common_linuxapp | 6 -- > > > > > > config/defconfig_ppc_64-power8-linuxapp-gcc | 2 - > > > > > > lib/Makefile | 1 - > > > > > > lib/librte_acl/Makefile | 5 +- > > > > > > lib/librte_cfgfile/Makefile | 4 +- > > > > > > lib/librte_cmdline/Makefile | 6 +- > > > > > > lib/librte_core/Makefile | 45 ++++++++++++= + > > > > > > lib/librte_distributor/Makefile | 5 +- > > > > > > lib/librte_eal/bsdapp/eal/Makefile | 3 +- > > > > > > lib/librte_eal/linuxapp/eal/Makefile | 3 +- > > > > > > lib/librte_ether/Makefile | 4 +- > > > > > > lib/librte_hash/Makefile | 4 +- > > > > > > lib/librte_ip_frag/Makefile | 6 +- > > > > > > lib/librte_ivshmem/Makefile | 4 +- > > > > > > lib/librte_kni/Makefile | 6 +- > > > > > > lib/librte_kvargs/Makefile | 6 +- > > > > > > lib/librte_lpm/Makefile | 6 +- > > > > > > lib/librte_malloc/Makefile | 2 +- > > > > > > lib/librte_mbuf/Makefile | 2 +- > > > > > > lib/librte_mempool/Makefile | 2 +- > > > > > > lib/librte_meter/Makefile | 4 +- > > > > > > lib/librte_pipeline/Makefile | 3 + > > > > > > lib/librte_pmd_af_packet/Makefile | 5 +- > > > > > > lib/librte_pmd_bond/Makefile | 7 +- > > > > > > lib/librte_pmd_e1000/Makefile | 8 ++- > > > > > > lib/librte_pmd_enic/Makefile | 8 ++- > > > > > > lib/librte_pmd_i40e/Makefile | 8 ++- > > > > > > lib/librte_pmd_ixgbe/Makefile | 8 ++- > > > > > > lib/librte_pmd_pcap/Makefile | 5 +- > > > > > > lib/librte_pmd_ring/Makefile | 6 +- > > > > > > lib/librte_pmd_virtio/Makefile | 7 +- > > > > > > lib/librte_pmd_vmxnet3/Makefile | 8 ++- > > > > > > lib/librte_pmd_xenvirt/Makefile | 8 ++- > > > > > > lib/librte_port/Makefile | 8 +-- > > > > > > lib/librte_power/Makefile | 4 +- > > > > > > lib/librte_ring/Makefile | 2 +- > > > > > > lib/librte_sched/Makefile | 7 +- > > > > > > lib/librte_table/Makefile | 8 +-- > > > > > > lib/librte_timer/Makefile | 6 +- > > > > > > lib/librte_vhost/Makefile | 9 +-- > > > > > > mk/exec-env/linuxapp/rte.vars.mk | 2 + > > > > > > mk/rte.app.mk | 53 ++++--------= --- > > > > > > mk/rte.corelib.mk | 84 ++++++++++++= +++++++++++ > > > > > > mk/rte.lib.mk | 49 +++---------= -- > > > > > > mk/rte.sdkbuild.mk | 3 - > > > > > > mk/rte.sharelib.mk | 101 ------------= ---------------- > > > > > > mk/rte.vars.mk | 9 --- > > > > > > 48 files changed, 276 insertions(+), 282 deletions(-) create > > > > > > mode > > > > > > 100644 lib/librte_core/Makefile create mode 100644 > > > > > > mk/rte.corelib.mk delete mode 100644 mk/rte.sharelib.mk > > > > > > > > > > > > -- > > > > > > 1.9.3 > > > > > > > > > > > > > > > > > Something occured to me thinking about this patch set. I > > > > > noticed recently that different rules are used to build the > > > > > shared combined lib from the individual shared objects. The > > > > > implication here is that linker options specified in individual > > > > > make files (like the LIBABIVER and EXPORT_MAP options in my ABI > > > > > versioning script) get ignored, which is bad. Any other file > > > > > specific linker options (like _LDFLAGS specified in > > > > > individual library makefiles are getting > > > dropped for the combined lib. > > > > > > > > > > It seems like it would be better if the combined libs were > > > > > manufactured as linker scripts themselves (textfiles that used > > > > > linker directives to include individual libraries under the > > > > > covers (see > > > /lib64/libc.so for an example). > > > > > > > > > > The disadvantage of such an approach are fairly minimal. With > > > > > such a combined library, you still need to install individual > > > > > libraries, but for applications that wish to link and run > > > > > against a single dpdk library will still work just as they > > > > > currently do, you can link to just a single > > > library. > > > > > > > > > > The advantage is clear however. By following a linker script > > > > > aproach, objects build as separate libraries are built exactly > > > > > the same way, using the same rules with the same options. It > > > > > reduces the dpdk build environment size and complexity, and > > > > > reduces the opportunity for bugs to creep in from forgetting to > > > > > add build options to multiple locations. It also provides a > > > > > more granular approach for grouping files. Creating a dpdk core > > > > > library becomes a matter of creating a one line linker script > > > > > named libdpdk_core.so, rather > > > than re- arraning sections of the build system. > > > > > > > > > > Thoughts? > > > > > Neil > > > > > > > > > Hi Neil, > > > > > > > > I think that is a very interesting approach. > > > > I have tried to do something similar in this patch by removing > > > > rte.sharelib.mk and just having rte.lib.mk to do the linking, > > > > leaving as you suggest a single file to modify anything related to = building > libs. > > > > > > > > I do think however that your proposal is an improvement over the > > > > current > > > patch. > > > > > > > > So basically we want: > > > > - get rid of rte.corelib.mk > > > > - generate librte_core.so linker script grouping core libs > > > > - we do not modify DEPDIR variables > > > > - when setting LDLIBS to each lib, we do specify -lrte_core, right? > > > > > > > Exactly, and librte_core.so is really just a text file containing > > > the following line > > > : > > > INPUT(-lrte_malloc -lrte_mbuf -lrte_eal ....) > > > > > > Adding in whatever libraries you want librte_core to consist of. > > > Truthfully, you could almost get rid of the COMBINE_LIBS option > > > entirely, and just create this file statically if you wanted to (not > > > sure thats the best approach, but its definately do-able). > > > > > Hi Neil, > > > > Actually, the first patch series does get rid of COMBINE_LIBS entirely. > > > Sorry, I didn't mean to imply your patch wasn't, just re-iterating that t= he > option is not needed using the alternate method we're discussing, but I r= eally > wasn't very clear on that. >=20 > > So as I was looking into this, by using this approach we do not resolve= the > interdependencies issue of the core libraries. > > We would effectively leave all core libraries (or at least EAL) without= proper > DT_NEEDED entries. > > > > Thoughts? > > > You're correct, or at least what you assert is possible, depending on the > implementation. Adding DT_NEEDED entries is something of an orthogonal > problem (though your current implementation I think handles it well). Yo= u > could specify linker directives when building each library so that each D= SO > contains the proper DT_NEEDED entries (using -l and --no-as-needed). > using a linker script approach doesn't preclude you from doing that, thou= gh > its not strictly speaking necessecary. When you write the linker script,= you > implicitly specify the link dependencies by the order in whcih you list t= he > inferior libraries in the scripts INPUT line. It doesn't give you the DT= _NEEDED > entries, but from an application build/run standpoint, it won't matter, > because the libraries will be linked/loaded in the order specified. You = can still > do the --no-as-needed method though if you like for safety on the part of > those using libraries independently. So would it be reasonable to add DT_NEEDED entries to all DPDK libraries bu= t EAL? If I understood what you were saying right, we could enforce the 'dependenc= y' in the=20 linker script with something like this: $ cat librte_eal.so INPUT( librte_eal.so.1 -lrte_mempool -lrte_malloc) We could have such linker script for librte_eal.so instead of the soft link= once versioning is in place. Things that would be missing versus the proposed patch: - As I have mention in previous post, ldd info for EAL library would not r= eflect its dependency to other DPDK libs. - I was enforcing resolving all references when building the libraries (-z= defs), so we either remove it altogether or skip eal. - All apps would show DT_NEEDED entries for a set of DPDK libraries that in most cases are required (eal, mempool, malloc, mbuf, ring VS dpdk_cor= e) I think that the linker script approach is reasonable if we prefer to go th= at way instead of creating a core library. Regards, Sergio > Neil >=20 > > Regards, > > Sergio > > > > > Regards > > > Neil > > > > >