Re: [PATCH v3 2/4] Provide initial versioning for all DPDK libraries

[Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>]

On Wed, Jan 14, 2015 at 04:29:29PM +0100, Thomas Monjalon wrote:
> 2014-12-23 10:51, Neil Horman:
> > Add linker version script files to each DPDK library to put a stake in the
> > ground from which we can start cleaning up API's
> 
> [...]
> 
> >  lib/librte_acl/Makefile                            |   2 +
> >  lib/librte_acl/rte_acl_version.map                 |  21 ++++
> >  lib/librte_cfgfile/Makefile                        |   2 +
> >  lib/librte_cfgfile/rte_cfgfile_version.map         |  14 +++
> >  lib/librte_cmdline/Makefile                        |   2 +
> >  lib/librte_cmdline/rte_cmdline_version.map         |  69 +++++++++++++
> >  lib/librte_distributor/Makefile                    |   2 +
> >  lib/librte_distributor/rte_distributor_version.map |  16 +++
> >  lib/librte_eal/bsdapp/eal/Makefile                 |   2 +
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map      |  90 ++++++++++++++++
> >  lib/librte_eal/linuxapp/eal/Makefile               |   2 +
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map    |  90 ++++++++++++++++
> >  lib/librte_ether/Makefile                          |   2 +
> >  lib/librte_ether/rte_ether_version.map             | 113 +++++++++++++++++++++
> >  lib/librte_hash/Makefile                           |   2 +
> >  lib/librte_hash/rte_hash_version.map               |  18 ++++
> >  lib/librte_ip_frag/Makefile                        |   2 +
> >  lib/librte_ip_frag/rte_ipfrag_version.map          |  14 +++
> >  lib/librte_ivshmem/Makefile                        |   2 +
> >  lib/librte_ivshmem/rte_ivshmem_version.map         |  13 +++
> >  lib/librte_kni/Makefile                            |   2 +
> >  lib/librte_kni/rte_kni_version.map                 |  20 ++++
> >  lib/librte_kvargs/Makefile                         |   2 +
> >  lib/librte_kvargs/rte_kvargs_version.map           |  10 ++
> >  lib/librte_lpm/Makefile                            |   2 +
> >  lib/librte_lpm/rte_lpm_version.map                 |  24 +++++
> >  lib/librte_malloc/Makefile                         |   2 +
> >  lib/librte_malloc/rte_malloc_version.map           |  19 ++++
> >  lib/librte_mbuf/Makefile                           |   2 +
> >  lib/librte_mbuf/rte_mbuf_version.map               |  14 +++
> >  lib/librte_mempool/Makefile                        |   2 +
> >  lib/librte_mempool/rte_mempool_version.map         |  18 ++++
> >  lib/librte_meter/Makefile                          |   2 +
> >  lib/librte_meter/rte_meter_version.map             |  13 +++
> >  lib/librte_pipeline/Makefile                       |   2 +
> >  lib/librte_pipeline/rte_pipeline_version.map       |  23 +++++
> >  lib/librte_pmd_af_packet/Makefile                  |   2 +
> >  .../rte_pmd_af_packet_version.map                  |   7 ++
> >  lib/librte_pmd_bond/Makefile                       |   2 +
> >  lib/librte_pmd_bond/rte_eth_bond_version.map       |  21 ++++
> >  lib/librte_pmd_e1000/Makefile                      |   2 +
> >  lib/librte_pmd_e1000/rte_pmd_e1000_version.map     |   5 +
> >  lib/librte_pmd_enic/Makefile                       |   2 +
> >  lib/librte_pmd_enic/rte_pmd_enic_version.map       |   5 +
> >  lib/librte_pmd_i40e/Makefile                       |   2 +
> >  lib/librte_pmd_i40e/rte_pmd_i40e_version.map       |   5 +
> >  lib/librte_pmd_ixgbe/Makefile                      |   2 +
> >  lib/librte_pmd_ixgbe/rte_pmd_ixgbe_version.map     |   5 +
> >  lib/librte_pmd_pcap/Makefile                       |   2 +
> >  lib/librte_pmd_pcap/rte_pmd_pcap_version.map       |   5 +
> >  lib/librte_pmd_ring/Makefile                       |   2 +
> >  lib/librte_pmd_ring/rte_eth_ring.c                 |   2 +-
> >  lib/librte_pmd_ring/rte_eth_ring.h                 |   6 --
> >  lib/librte_pmd_ring/rte_eth_ring_version.map       |  10 ++
> >  lib/librte_pmd_virtio/Makefile                     |   1 +
> >  lib/librte_pmd_virtio/rte_pmd_virtio_version.map   |   5 +
> >  lib/librte_pmd_vmxnet3/Makefile                    |   2 +
> >  lib/librte_pmd_vmxnet3/rte_pmd_vmxnet3_version.map |   5 +
> >  lib/librte_pmd_xenvirt/Makefile                    |   2 +
> >  lib/librte_pmd_xenvirt/rte_eth_xenvirt_version.map |   8 ++
> >  lib/librte_port/Makefile                           |   2 +
> >  lib/librte_port/rte_port_version.map               |  18 ++++
> >  lib/librte_power/Makefile                          |   2 +
> >  lib/librte_power/rte_power_version.map             |  18 ++++
> >  lib/librte_ring/Makefile                           |   2 +
> >  lib/librte_ring/rte_ring_version.map               |  12 +++
> >  lib/librte_sched/Makefile                          |   2 +
> >  lib/librte_sched/rte_sched_version.map             |  22 ++++
> >  lib/librte_table/Makefile                          |   2 +
> >  lib/librte_table/rte_table_version.map             |  22 ++++
> >  lib/librte_timer/Makefile                          |   2 +
> >  lib/librte_timer/rte_timer_version.map             |  16 +++
> >  lib/librte_vhost/Makefile                          |   2 +
> >  lib/librte_vhost/rte_vhost_version.map             |  14 +++
> 
> Honestly, this patch is difficult to review.
> How have you populated .map files? Did you use some script?
> 
I did it the same way I did it the first time I posted this patch way back for
1.8.  I used the libraries' exported header files as a starting point, and
stripped them so that only the function symbols remained (as those are the only
symbols that can be versioned).  I tagged those with a version number, and the
appropriate version script boilerplate syntax.

Not sure what I can do to make review simpler.  But if you have a suggestion,
I'm happy to listen.

> [...]
> 
> > --- a/mk/rte.lib.mk
> > +++ b/mk/rte.lib.mk
> > -CPU_LDFLAGS += --version-script=$(EXPORT_MAP)
> > +CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
> 
> I guess this change should go in patch 1 which introduced this option.
> 
I can, though its not really necessecary, since I didn't list any EXPORT_MAP
variables in patch 1.

Regards
Neil


> -- 
> Thomas
>