| Message ID | 1419349913-21674-1-git-send-email-nhorman@tuxdriver.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Dec 23, 2014 at 10:51:50AM -0500, Neil Horman wrote: > Add initial pass header files to support symbol versioning. > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > CC: Thomas Monjalon <thomas.monjalon@6wind.com> > CC: "Richardson, Bruce" <bruce.richardson@intel.com> > CC: "Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com> > > Change Notes: > V2) > Moved ifeq to _INSTALL target > > V3) > Undo V2 changes and make librte_compat use the rte.install.mk file > instead > --- > lib/Makefile | 1 + > lib/librte_compat/Makefile | 38 +++++++++++++++++ > lib/librte_compat/rte_compat.h | 96 ++++++++++++++++++++++++++++++++++++++++++ > mk/rte.lib.mk | 4 ++ > 4 files changed, 139 insertions(+) > create mode 100644 lib/librte_compat/Makefile > create mode 100644 lib/librte_compat/rte_compat.h > Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Hi Neil, 2014-12-23 10:51, Neil Horman: > Add initial pass header files to support symbol versioning. [...] > +# Copyright(c) 2010-2014 Neil Horman <nhorman@tuxdriver.com> Why these dates? > +# All rights reserved. I think this line is not required anymore: http://en.wikipedia.org/wiki/All_rights_reserved [...] > +#ifndef _RTE_COMPAT_H_ > +#define _RTE_COMPAT_H_ Why using underscores? I think it's reserved: http://en.wikipedia.org/wiki/Include_guard#Use_of_.23include_guards > +#define SA(x) #x It should be prefixed. But it's better to use RTE_STR. > +#ifdef RTE_BUILD_SHARED_LIB > + > +/* > + * 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 Should be "To support that," with uppercase and comma. > + * VERSION_SYMBOL macros are created. They, in conjunction with the > + * <library>_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>/library_version.map to add a DPDK_1.(X+1) 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 a new version of foo. > + * char foo(int value, int otherval) { ...} > + * > + * 5) Mark the newest version as the default version > + * BIND_DEFAULT_SYMBOL(foo, 1.9); > + * > + */ Thanks for this good tutorial. > +#define VERSION_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ", "SA(b)"@DPDK_"SA(v)) > +#define BASE_SYMBOL(b, n) __asm__(".symver " SA(n) ", "SA(b)"@") > +#define BIND_DEFAULT_SYMBOL(b, v) __asm__(".symver " SA(b) ", "SA(b)"@@DPDK_"SA(v)) > +#define __vsym __attribute__((used)) OK. It would be simpler to read if b, e, v and n were formally defined in a comment. > +#else [...] > +/* > + * RTE_BUILD_SHARED_LIB > + */ This type of comment is strange. It makes me think that we are in the case RTE_BUILD_SHARED_LIB=y > +#endif [...] > + > +CPU_LDFLAGS += --version-script=$(EXPORT_MAP) Why this variable name? VERSION_SCRIPT or VERSION_MAP seems more appropriate. > + > endif > > + Why this newline? > _BUILD = $(LIB)
On Wed, Jan 14, 2015 at 04:25:19PM +0100, Thomas Monjalon wrote: > Hi Neil, > > 2014-12-23 10:51, Neil Horman: > > Add initial pass header files to support symbol versioning. > > [...] > > > +# Copyright(c) 2010-2014 Neil Horman <nhorman@tuxdriver.com> > > Why these dates? > Because I copied the Makefile from librte_acl, and modified the name but not the dates. > > +# All rights reserved. > > I think this line is not required anymore: > http://en.wikipedia.org/wiki/All_rights_reserved > Hmm, apparently so. However, since it exists in every other copyright notice in the tree, I'd just as soon keep this language consistent, and make a tree wide change in a separate patch if the consensus is to do so. > [...] > > > +#ifndef _RTE_COMPAT_H_ > > +#define _RTE_COMPAT_H_ > > Why using underscores? > I think it's reserved: > http://en.wikipedia.org/wiki/Include_guard#Use_of_.23include_guards > Its reserved for the implementation, and must not be used by a user using the header file. Its ok, and is common practice. See every other symlinked header file in the DPDK. > > +#define SA(x) #x > > It should be prefixed. But it's better to use RTE_STR. > very well > > +#ifdef RTE_BUILD_SHARED_LIB > > + > > +/* > > + * 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 > > Should be "To support that," with uppercase and comma. > yup > > + * VERSION_SYMBOL macros are created. They, in conjunction with the > > + * <library>_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>/library_version.map to add a DPDK_1.(X+1) 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 a new version of foo. > > + * char foo(int value, int otherval) { ...} > > + * > > + * 5) Mark the newest version as the default version > > + * BIND_DEFAULT_SYMBOL(foo, 1.9); > > + * > > + */ > > Thanks for this good tutorial. > > > +#define VERSION_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ", "SA(b)"@DPDK_"SA(v)) > > +#define BASE_SYMBOL(b, n) __asm__(".symver " SA(n) ", "SA(b)"@") > > +#define BIND_DEFAULT_SYMBOL(b, v) __asm__(".symver " SA(b) ", "SA(b)"@@DPDK_"SA(v)) > > +#define __vsym __attribute__((used)) > > OK. It would be simpler to read if b, e, v and n were formally defined in a comment. > > > +#else > [...] > > +/* > > + * RTE_BUILD_SHARED_LIB > > + */ > > This type of comment is strange. It makes me think that we are in the case > RTE_BUILD_SHARED_LIB=y > > > +#endif > > [...] > > > + > > +CPU_LDFLAGS += --version-script=$(EXPORT_MAP) > > Why this variable name? VERSION_SCRIPT or VERSION_MAP seems more appropriate. > > > + > > endif > > > > + > > Why this newline? > > > _BUILD = $(LIB) > > -- > Thomas >
diff --git a/lib/Makefile b/lib/Makefile index 0ffc982..d617d81 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -31,6 +31,7 @@ include $(RTE_SDK)/mk/rte.vars.mk +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..46d5905 --- /dev/null +++ b/lib/librte_compat/Makefile @@ -0,0 +1,38 @@ +# BSD LICENSE +# +# Copyright(c) 2010-2014 Neil Horman <nhorman@tuxdriver.com> +# 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.install.mk diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h new file mode 100644 index 0000000..d99e362 --- /dev/null +++ b/lib/librte_compat/rte_compat.h @@ -0,0 +1,96 @@ +/*- + * BSD LICENSE + * + * Copyright(c) 2010-2014 Neil Horman <nhorman@tuxdriver.com>. + * 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_BUILD_SHARED_LIB + +/* + * 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 + * <library>_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>/library_version.map to add a DPDK_1.(X+1) 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 a new version of foo. + * char foo(int value, int otherval) { ...} + * + * 5) Mark the newest version as the default version + * BIND_DEFAULT_SYMBOL(foo, 1.9); + * + */ +#define VERSION_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ", "SA(b)"@DPDK_"SA(v)) +#define BASE_SYMBOL(b, n) __asm__(".symver " SA(n) ", "SA(b)"@") +#define BIND_DEFAULT_SYMBOL(b, v) __asm__(".symver " SA(b) ", "SA(b)"@@DPDK_"SA(v)) +#define __vsym __attribute__((used)) + +#else +/* + * No symbol versioning in use + */ +#define VERSION_SYMBOL(b, e, v) +#define __vsym +#define BASE_SYMBOL(b, n) +#define BIND_DEFAULT_SYMBOL(b, v) + +/* + * RTE_BUILD_SHARED_LIB + */ +#endif + + +#endif /* _RTE_COMPAT_H_ */ diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk index 81bf8e1..75e3652 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
Add initial pass header files to support symbol versioning. Signed-off-by: Neil Horman <nhorman@tuxdriver.com> CC: Thomas Monjalon <thomas.monjalon@6wind.com> CC: "Richardson, Bruce" <bruce.richardson@intel.com> CC: "Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com> Change Notes: V2) Moved ifeq to _INSTALL target V3) Undo V2 changes and make librte_compat use the rte.install.mk file instead --- lib/Makefile | 1 + lib/librte_compat/Makefile | 38 +++++++++++++++++ lib/librte_compat/rte_compat.h | 96 ++++++++++++++++++++++++++++++++++++++++++ mk/rte.lib.mk | 4 ++ 4 files changed, 139 insertions(+) create mode 100644 lib/librte_compat/Makefile create mode 100644 lib/librte_compat/rte_compat.h