Re: [PATCH 0/8] Improve build process
From: Neil Horman <hidden>
Date: 2015-01-29 19:45:39
On Thu, Jan 29, 2015 at 05:04:20PM +0000, Gonzalez Monroy, Sergio wrote:
quoted
From: Neil Horman [mailto:nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org] Sent: Thursday, January 29, 2015 4:39 PM To: Gonzalez Monroy, Sergio Cc: dev-VfR2kkLFssw@public.gmane.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:quoted
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.3Something 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 <file>_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? NeilHi 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). Regards Neil