Re: [dpdk-dev] [PATCH v5 00/14] dpdk: Separate compile time linkage between eal lib and pmd's

[Neil Horman <nhorman@tuxdriver.com>]

On Mon, Apr 21, 2014 at 01:10:00PM -0700, Stephen Hemminger wrote:
> On Mon, 21 Apr 2014 10:59:25 -0400
> Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > Disconnect compile time linkage between eal library / applications and pmd's
> > 
> > I noticed that, while tinkering with dpdk, building for shared libraries still
> > resulted in all the test applications linking to all the built pmd's, despite
> > not actually needing them all.  We are able to tell an application at run time
> > (via the -d/--blacklist/--whitelist/--vdev options) which pmd's we want to use,
> > and so have no need to link them at all. The only reason they get pulled in is
> > because rte_eal_non_pci_init_etherdev and rte_pmd_init_all contain static lists
> > to the individual pmd init functions. The result is that, even when building as
> > DSO's, we have to load all the pmd libraries, which is space inefficient and
> > defeating of some of the purpose of shared objects.
> > 
> > To correct this, I developed this patch series, which introduces a new macro,
> > PMD_REGISTER_DRIVER, which wraps up Oliviers work using constructors on the
> > virtual device pmds, then expands on it to include the physical device pmds,
> > allowing us to break linkages between dpdk applications and pmd's almost
> > entirely (save for the ring and xenvirt drivers, which have additional api's
> > outside of the standard dpdk code that we need to further fix).  This also
> > allows us to completely remove the rte_pmd_init_all routine, hiding its function
> > internally to the rte_eal_init path.
> > 
> > I've tested this feature using the igb and pcap pmd's, both statically and
> > dynamically linked with the test and testpmd sample applications, and it seems
> > to work well.
> > 
> > Note, I encountered  a few bugs along the way, which I fixed and noted in the
> > series.
> > 
> > Regards
> > Neil
> > 
> > Change Notes:
> > 
> > 1) Reposted the entire series.  Recent changes permeated accross several
> > patches, and since a few patches already had multiple versions, I've reposted
> > the entire series and bumped the version number to 5, whcih skips a few
> > versions, but ensures this series is seen as the most recent.
> > 
> > 2) Merged the PMD_REGISTER_DRIVER macro into rte_dev.h, which is a better
> > location for it.  Required removing include of rte_pmd.h across most of the
> > patches in the series.
> > 
> > 3) Cleaned up various leftover "virtual" comments in the driver registration api
> > that no longer belong there after making it generic
> > 
> > 4) Fixed the LINK_USING_CC check in rte.lib.mk so it works like other uses
> > 
> > 5) Fixed CPU_LDFLAGS conversion to use -Wl in case we link with gcc.
> 
> I am ok with build device drivers as libraries, but there can be a
> performance impact.  For performance, we link with link-time optimization
> and that is not possible as shared library. Haven't measured but at least
> in the kernel, modules add a performance tax because they aren't in same
> memory region and therefore cause a TLB miss. Might have same impact in
> user mode.
> 
Possibly, but that seems like a build time decision.  Performance numbers are
definately good for guidance here, but one way or another if you're not ok with
the impact, you configure to build statically, and go on about your day.  We're
not preventing that here, just making the DSO build case work a little more
sanely, so that the core libraries and applications don't have to link against
the DSO's directly.

> You might be able to get some of this back by compiling with link-time-optimization
> on a device at a time, and
> also use the flags to tell compiler that only certain entry points are
> shared and need tob e global.
> 
That seems like a fine idea.  In fact with this patch, a typical pmd would need
no global entry points, as the constructor registers the init routine (which can
be static), and the init routine registers the ethernet device driver.  We could
probably (with this changeset) create a boilerplate linker script for the pmds
that marked all the symbols except constructors as local (xenvirt and ring are
notable exceptions to this rule, but those can likely be fixed up).

Regardless, I'd like to address that in a separate patch, as I think this series
is doing enough on its own.  It a good idea for optimization though.

Neil