From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 31D08137D for ; Tue, 23 Sep 2014 18:37:15 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 23 Sep 2014 09:20:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,581,1406617200"; d="scan'208";a="595684375" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by fmsmga001.fm.intel.com with ESMTP; 23 Sep 2014 09:29:49 -0700 Received: from sivswdev02.ir.intel.com (sivswdev02.ir.intel.com [10.237.217.46]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id s8NGTnY7001731; Tue, 23 Sep 2014 17:29:49 +0100 Received: from sivswdev02.ir.intel.com (localhost [127.0.0.1]) by sivswdev02.ir.intel.com with ESMTP id s8NGTmZM028286; Tue, 23 Sep 2014 17:29:48 +0100 Received: (from smonroy@localhost) by sivswdev02.ir.intel.com with id s8NGTmH3028281; Tue, 23 Sep 2014 17:29:48 +0100 Date: Tue, 23 Sep 2014 17:29:48 +0100 From: Sergio Gonzalez Monroy To: Neil Horman Message-ID: <20140923162947.GA22463@sivswdev02.ir.intel.com> References: <1410809031-19114-1-git-send-email-nhorman@tuxdriver.com> <1410809031-19114-2-git-send-email-nhorman@tuxdriver.com> <20140923103923.GA4642@sivswdev02.ir.intel.com> <20140923145829.GB12884@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140923145829.GB12884@hmsreliant.think-freely.org> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 1/4] compat: Add infrastructure to support symbol versioning 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: Tue, 23 Sep 2014 16:37:16 -0000 On Tue, Sep 23, 2014 at 10:58:29AM -0400, Neil Horman wrote: > On Tue, Sep 23, 2014 at 11:39:29AM +0100, Sergio Gonzalez Monroy wrote: > > Hi Neil, > > > > On Mon, Sep 15, 2014 at 03:23:48PM -0400, Neil Horman wrote: > > > Add initial pass header files to support symbol versioning. > > > > > > Signed-off-by: Neil Horman > > > CC: Thomas Monjalon > > > CC: "Richardson, Bruce" > > > --- > > > lib/Makefile | 1 + > > > lib/librte_compat/Makefile | 38 +++++++++++++++++++ > > > lib/librte_compat/rte_compat.h | 86 ++++++++++++++++++++++++++++++++++++++++++ > > > mk/rte.lib.mk | 6 +++ > > > 4 files changed, 131 insertions(+) > > > create mode 100644 lib/librte_compat/Makefile > > > create mode 100644 lib/librte_compat/rte_compat.h > > > > > > diff --git a/lib/Makefile b/lib/Makefile > > > index 10c5bb3..a85b55b 100644 > > > --- a/lib/Makefile > > > +++ b/lib/Makefile > > > @@ -32,6 +32,7 @@ > > > include $(RTE_SDK)/mk/rte.vars.mk > > > > > > DIRS-$(CONFIG_RTE_LIBC) += libc > > > +DIRS-y += librte_compat > > > DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal > > > DIRS-$(CONFIG_RTE_LIBRTE_MALLOC) += librte_malloc > > > DIRS-$(CONFIG_RTE_LIBRTE_RING) += librte_ring > > > diff --git a/lib/librte_compat/Makefile b/lib/librte_compat/Makefile > > > new file mode 100644 > > > index 0000000..a61511a > > > --- /dev/null > > > +++ b/lib/librte_compat/Makefile > > > @@ -0,0 +1,38 @@ > > > +# BSD LICENSE > > > +# > > > +# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > > > +# All rights reserved. > > > +# > > > +# Redistribution and use in source and binary forms, with or without > > > +# modification, are permitted provided that the following conditions > > > +# are met: > > > +# > > > +# * Redistributions of source code must retain the above copyright > > > +# notice, this list of conditions and the following disclaimer. > > > +# * Redistributions in binary form must reproduce the above copyright > > > +# notice, this list of conditions and the following disclaimer in > > > +# the documentation and/or other materials provided with the > > > +# distribution. > > > +# * Neither the name of Intel Corporation nor the names of its > > > +# contributors may be used to endorse or promote products derived > > > +# from this software without specific prior written permission. > > > +# > > > +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > > +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > > +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > > > +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > > > +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > > > +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > > +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > > > +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > > > +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > > +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > > > +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > > + > > > +include $(RTE_SDK)/mk/rte.vars.mk > > > + > > > + > > > +# install includes > > > +SYMLINK-y-include := rte_compat.h > > > + > > > +include $(RTE_SDK)/mk/rte.lib.mk > > > diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h > > > new file mode 100644 > > > index 0000000..6d65a53 > > > --- /dev/null > > > +++ b/lib/librte_compat/rte_compat.h > > > @@ -0,0 +1,86 @@ > > > +/*- > > > + * BSD LICENSE > > > + * > > > + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > > > + * All rights reserved. > > > + * > > > + * Redistribution and use in source and binary forms, with or without > > > + * modification, are permitted provided that the following conditions > > > + * are met: > > > + * > > > + * * Redistributions of source code must retain the above copyright > > > + * notice, this list of conditions and the following disclaimer. > > > + * * Redistributions in binary form must reproduce the above copyright > > > + * notice, this list of conditions and the following disclaimer in > > > + * the documentation and/or other materials provided with the > > > + * distribution. > > > + * * Neither the name of Intel Corporation nor the names of its > > > + * contributors may be used to endorse or promote products derived > > > + * from this software without specific prior written permission. > > > + * > > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > > + */ > > > + > > > +#ifndef _RTE_COMPAT_H_ > > > +#define _RTE_COMPAT_H_ > > > + > > > +/* > > > + * This is just a stringification macro for use below. > > > + */ > > > +#define SA(x) #x > > > + > > > +#ifdef RTE_SYMBOL_VERSIONING > > > + > > > +/* > > > + * Provides backwards compatibility when updating exported functions. > > > + * When a symol is exported from a library to provide an API, it also provides a > > > + * calling convention (ABI) that is embodied in its name, return type, > > > + * arguments, etc. On occasion that function may need to change to accomodate > > > + * new functionality, behavior, etc. When that occurs, it is desireable to > > > + * allow for backwards compatibility for a time with older binaries that are > > > + * dynamically linked to the dpdk. to support that the __vsym and > > > + * VERSION_SYMBOL macros are created. They, in conjunction with the > > > + * _version.map file for a given library allow for multiple versions of > > > + * a symbol to exist in a shared library so that older binaries need not be > > > + * immediately recompiled. Their use is outlined in the following example: > > > + * Assumptions: DPDK 1.(X) contains a function int foo(char *string) > > > + * DPDK 1.(X+1) needs to change foo to be int foo(int index) > > > + * > > > + * To accomplish this: > > > + * 1) Edit lib//library_version.map to add a DPDK_1.8 node, in which > > > + * foo is exported as a global symbol > > > + * > > > + * 2) rename the existing function int foo(char *string) to > > > + * int __vsym foo_v18(char *string) > > > + * > > > + * 3) Add this macro immediately below the function > > > + * VERSION_SYMBOL(foo, _v18, 1.8); > > > + * > > > + */ > > > +#define VERSION_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ", "SA(b)"@DPDK_"SA(v)) > > > +#define __vsym __attribute__((used)) > > > + > > > > I may be missing something here but would it not be neccessary to define a > > default symbol? > > Otherwise there would be multiple definitions of a given symbol and the linker > > won't know which symbol version to bind to. > > > > Following your example, something along these lines: > > 4) Edit lib//library_version.map to add a DPDK_1.9 node that is a > > successor to DPDK_1.8, in which foo is exported as a global symbol > > DPDK_1.9 { > > global: foo; > > } DPDK_1.8; > > > > 5) rename new function int foo(int index) to > > int __vsym foo_v19(int index) > > > > 6) Add this macro immediately below the function: > > DEFAULT_SYMBOL(foo, _v19, 1.9); > > > > #define DEFAULT_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ", "SA(b)"@@DPDK_"SA(v)) > > > > You're spot on (though the macro that I created in rte_compat.h is > VERSION_SYMBOL). > > When you use a version script to create a DSO, at link time, the appropriate > version is appended to the symbol name (you can see it with objdump -t in a > linked binary). If you want to update the symbol to a new version, you do what > I documented in the header file (though now that I re-read it, it could be more > clear. Hows this for a change to the documentation: > > To make a new version of a function foo in a DSO: > > 1) Edit lib//library_version.map to add a DPDK_1.8 node, in which > foo is exported as a global symbol > > 2) rename the existing function int foo(char *string) to > int __vsym foo_v18(char *string) > > 3) Add this macro immediately below the function > VERSION_SYMBOL(foo, _v18, 1.8); > > 4) Implement the new version of the function foo. > > > Those steps above will create two symbols in your export table of the DSO: > > foo@@DPDK_1.8 > foo@@DPDK_1.9 > > Any application linked against this DSO will link against the latest version > (DPDK_1.9). But if you look at the symbols referenced in a binary linked > against an older version of the same DSO, you'll note they explicitly look for > foo@@DPDK_1.8. Thats how we provide backwards compatibility > > Does that answer your questions? > > Neil > Correct me if I am wrong but when we define multiple versions of a symbol we need to specify a default one. As an example, if we were to have three versions of foo the export table of the DSO should look something like this: foo@VER_1.0 foo@VER_1.1 foo@@VER_1.2 In the above example, foo VER_1.2 is the default one and is indicated by having double @. Effectively we would need two macros VERSION_SYMBOL and DEFAULT_VERSION_SYMBOL (maybe this name is more appropriate). #define VERSION_SYMBOL(b, e, v) \ __asm__(".symver " SA(b) SA(e) ", "SA(b)"@DPDK_"SA(v)) #define DEFAULT_VERSION_SYMBOL(b, e, v) \ __asm__(".symver " SA(b) SA(e) ", "SA(b)"@@DPDK_"SA(v)) Following on the example, we should have something like: int __vsym foo_v18(char *string) {...} VERSION_SYMBOL(foo, _v18, 1.8); int __vsym foo_v19(int index) {...} DEFAULT_VERSION_SYMBOL(foo, _v19, 1.9); The DSO export table would have the following symbols: foo@DPDK_1.8 foo@@DPDK_1.9 Old binaries linked against DPDK 1.8 would have references to: foo@@DPDK_1.8 and new binaries linked against DPDK 1.9 would have to: foo@@DPDK_1.9 Sergio > > > +#else > > > +/* > > > + * No symbol versioning in use > > > + */ > > > +#define VERSION_SYMBOL(b, e, v) > > > +#define __vsym > > > + > > > +/* > > > + * RTE_SYMBOL_VERSIONING > > > + */ > > > +#endif > > > + > > > + > > > +#endif /* _RTE_COMPAT_H_ */ > > > diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk > > > index f458258..82ac309 100644 > > > --- a/mk/rte.lib.mk > > > +++ b/mk/rte.lib.mk > > > @@ -40,8 +40,12 @@ VPATH += $(SRCDIR) > > > > > > ifeq ($(RTE_BUILD_SHARED_LIB),y) > > > LIB := $(patsubst %.a,%.so,$(LIB)) > > > + > > > +CPU_LDFLAGS += --version-script=$(EXPORT_MAP) > > > + > > > endif > > > > > > + > > > _BUILD = $(LIB) > > > _INSTALL = $(INSTALL-FILES-y) $(SYMLINK-FILES-y) $(RTE_OUTPUT)/lib/$(LIB) > > > _CLEAN = doclean > > > @@ -160,7 +164,9 @@ endif > > > $(RTE_OUTPUT)/lib/$(LIB): $(LIB) > > > @echo " INSTALL-LIB $(LIB)" > > > @[ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib > > > +ifneq ($(LIB),) > > > $(Q)cp -f $(LIB) $(RTE_OUTPUT)/lib > > > +endif > > > > > > # > > > # Clean all generated files > > > -- > > > 1.9.3 > > > > > > > Sergio > >