Re: [dpdk-dev] [PATCH 0/8] Improve build process

[Neil Horman <nhorman@tuxdriver.com>]

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 <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?
> > 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).

Regards
Neil