* [dpdk-dev] [PATCH 0/2] Improve function versioning meson support
@ 2019-09-27 19:49 Bruce Richardson
2019-09-27 19:49 ` [dpdk-dev] [PATCH 1/2] eal: split compat header file Bruce Richardson
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Bruce Richardson @ 2019-09-27 19:49 UTC (permalink / raw)
To: dev
Cc: Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman,
Bruce Richardson
Adding support for LTO has exposed some issues with how the functions
versioning was supported by meson, which was always set to build both
shared and static libraries.
For plain C code, so long as the -fPIC compiler flag was passed, the
output is identical whether or not the code is to be included in a
static library or a dynamic one. Unfortunately, when using function
versioning that no longer held as different macros were used for the
versioned functions depending on which type of build it was. This means
that any files that use versioning need to be built twice, with
different defines in each case.
While the trivial solution here is just to rebuild everything twice,
that involves a lot of unnecessary work when building DPDK. A better
option is to identify those files or components which need multiple
builds and rebuild only those. To do this, we add a new meson.build
setting for libraries "use_function_versioning" and when that is set, we
rebuild all source files twice, initially for static library and then
with -DRTE_BUILD_SHARED_LIB for the shared library.
If the flag is not set, then the static versioning setting only is used,
which could lead to the build succeeding but later causing problems. To
avoid that, we add a new define which must be set when the versioning
header is included. This addition while solving 1 problem raises 2
other, more minor problems:
* what to do with make builds? since make only builds one library type,
we can just always define the new value.
* what about files that include rte_compat.h for the macro for
"experimental"? To solve this, we can split compat.h in two, since the
versioning macro should be internal only to DPDK (as no public header
should expose anything but the latest APIs), while the experimental
macros are primarily for public use.
Bruce Richardson (2):
eal: split compat header file
build: support building ABI versioned files twice
config/common_base | 1 +
config/rte_config.h | 3 ---
doc/api/doxy-api-index.md | 3 ++-
doc/guides/contributing/coding_style.rst | 7 ++++++
doc/guides/contributing/versioning.rst | 4 ++--
lib/librte_distributor/meson.build | 1 +
lib/librte_distributor/rte_distributor.c | 2 +-
lib/librte_distributor/rte_distributor_v20.c | 2 +-
lib/librte_eal/common/Makefile | 1 +
...rte_compat.h => rte_function_versioning.h} | 23 ++++++-------------
lib/librte_lpm/meson.build | 1 +
lib/librte_lpm/rte_lpm.c | 1 +
lib/librte_lpm/rte_lpm.h | 1 -
lib/librte_lpm/rte_lpm6.c | 1 +
lib/librte_timer/meson.build | 1 +
lib/librte_timer/rte_timer.c | 2 +-
lib/meson.build | 16 ++++++++++---
17 files changed, 41 insertions(+), 29 deletions(-)
rename lib/librte_eal/common/include/{rte_compat.h => rte_function_versioning.h} (89%)
--
2.21.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [dpdk-dev] [PATCH 1/2] eal: split compat header file 2019-09-27 19:49 [dpdk-dev] [PATCH 0/2] Improve function versioning meson support Bruce Richardson @ 2019-09-27 19:49 ` Bruce Richardson 2019-09-27 20:48 ` Bruce Richardson 2019-09-27 19:49 ` [dpdk-dev] [PATCH 2/2] build: support building ABI versioned files twice Bruce Richardson ` (3 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Bruce Richardson @ 2019-09-27 19:49 UTC (permalink / raw) To: dev Cc: Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman, Bruce Richardson The compat.h header file provided macros for two purposes: 1. it provided the macros for marking functions as rte_experimental 2. it provided the macros for doing function versioning Although these were in the same file, #1 is something that is for use by public header files, which #2 is for internal use only. Therefore, we can split these into two headers, keeping #1 in rte_compat.h and #2 in a new file rte_function_versioning.h. For "make" builds, since internal objects pick up the headers from the "include/" folder, we need to add the new header to the installation list, but for "meson" builds it does not need to be installed as it's not for public use. The rework also serves to allow the use of the function versioning macros to files that actually need them, so the use of experimental functions does not need including of the versioning code. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- doc/api/doxy-api-index.md | 3 ++- doc/guides/contributing/versioning.rst | 4 ++-- lib/librte_distributor/rte_distributor.c | 2 +- lib/librte_distributor/rte_distributor_v20.c | 2 +- lib/librte_eal/common/Makefile | 1 + ...rte_compat.h => rte_function_versioning.h} | 19 +++---------------- lib/librte_lpm/rte_lpm.c | 1 + lib/librte_lpm/rte_lpm.h | 1 - lib/librte_lpm/rte_lpm6.c | 1 + lib/librte_timer/rte_timer.c | 2 +- 10 files changed, 13 insertions(+), 23 deletions(-) rename lib/librte_eal/common/include/{rte_compat.h => rte_function_versioning.h} (89%) diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index 6c2d888ee..9acf36ba1 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -171,5 +171,6 @@ The public API headers are grouped by topics: - **misc**: [EAL config] (@ref rte_eal.h), [common] (@ref rte_common.h), - [ABI compat] (@ref rte_compat.h), + [experimental APIs] (@ref rte_compat.h), + [ABI versioning] (@ref rte_function_versioning.h), [version] (@ref rte_version.h) diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst index 3ab2c4346..64984c54e 100644 --- a/doc/guides/contributing/versioning.rst +++ b/doc/guides/contributing/versioning.rst @@ -206,7 +206,7 @@ functionality or behavior. When that occurs, it is desirable to allow for backward compatibility for a time with older binaries that are dynamically linked to the DPDK. -To support backward compatibility the ``rte_compat.h`` +To support backward compatibility the ``rte_function_versioning.h`` header file provides macros to use when updating exported functions. These macros are used in conjunction with the ``rte_<library>_version.map`` file for a given library to allow multiple versions of a symbol to exist in a shared @@ -362,7 +362,7 @@ the function, we add this line of code VERSION_SYMBOL(rte_acl_create, _v20, 2.0); -Remembering to also add the rte_compat.h header to the requisite c file where +Remembering to also add the rte_function_versioning.h header to the requisite c file where these changes are being made. The above macro instructs the linker to create a new symbol ``rte_acl_create@DPDK_2.0``, which matches the symbol created in older builds, but now points to the above newly named function. We have now mapped diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c index 21eb1fb0a..6d1e971a9 100644 --- a/lib/librte_distributor/rte_distributor.c +++ b/lib/librte_distributor/rte_distributor.c @@ -8,7 +8,7 @@ #include <rte_mbuf.h> #include <rte_memory.h> #include <rte_cycles.h> -#include <rte_compat.h> +#include <rte_function_versioning.h> #include <rte_memzone.h> #include <rte_errno.h> #include <rte_string_fns.h> diff --git a/lib/librte_distributor/rte_distributor_v20.c b/lib/librte_distributor/rte_distributor_v20.c index cdc0969a8..64c611fa9 100644 --- a/lib/librte_distributor/rte_distributor_v20.c +++ b/lib/librte_distributor/rte_distributor_v20.c @@ -9,7 +9,7 @@ #include <rte_memory.h> #include <rte_memzone.h> #include <rte_errno.h> -#include <rte_compat.h> +#include <rte_function_versioning.h> #include <rte_string_fns.h> #include <rte_eal_memconfig.h> #include <rte_pause.h> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile index a00d4fcad..d70f84fd7 100644 --- a/lib/librte_eal/common/Makefile +++ b/lib/librte_eal/common/Makefile @@ -4,6 +4,7 @@ include $(RTE_SDK)/mk/rte.vars.mk INC := rte_branch_prediction.h rte_common.h rte_compat.h +INC += rte_function_versioning.h INC += rte_debug.h rte_eal.h rte_eal_interrupts.h INC += rte_errno.h rte_launch.h rte_lcore.h INC += rte_log.h rte_memory.h rte_memzone.h diff --git a/lib/librte_eal/common/include/rte_compat.h b/lib/librte_eal/common/include/rte_function_versioning.h similarity index 89% rename from lib/librte_eal/common/include/rte_compat.h rename to lib/librte_eal/common/include/rte_function_versioning.h index 92ff28faf..ce963d4b1 100644 --- a/lib/librte_eal/common/include/rte_compat.h +++ b/lib/librte_eal/common/include/rte_function_versioning.h @@ -3,8 +3,8 @@ * All rights reserved. */ -#ifndef _RTE_COMPAT_H_ -#define _RTE_COMPAT_H_ +#ifndef _RTE_FUNCTION_VERSIONING_H_ +#define _RTE_FUNCTION_VERSIONING_H_ #include <rte_common.h> #ifdef RTE_BUILD_SHARED_LIB @@ -76,17 +76,4 @@ */ #endif -#ifndef ALLOW_EXPERIMENTAL_API - -#define __rte_experimental \ -__attribute__((deprecated("Symbol is not yet part of stable ABI"), \ -section(".text.experimental"))) - -#else - -#define __rte_experimental \ -__attribute__((section(".text.experimental"))) - -#endif - -#endif /* _RTE_COMPAT_H_ */ +#endif /* _RTE_FUNCTION_VERSIONING_H_ */ diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index 3a929a1b1..c96395e26 100644 --- a/lib/librte_lpm/rte_lpm.c +++ b/lib/librte_lpm/rte_lpm.c @@ -22,6 +22,7 @@ #include <rte_rwlock.h> #include <rte_spinlock.h> #include <rte_tailq.h> +#include <rte_function_versioning.h> #include "rte_lpm.h" diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index 906ec4483..26303e628 100644 --- a/lib/librte_lpm/rte_lpm.h +++ b/lib/librte_lpm/rte_lpm.h @@ -20,7 +20,6 @@ #include <rte_memory.h> #include <rte_common.h> #include <rte_vect.h> -#include <rte_compat.h> #ifdef __cplusplus extern "C" { diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c index 9b8aeb972..e20f82460 100644 --- a/lib/librte_lpm/rte_lpm6.c +++ b/lib/librte_lpm/rte_lpm6.c @@ -25,6 +25,7 @@ #include <assert.h> #include <rte_jhash.h> #include <rte_tailq.h> +#include <rte_function_versioning.h> #include "rte_lpm6.h" diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index bdcf05d06..3834c9473 100644 --- a/lib/librte_timer/rte_timer.c +++ b/lib/librte_timer/rte_timer.c @@ -25,8 +25,8 @@ #include <rte_pause.h> #include <rte_memzone.h> #include <rte_malloc.h> -#include <rte_compat.h> #include <rte_errno.h> +#include <rte_function_versioning.h> #include "rte_timer.h" -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] eal: split compat header file 2019-09-27 19:49 ` [dpdk-dev] [PATCH 1/2] eal: split compat header file Bruce Richardson @ 2019-09-27 20:48 ` Bruce Richardson 0 siblings, 0 replies; 17+ messages in thread From: Bruce Richardson @ 2019-09-27 20:48 UTC (permalink / raw) To: dev; +Cc: Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman On Fri, Sep 27, 2019 at 08:49:31PM +0100, Bruce Richardson wrote: > The compat.h header file provided macros for two purposes: > 1. it provided the macros for marking functions as rte_experimental > 2. it provided the macros for doing function versioning > > Although these were in the same file, #1 is something that is for use by > public header files, which #2 is for internal use only. Therefore, we can > split these into two headers, keeping #1 in rte_compat.h and #2 in a new > file rte_function_versioning.h. For "make" builds, since internal objects > pick up the headers from the "include/" folder, we need to add the new > header to the installation list, but for "meson" builds it does not need to > be installed as it's not for public use. > > The rework also serves to allow the use of the function versioning macros > to files that actually need them, so the use of experimental functions does > not need including of the versioning code. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > doc/api/doxy-api-index.md | 3 ++- > doc/guides/contributing/versioning.rst | 4 ++-- > lib/librte_distributor/rte_distributor.c | 2 +- > lib/librte_distributor/rte_distributor_v20.c | 2 +- > lib/librte_eal/common/Makefile | 1 + > ...rte_compat.h => rte_function_versioning.h} | 19 +++---------------- > lib/librte_lpm/rte_lpm.c | 1 + > lib/librte_lpm/rte_lpm.h | 1 - > lib/librte_lpm/rte_lpm6.c | 1 + > lib/librte_timer/rte_timer.c | 2 +- > 10 files changed, 13 insertions(+), 23 deletions(-) > rename lib/librte_eal/common/include/{rte_compat.h => rte_function_versioning.h} (89%) > Apologies, but one of the parts of the split file somehow missed making the patch. I'll do a v2 soon. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH 2/2] build: support building ABI versioned files twice 2019-09-27 19:49 [dpdk-dev] [PATCH 0/2] Improve function versioning meson support Bruce Richardson 2019-09-27 19:49 ` [dpdk-dev] [PATCH 1/2] eal: split compat header file Bruce Richardson @ 2019-09-27 19:49 ` Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 0/2] Improve function versioning meson support Bruce Richardson ` (2 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Bruce Richardson @ 2019-09-27 19:49 UTC (permalink / raw) To: dev Cc: Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman, Bruce Richardson Any file with ABI versioned functions needs different macros for shared and static builds, so we need to accomodate that. Rather than building everything twice, we just flag to the build system which libraries need that handling, by setting use_function_versioning in the meson.build files. To ensure we don't get silent errors at build time due to this meson flag being missed, we add an explicit error to the function versioning header file if a known C macro is not defined. Since "make" builds always only build one of shared or static libraries, this define can be always set, and so is added to the common_base file. For meson, the build flag - and therefore the C define - is set for the three libraries that need the function versioning: "distributor", "lpm" and "timer". Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- config/common_base | 1 + config/rte_config.h | 3 --- doc/guides/contributing/coding_style.rst | 7 +++++++ lib/librte_distributor/meson.build | 1 + .../common/include/rte_function_versioning.h | 4 ++++ lib/librte_lpm/meson.build | 1 + lib/librte_timer/meson.build | 1 + lib/meson.build | 16 +++++++++++++--- 8 files changed, 28 insertions(+), 6 deletions(-) diff --git a/config/common_base b/config/common_base index 8ef75c203..655258a97 100644 --- a/config/common_base +++ b/config/common_base @@ -111,6 +111,7 @@ CONFIG_RTE_MAX_VFIO_CONTAINERS=64 CONFIG_RTE_MALLOC_DEBUG=n CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n CONFIG_RTE_USE_LIBBSD=n +CONFIG_RTE_USE_FUNCTION_VERSIONING=y # # Recognize/ignore the AVX/AVX512 CPU flags for performance/power testing. diff --git a/config/rte_config.h b/config/rte_config.h index 0bbbe274f..b63a2fdea 100644 --- a/config/rte_config.h +++ b/config/rte_config.h @@ -31,9 +31,6 @@ /****** library defines ********/ -/* compat defines */ -#define RTE_BUILD_SHARED_LIB - /* EAL defines */ #define RTE_MAX_HEAPS 32 #define RTE_MAX_MEMSEG_LISTS 128 diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst index 449b33494..e95a1a2be 100644 --- a/doc/guides/contributing/coding_style.rst +++ b/doc/guides/contributing/coding_style.rst @@ -948,6 +948,13 @@ reason built. For missing dependencies this should be of the form ``'missing dependency, "libname"'``. +use_function_versioning + **Default Value = false**. + Specifies if the library in question has ABI versioned functions. If it + has, this value should be set to ensure that the C files are compiled + twice with suitable parameters for each of shared or static library + builds. + version **Default Value = 1**. Specifies the ABI version of the library, and is used as the major diff --git a/lib/librte_distributor/meson.build b/lib/librte_distributor/meson.build index dba7e3b2a..5149f9bf5 100644 --- a/lib/librte_distributor/meson.build +++ b/lib/librte_distributor/meson.build @@ -9,3 +9,4 @@ else endif headers = files('rte_distributor.h') deps += ['mbuf'] +use_function_versioning = true diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h index ce963d4b1..55e88ffae 100644 --- a/lib/librte_eal/common/include/rte_function_versioning.h +++ b/lib/librte_eal/common/include/rte_function_versioning.h @@ -7,6 +7,10 @@ #define _RTE_FUNCTION_VERSIONING_H_ #include <rte_common.h> +#ifndef RTE_USE_FUNCTION_VERSIONING +#error Use of function versioning disabled, is "use_function_versioning=true" in meson.build? +#endif + #ifdef RTE_BUILD_SHARED_LIB /* diff --git a/lib/librte_lpm/meson.build b/lib/librte_lpm/meson.build index a5176d8ae..4e3920660 100644 --- a/lib/librte_lpm/meson.build +++ b/lib/librte_lpm/meson.build @@ -8,3 +8,4 @@ headers = files('rte_lpm.h', 'rte_lpm6.h') # without worrying about which architecture we actually need headers += files('rte_lpm_altivec.h', 'rte_lpm_neon.h', 'rte_lpm_sse.h') deps += ['hash'] +use_function_versioning = true diff --git a/lib/librte_timer/meson.build b/lib/librte_timer/meson.build index d3b828ce9..b7edfe2e7 100644 --- a/lib/librte_timer/meson.build +++ b/lib/librte_timer/meson.build @@ -4,3 +4,4 @@ sources = files('rte_timer.c') headers = files('rte_timer.h') allow_experimental_apis = true +use_function_versioning = true diff --git a/lib/meson.build b/lib/meson.build index e5ff83893..1b0ed767c 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -47,6 +47,7 @@ foreach l:libraries name = l version = 1 allow_experimental_apis = false + use_function_versioning = false sources = [] headers = [] includes = [] @@ -96,6 +97,9 @@ foreach l:libraries if allow_experimental_apis cflags += '-DALLOW_EXPERIMENTAL_API' endif + if use_function_versioning + cflags += '-DRTE_USE_FUNCTION_VERSIONING' + endif if get_option('per_library_versions') lib_version = '@0@.1'.format(version) @@ -117,9 +121,15 @@ foreach l:libraries include_directories: includes, dependencies: static_deps) - # then use pre-build objects to build shared lib - sources = [] - objs += static_lib.extract_all_objects(recursive: false) + if not use_function_versioning + # use pre-build objects to build shared lib + sources = [] + objs += static_lib.extract_all_objects(recursive: false) + else + # for compat we need to rebuild with + # RTE_BUILD_SHARED_LIB defined + cflags += '-DRTE_BUILD_SHARED_LIB' + endif version_map = '@0@/@1@/rte_@2@_version.map'.format( meson.current_source_dir(), dir_name, name) implib = dir_name + '.dll.a' -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v2 0/2] Improve function versioning meson support 2019-09-27 19:49 [dpdk-dev] [PATCH 0/2] Improve function versioning meson support Bruce Richardson 2019-09-27 19:49 ` [dpdk-dev] [PATCH 1/2] eal: split compat header file Bruce Richardson 2019-09-27 19:49 ` [dpdk-dev] [PATCH 2/2] build: support building ABI versioned files twice Bruce Richardson @ 2019-09-27 20:59 ` Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 1/2] eal: split compat header file Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice Bruce Richardson 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Bruce Richardson 2019-10-09 22:59 ` [dpdk-dev] [PATCH " Stephen Hemminger 4 siblings, 2 replies; 17+ messages in thread From: Bruce Richardson @ 2019-09-27 20:59 UTC (permalink / raw) To: dev Cc: Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman, bluca, Bruce Richardson Adding support for LTO has exposed some issues with how the functions versioning was supported by meson, which was always set to build both shared and static libraries. For plain C code, so long as the -fPIC compiler flag was passed, the output is identical whether or not the code is to be included in a static library or a dynamic one. Unfortunately, when using function versioning that no longer held as different macros were used for the versioned functions depending on which type of build it was. This means that any files that use versioning need to be built twice, with different defines in each case. While the trivial solution here is just to rebuild everything twice, that involves a lot of unnecessary work when building DPDK. A better option is to identify those files or components which need multiple builds and rebuild only those. To do this, we add a new meson.build setting for libraries "use_function_versioning" and when that is set, we rebuild all source files twice, initially for static library and then with -DRTE_BUILD_SHARED_LIB for the shared library. If the flag is not set, then the static versioning setting only is used, which could lead to the build succeeding but later causing problems. To avoid that, we add a new define which must be set when the versioning header is included. This addition while solving 1 problem raises 2 other, more minor problems: * what to do with make builds? since make only builds one library type, we can just always define the new value. * what about files that include rte_compat.h for the macro for "experimental"? To solve this, we can split compat.h in two, since the versioning macro should be internal only to DPDK (as no public header should expose anything but the latest APIs), while the experimental macros are primarily for public use. V2: added in file that missed a "git add" when doing V1 Bruce Richardson (2): eal: split compat header file build: support building ABI versioned files twice config/common_base | 1 + config/rte_config.h | 3 - doc/api/doxy-api-index.md | 3 +- doc/guides/contributing/coding_style.rst | 7 ++ doc/guides/contributing/versioning.rst | 4 +- lib/librte_distributor/meson.build | 1 + lib/librte_distributor/rte_distributor.c | 2 +- lib/librte_distributor/rte_distributor_v20.c | 2 +- lib/librte_eal/common/Makefile | 1 + lib/librte_eal/common/include/rte_compat.h | 70 ---------------- .../common/include/rte_function_versioning.h | 83 +++++++++++++++++++ lib/librte_lpm/meson.build | 1 + lib/librte_lpm/rte_lpm.c | 1 + lib/librte_lpm/rte_lpm.h | 1 - lib/librte_lpm/rte_lpm6.c | 1 + lib/librte_timer/meson.build | 1 + lib/librte_timer/rte_timer.c | 2 +- lib/meson.build | 16 +++- 18 files changed, 117 insertions(+), 83 deletions(-) create mode 100644 lib/librte_eal/common/include/rte_function_versioning.h -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] eal: split compat header file 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 0/2] Improve function versioning meson support Bruce Richardson @ 2019-09-27 20:59 ` Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice Bruce Richardson 1 sibling, 0 replies; 17+ messages in thread From: Bruce Richardson @ 2019-09-27 20:59 UTC (permalink / raw) To: dev Cc: Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman, bluca, Bruce Richardson The compat.h header file provided macros for two purposes: 1. it provided the macros for marking functions as rte_experimental 2. it provided the macros for doing function versioning Although these were in the same file, #1 is something that is for use by public header files, which #2 is for internal use only. Therefore, we can split these into two headers, keeping #1 in rte_compat.h and #2 in a new file rte_function_versioning.h. For "make" builds, since internal objects pick up the headers from the "include/" folder, we need to add the new header to the installation list, but for "meson" builds it does not need to be installed as it's not for public use. The rework also serves to allow the use of the function versioning macros to files that actually need them, so the use of experimental functions does not need including of the versioning code. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- V2: added in missing rte_compat.h file --- doc/api/doxy-api-index.md | 3 +- doc/guides/contributing/versioning.rst | 4 +- lib/librte_distributor/rte_distributor.c | 2 +- lib/librte_distributor/rte_distributor_v20.c | 2 +- lib/librte_eal/common/Makefile | 1 + lib/librte_eal/common/include/rte_compat.h | 70 ---------------- .../common/include/rte_function_versioning.h | 79 +++++++++++++++++++ lib/librte_lpm/rte_lpm.c | 1 + lib/librte_lpm/rte_lpm.h | 1 - lib/librte_lpm/rte_lpm6.c | 1 + lib/librte_timer/rte_timer.c | 2 +- 11 files changed, 89 insertions(+), 77 deletions(-) create mode 100644 lib/librte_eal/common/include/rte_function_versioning.h diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index 6c2d888ee..9acf36ba1 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -171,5 +171,6 @@ The public API headers are grouped by topics: - **misc**: [EAL config] (@ref rte_eal.h), [common] (@ref rte_common.h), - [ABI compat] (@ref rte_compat.h), + [experimental APIs] (@ref rte_compat.h), + [ABI versioning] (@ref rte_function_versioning.h), [version] (@ref rte_version.h) diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst index 3ab2c4346..64984c54e 100644 --- a/doc/guides/contributing/versioning.rst +++ b/doc/guides/contributing/versioning.rst @@ -206,7 +206,7 @@ functionality or behavior. When that occurs, it is desirable to allow for backward compatibility for a time with older binaries that are dynamically linked to the DPDK. -To support backward compatibility the ``rte_compat.h`` +To support backward compatibility the ``rte_function_versioning.h`` header file provides macros to use when updating exported functions. These macros are used in conjunction with the ``rte_<library>_version.map`` file for a given library to allow multiple versions of a symbol to exist in a shared @@ -362,7 +362,7 @@ the function, we add this line of code VERSION_SYMBOL(rte_acl_create, _v20, 2.0); -Remembering to also add the rte_compat.h header to the requisite c file where +Remembering to also add the rte_function_versioning.h header to the requisite c file where these changes are being made. The above macro instructs the linker to create a new symbol ``rte_acl_create@DPDK_2.0``, which matches the symbol created in older builds, but now points to the above newly named function. We have now mapped diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c index 21eb1fb0a..6d1e971a9 100644 --- a/lib/librte_distributor/rte_distributor.c +++ b/lib/librte_distributor/rte_distributor.c @@ -8,7 +8,7 @@ #include <rte_mbuf.h> #include <rte_memory.h> #include <rte_cycles.h> -#include <rte_compat.h> +#include <rte_function_versioning.h> #include <rte_memzone.h> #include <rte_errno.h> #include <rte_string_fns.h> diff --git a/lib/librte_distributor/rte_distributor_v20.c b/lib/librte_distributor/rte_distributor_v20.c index cdc0969a8..64c611fa9 100644 --- a/lib/librte_distributor/rte_distributor_v20.c +++ b/lib/librte_distributor/rte_distributor_v20.c @@ -9,7 +9,7 @@ #include <rte_memory.h> #include <rte_memzone.h> #include <rte_errno.h> -#include <rte_compat.h> +#include <rte_function_versioning.h> #include <rte_string_fns.h> #include <rte_eal_memconfig.h> #include <rte_pause.h> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile index a00d4fcad..d70f84fd7 100644 --- a/lib/librte_eal/common/Makefile +++ b/lib/librte_eal/common/Makefile @@ -4,6 +4,7 @@ include $(RTE_SDK)/mk/rte.vars.mk INC := rte_branch_prediction.h rte_common.h rte_compat.h +INC += rte_function_versioning.h INC += rte_debug.h rte_eal.h rte_eal_interrupts.h INC += rte_errno.h rte_launch.h rte_lcore.h INC += rte_log.h rte_memory.h rte_memzone.h diff --git a/lib/librte_eal/common/include/rte_compat.h b/lib/librte_eal/common/include/rte_compat.h index 92ff28faf..3eb33784b 100644 --- a/lib/librte_eal/common/include/rte_compat.h +++ b/lib/librte_eal/common/include/rte_compat.h @@ -5,76 +5,6 @@ #ifndef _RTE_COMPAT_H_ #define _RTE_COMPAT_H_ -#include <rte_common.h> - -#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 accommodate - * new functionality, behavior, etc. When that occurs, it is desirable 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. - * - * Refer to the guidelines document in the docs subdirectory for details on the - * use of these macros - */ - -/* - * Macro Parameters: - * b - function base name - * e - function version extension, to be concatenated with base name - * n - function symbol version string to be applied - * f - function prototype - * p - full function symbol name - */ - -/* - * VERSION_SYMBOL - * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the internal - * function name <b>_<e> - */ -#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n)) - -/* - * BIND_DEFAULT_SYMBOL - * Creates a symbol version entry instructing the linker to bind references to - * symbol <b> to the internal symbol <b>_<e> - */ -#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n)) -#define __vsym __attribute__((used)) - -/* - * MAP_STATIC_SYMBOL - * If a function has been bifurcated into multiple versions, none of which - * are defined as the exported symbol name in the map file, this macro can be - * used to alias a specific version of the symbol to its exported name. For - * example, if you have 2 versions of a function foo_v1 and foo_v2, where the - * former is mapped to foo@DPDK_1 and the latter is mapped to foo@DPDK_2 when - * building a shared library, this macro can be used to map either foo_v1 or - * foo_v2 to the symbol foo when building a static library, e.g.: - * MAP_STATIC_SYMBOL(void foo(), foo_v2); - */ -#define MAP_STATIC_SYMBOL(f, p) - -#else -/* - * No symbol versioning in use - */ -#define VERSION_SYMBOL(b, e, n) -#define __vsym -#define BIND_DEFAULT_SYMBOL(b, e, n) -#define MAP_STATIC_SYMBOL(f, p) f __attribute__((alias(RTE_STR(p)))) -/* - * RTE_BUILD_SHARED_LIB=n - */ -#endif #ifndef ALLOW_EXPERIMENTAL_API diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h new file mode 100644 index 000000000..ce963d4b1 --- /dev/null +++ b/lib/librte_eal/common/include/rte_function_versioning.h @@ -0,0 +1,79 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2015 Neil Horman <nhorman@tuxdriver.com>. + * All rights reserved. + */ + +#ifndef _RTE_FUNCTION_VERSIONING_H_ +#define _RTE_FUNCTION_VERSIONING_H_ +#include <rte_common.h> + +#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 accommodate + * new functionality, behavior, etc. When that occurs, it is desirable 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. + * + * Refer to the guidelines document in the docs subdirectory for details on the + * use of these macros + */ + +/* + * Macro Parameters: + * b - function base name + * e - function version extension, to be concatenated with base name + * n - function symbol version string to be applied + * f - function prototype + * p - full function symbol name + */ + +/* + * VERSION_SYMBOL + * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the internal + * function name <b>_<e> + */ +#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n)) + +/* + * BIND_DEFAULT_SYMBOL + * Creates a symbol version entry instructing the linker to bind references to + * symbol <b> to the internal symbol <b>_<e> + */ +#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n)) +#define __vsym __attribute__((used)) + +/* + * MAP_STATIC_SYMBOL + * If a function has been bifurcated into multiple versions, none of which + * are defined as the exported symbol name in the map file, this macro can be + * used to alias a specific version of the symbol to its exported name. For + * example, if you have 2 versions of a function foo_v1 and foo_v2, where the + * former is mapped to foo@DPDK_1 and the latter is mapped to foo@DPDK_2 when + * building a shared library, this macro can be used to map either foo_v1 or + * foo_v2 to the symbol foo when building a static library, e.g.: + * MAP_STATIC_SYMBOL(void foo(), foo_v2); + */ +#define MAP_STATIC_SYMBOL(f, p) + +#else +/* + * No symbol versioning in use + */ +#define VERSION_SYMBOL(b, e, n) +#define __vsym +#define BIND_DEFAULT_SYMBOL(b, e, n) +#define MAP_STATIC_SYMBOL(f, p) f __attribute__((alias(RTE_STR(p)))) +/* + * RTE_BUILD_SHARED_LIB=n + */ +#endif + +#endif /* _RTE_FUNCTION_VERSIONING_H_ */ diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index 3a929a1b1..c96395e26 100644 --- a/lib/librte_lpm/rte_lpm.c +++ b/lib/librte_lpm/rte_lpm.c @@ -22,6 +22,7 @@ #include <rte_rwlock.h> #include <rte_spinlock.h> #include <rte_tailq.h> +#include <rte_function_versioning.h> #include "rte_lpm.h" diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index 906ec4483..26303e628 100644 --- a/lib/librte_lpm/rte_lpm.h +++ b/lib/librte_lpm/rte_lpm.h @@ -20,7 +20,6 @@ #include <rte_memory.h> #include <rte_common.h> #include <rte_vect.h> -#include <rte_compat.h> #ifdef __cplusplus extern "C" { diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c index 9b8aeb972..e20f82460 100644 --- a/lib/librte_lpm/rte_lpm6.c +++ b/lib/librte_lpm/rte_lpm6.c @@ -25,6 +25,7 @@ #include <assert.h> #include <rte_jhash.h> #include <rte_tailq.h> +#include <rte_function_versioning.h> #include "rte_lpm6.h" diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index bdcf05d06..3834c9473 100644 --- a/lib/librte_timer/rte_timer.c +++ b/lib/librte_timer/rte_timer.c @@ -25,8 +25,8 @@ #include <rte_pause.h> #include <rte_memzone.h> #include <rte_malloc.h> -#include <rte_compat.h> #include <rte_errno.h> +#include <rte_function_versioning.h> #include "rte_timer.h" -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 0/2] Improve function versioning meson support Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 1/2] eal: split compat header file Bruce Richardson @ 2019-09-27 20:59 ` Bruce Richardson 2019-10-01 13:23 ` Andrzej Ostruszka 1 sibling, 1 reply; 17+ messages in thread From: Bruce Richardson @ 2019-09-27 20:59 UTC (permalink / raw) To: dev Cc: Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman, bluca, Bruce Richardson Any file with ABI versioned functions needs different macros for shared and static builds, so we need to accomodate that. Rather than building everything twice, we just flag to the build system which libraries need that handling, by setting use_function_versioning in the meson.build files. To ensure we don't get silent errors at build time due to this meson flag being missed, we add an explicit error to the function versioning header file if a known C macro is not defined. Since "make" builds always only build one of shared or static libraries, this define can be always set, and so is added to the common_base file. For meson, the build flag - and therefore the C define - is set for the three libraries that need the function versioning: "distributor", "lpm" and "timer". Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- config/common_base | 1 + config/rte_config.h | 3 --- doc/guides/contributing/coding_style.rst | 7 +++++++ lib/librte_distributor/meson.build | 1 + .../common/include/rte_function_versioning.h | 4 ++++ lib/librte_lpm/meson.build | 1 + lib/librte_timer/meson.build | 1 + lib/meson.build | 16 +++++++++++++--- 8 files changed, 28 insertions(+), 6 deletions(-) diff --git a/config/common_base b/config/common_base index 8ef75c203..655258a97 100644 --- a/config/common_base +++ b/config/common_base @@ -111,6 +111,7 @@ CONFIG_RTE_MAX_VFIO_CONTAINERS=64 CONFIG_RTE_MALLOC_DEBUG=n CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n CONFIG_RTE_USE_LIBBSD=n +CONFIG_RTE_USE_FUNCTION_VERSIONING=y # # Recognize/ignore the AVX/AVX512 CPU flags for performance/power testing. diff --git a/config/rte_config.h b/config/rte_config.h index 0bbbe274f..b63a2fdea 100644 --- a/config/rte_config.h +++ b/config/rte_config.h @@ -31,9 +31,6 @@ /****** library defines ********/ -/* compat defines */ -#define RTE_BUILD_SHARED_LIB - /* EAL defines */ #define RTE_MAX_HEAPS 32 #define RTE_MAX_MEMSEG_LISTS 128 diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst index 449b33494..e95a1a2be 100644 --- a/doc/guides/contributing/coding_style.rst +++ b/doc/guides/contributing/coding_style.rst @@ -948,6 +948,13 @@ reason built. For missing dependencies this should be of the form ``'missing dependency, "libname"'``. +use_function_versioning + **Default Value = false**. + Specifies if the library in question has ABI versioned functions. If it + has, this value should be set to ensure that the C files are compiled + twice with suitable parameters for each of shared or static library + builds. + version **Default Value = 1**. Specifies the ABI version of the library, and is used as the major diff --git a/lib/librte_distributor/meson.build b/lib/librte_distributor/meson.build index dba7e3b2a..5149f9bf5 100644 --- a/lib/librte_distributor/meson.build +++ b/lib/librte_distributor/meson.build @@ -9,3 +9,4 @@ else endif headers = files('rte_distributor.h') deps += ['mbuf'] +use_function_versioning = true diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h index ce963d4b1..55e88ffae 100644 --- a/lib/librte_eal/common/include/rte_function_versioning.h +++ b/lib/librte_eal/common/include/rte_function_versioning.h @@ -7,6 +7,10 @@ #define _RTE_FUNCTION_VERSIONING_H_ #include <rte_common.h> +#ifndef RTE_USE_FUNCTION_VERSIONING +#error Use of function versioning disabled, is "use_function_versioning=true" in meson.build? +#endif + #ifdef RTE_BUILD_SHARED_LIB /* diff --git a/lib/librte_lpm/meson.build b/lib/librte_lpm/meson.build index a5176d8ae..4e3920660 100644 --- a/lib/librte_lpm/meson.build +++ b/lib/librte_lpm/meson.build @@ -8,3 +8,4 @@ headers = files('rte_lpm.h', 'rte_lpm6.h') # without worrying about which architecture we actually need headers += files('rte_lpm_altivec.h', 'rte_lpm_neon.h', 'rte_lpm_sse.h') deps += ['hash'] +use_function_versioning = true diff --git a/lib/librte_timer/meson.build b/lib/librte_timer/meson.build index d3b828ce9..b7edfe2e7 100644 --- a/lib/librte_timer/meson.build +++ b/lib/librte_timer/meson.build @@ -4,3 +4,4 @@ sources = files('rte_timer.c') headers = files('rte_timer.h') allow_experimental_apis = true +use_function_versioning = true diff --git a/lib/meson.build b/lib/meson.build index e5ff83893..1b0ed767c 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -47,6 +47,7 @@ foreach l:libraries name = l version = 1 allow_experimental_apis = false + use_function_versioning = false sources = [] headers = [] includes = [] @@ -96,6 +97,9 @@ foreach l:libraries if allow_experimental_apis cflags += '-DALLOW_EXPERIMENTAL_API' endif + if use_function_versioning + cflags += '-DRTE_USE_FUNCTION_VERSIONING' + endif if get_option('per_library_versions') lib_version = '@0@.1'.format(version) @@ -117,9 +121,15 @@ foreach l:libraries include_directories: includes, dependencies: static_deps) - # then use pre-build objects to build shared lib - sources = [] - objs += static_lib.extract_all_objects(recursive: false) + if not use_function_versioning + # use pre-build objects to build shared lib + sources = [] + objs += static_lib.extract_all_objects(recursive: false) + else + # for compat we need to rebuild with + # RTE_BUILD_SHARED_LIB defined + cflags += '-DRTE_BUILD_SHARED_LIB' + endif version_map = '@0@/@1@/rte_@2@_version.map'.format( meson.current_source_dir(), dir_name, name) implib = dir_name + '.dll.a' -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice Bruce Richardson @ 2019-10-01 13:23 ` Andrzej Ostruszka 2019-10-01 16:53 ` Bruce Richardson 0 siblings, 1 reply; 17+ messages in thread From: Andrzej Ostruszka @ 2019-10-01 13:23 UTC (permalink / raw) To: Bruce Richardson, dev; +Cc: Thomas Monjalon, Ray Kinsella, Neil Horman, bluca Thanks Bruce for the patch. I like the idea of splitting versioning out of rte_compat.h, but I have some comments. On 9/27/19 10:59 PM, Bruce Richardson wrote: [...] > --- a/config/common_base > +++ b/config/common_base > @@ -111,6 +111,7 @@ CONFIG_RTE_MAX_VFIO_CONTAINERS=64 > CONFIG_RTE_MALLOC_DEBUG=n > CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n > CONFIG_RTE_USE_LIBBSD=n > +CONFIG_RTE_USE_FUNCTION_VERSIONING=y I'm not fond of this config option - it is not really an option to be changed by the user. I would prefer to just add flag to CFLAGS in mk/target/generic/rte.vars.mk. > # > # Recognize/ignore the AVX/AVX512 CPU flags for performance/power testing. > diff --git a/config/rte_config.h b/config/rte_config.h > index 0bbbe274f..b63a2fdea 100644 > --- a/config/rte_config.h > +++ b/config/rte_config.h > @@ -31,9 +31,6 @@ > > /****** library defines ********/ > > -/* compat defines */ > -#define RTE_BUILD_SHARED_LIB > - So now everything builds "as static lib" (but with "-fPIC") apart from those libraries that use symbol versioning. I'm OK with that however I'd like to note that code might be using RTE_BUILD_SHARED_LIB and do different things e.g. app/test-bbdev/test_bbdev_perf.c. I know that was already the case - just wanted to say that aloud to make sure we are all aware of this :). > diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst > index 449b33494..e95a1a2be 100644 > --- a/doc/guides/contributing/coding_style.rst > +++ b/doc/guides/contributing/coding_style.rst > @@ -948,6 +948,13 @@ reason > built. For missing dependencies this should be of the form > ``'missing dependency, "libname"'``. > > +use_function_versioning > + **Default Value = false**. > + Specifies if the library in question has ABI versioned functions. If it > + has, this value should be set to ensure that the C files are compiled > + twice with suitable parameters for each of shared or static library > + builds. > + Maybe a different name for this option? In general an "ideal theoretical" solution would be for build system to figure out on its own that separate build is necessary automatically - but that might incur some performance penalty (additional grep'ing of sources or so). So I'm fine with this option however I'd like to rename it to actually indicate what it's effect is. Like 'separate_build' or 'split_build' or 'rebuild_objects' or ... The intention of using of versioned symbols is already indicated by inclusion of the relevant header. > diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h > index ce963d4b1..55e88ffae 100644 > --- a/lib/librte_eal/common/include/rte_function_versioning.h > +++ b/lib/librte_eal/common/include/rte_function_versioning.h > @@ -7,6 +7,10 @@ > #define _RTE_FUNCTION_VERSIONING_H_ > #include <rte_common.h> > > +#ifndef RTE_USE_FUNCTION_VERSIONING > +#error Use of function versioning disabled, is "use_function_versioning=true" in meson.build? > +#endif If you accept above suggestion then change this message to something like: "Function versioning requires 'separate_build=true' in meson.build" BTW it turned out that this need for separate build for versioned symbols is a result of long standing gcc bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48200 I'll test this with clang and if this will work then maybe we could guard this #if with another check for 'gcc'. Best regards Andrzej Tested-by: Andrzej Ostruszka <amo@semihalf.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice 2019-10-01 13:23 ` Andrzej Ostruszka @ 2019-10-01 16:53 ` Bruce Richardson 2019-10-07 15:57 ` Bruce Richardson 0 siblings, 1 reply; 17+ messages in thread From: Bruce Richardson @ 2019-10-01 16:53 UTC (permalink / raw) To: Andrzej Ostruszka; +Cc: dev, Thomas Monjalon, Ray Kinsella, Neil Horman, bluca On Tue, Oct 01, 2019 at 03:23:47PM +0200, Andrzej Ostruszka wrote: > Thanks Bruce for the patch. I like the idea of splitting versioning out > of rte_compat.h, but I have some comments. > > On 9/27/19 10:59 PM, Bruce Richardson wrote: > [...] > > --- a/config/common_base > > +++ b/config/common_base > > @@ -111,6 +111,7 @@ CONFIG_RTE_MAX_VFIO_CONTAINERS=64 > > CONFIG_RTE_MALLOC_DEBUG=n > > CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n > > CONFIG_RTE_USE_LIBBSD=n > > +CONFIG_RTE_USE_FUNCTION_VERSIONING=y > > I'm not fond of this config option - it is not really an option to be > changed by the user. I would prefer to just add flag to CFLAGS in > mk/target/generic/rte.vars.mk. > Ok, that sounds reasonable enough. > > # > > # Recognize/ignore the AVX/AVX512 CPU flags for performance/power testing. > > diff --git a/config/rte_config.h b/config/rte_config.h > > index 0bbbe274f..b63a2fdea 100644 > > --- a/config/rte_config.h > > +++ b/config/rte_config.h > > @@ -31,9 +31,6 @@ > > > > /****** library defines ********/ > > > > -/* compat defines */ > > -#define RTE_BUILD_SHARED_LIB > > - > > So now everything builds "as static lib" (but with "-fPIC") apart from > those libraries that use symbol versioning. I'm OK with that however > I'd like to note that code might be using RTE_BUILD_SHARED_LIB and do > different things e.g. app/test-bbdev/test_bbdev_perf.c. I know that was > already the case - just wanted to say that aloud to make sure we are all > aware of this :). Thanks for pointing this out, I wasn't aware of it! Doing a git grep this seems to be the only place in a C file (other than rte_config.h and rte_compat.h) where the SHARED_LIB flag is being checked. I'll need to follow up on that to see what the logic is there, because it seems strange to require such a check. > > > diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst > > index 449b33494..e95a1a2be 100644 > > --- a/doc/guides/contributing/coding_style.rst > > +++ b/doc/guides/contributing/coding_style.rst > > @@ -948,6 +948,13 @@ reason > > built. For missing dependencies this should be of the form > > ``'missing dependency, "libname"'``. > > > > +use_function_versioning > > + **Default Value = false**. > > + Specifies if the library in question has ABI versioned functions. If it > > + has, this value should be set to ensure that the C files are compiled > > + twice with suitable parameters for each of shared or static library > > + builds. > > + > > Maybe a different name for this option? In general an "ideal > theoretical" solution would be for build system to figure out on its own > that separate build is necessary automatically - but that might incur > some performance penalty (additional grep'ing of sources or so). I was thinking about that, and how we can do it automatically. The trouble is that for correctness we would need to recheck every file after it had changed, and since the result of the check means that we have different build steps it would basically mean doing a full reconfiguration for every file change. That's not really practical, hence this proposed solution. > So I'm > fine with this option however I'd like to rename it to actually indicate > what it's effect is. Like 'separate_build' or 'split_build' or > 'rebuild_objects' or ... > > The intention of using of versioned symbols is already indicated by > inclusion of the relevant header. I actually feel the opposite. I'd rather have the name tied in to the fact that it's related to using function versioning - subject to what is found on investigating the #ifdef in the bbdev perf test. However, if you feel strongly about the name something else, I can probably compromise on it :-) > > > diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h > > index ce963d4b1..55e88ffae 100644 > > --- a/lib/librte_eal/common/include/rte_function_versioning.h > > +++ b/lib/librte_eal/common/include/rte_function_versioning.h > > @@ -7,6 +7,10 @@ > > #define _RTE_FUNCTION_VERSIONING_H_ > > #include <rte_common.h> > > > > +#ifndef RTE_USE_FUNCTION_VERSIONING > > +#error Use of function versioning disabled, is "use_function_versioning=true" in meson.build? > > +#endif > > If you accept above suggestion then change this message to something > like: "Function versioning requires 'separate_build=true' in meson.build" > Agreed. This obviously needs to be kept in sync with whatever the build flag is. > BTW it turned out that this need for separate build for versioned > symbols is a result of long standing gcc bug: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48200 > > I'll test this with clang and if this will work then maybe we could > guard this #if with another check for 'gcc'. > > Best regards > Andrzej > > Tested-by: Andrzej Ostruszka <amo@semihalf.com> Let me know what your testing throws up, thanks. /Bruce ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice 2019-10-01 16:53 ` Bruce Richardson @ 2019-10-07 15:57 ` Bruce Richardson 0 siblings, 0 replies; 17+ messages in thread From: Bruce Richardson @ 2019-10-07 15:57 UTC (permalink / raw) To: Andrzej Ostruszka; +Cc: dev, Thomas Monjalon, Ray Kinsella, Neil Horman, bluca On Tue, Oct 01, 2019 at 05:53:05PM +0100, Bruce Richardson wrote: > On Tue, Oct 01, 2019 at 03:23:47PM +0200, Andrzej Ostruszka wrote: > > Thanks Bruce for the patch. I like the idea of splitting versioning out > > of rte_compat.h, but I have some comments. > > > > On 9/27/19 10:59 PM, Bruce Richardson wrote: > > [...] > > > --- a/config/common_base > > > +++ b/config/common_base > > > @@ -111,6 +111,7 @@ CONFIG_RTE_MAX_VFIO_CONTAINERS=64 > > > CONFIG_RTE_MALLOC_DEBUG=n > > > CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n > > > CONFIG_RTE_USE_LIBBSD=n > > > +CONFIG_RTE_USE_FUNCTION_VERSIONING=y > > > > I'm not fond of this config option - it is not really an option to be > > changed by the user. I would prefer to just add flag to CFLAGS in > > mk/target/generic/rte.vars.mk. > > > > Ok, that sounds reasonable enough. Done in V3. > > > > # > > > # Recognize/ignore the AVX/AVX512 CPU flags for performance/power testing. > > > diff --git a/config/rte_config.h b/config/rte_config.h > > > index 0bbbe274f..b63a2fdea 100644 > > > --- a/config/rte_config.h > > > +++ b/config/rte_config.h > > > @@ -31,9 +31,6 @@ > > > > > > /****** library defines ********/ > > > > > > -/* compat defines */ > > > -#define RTE_BUILD_SHARED_LIB > > > - > > > > So now everything builds "as static lib" (but with "-fPIC") apart from > > those libraries that use symbol versioning. I'm OK with that however > > I'd like to note that code might be using RTE_BUILD_SHARED_LIB and do > > different things e.g. app/test-bbdev/test_bbdev_perf.c. I know that was > > already the case - just wanted to say that aloud to make sure we are all > > aware of this :). > > Thanks for pointing this out, I wasn't aware of it! Doing a git grep this > seems to be the only place in a C file (other than rte_config.h and > rte_compat.h) where the SHARED_LIB flag is being checked. I'll need to > follow up on that to see what the logic is there, because it seems strange > to require such a check. > This #ifdef can be removed, see patchset: http://patches.dpdk.org/project/dpdk/list/?series=6699 This then leaves function versioning as the only use-case where we need different code paths for static vs shared builds. > > > > > diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst > > > index 449b33494..e95a1a2be 100644 > > > --- a/doc/guides/contributing/coding_style.rst > > > +++ b/doc/guides/contributing/coding_style.rst > > > @@ -948,6 +948,13 @@ reason > > > built. For missing dependencies this should be of the form > > > ``'missing dependency, "libname"'``. > > > > > > +use_function_versioning > > > + **Default Value = false**. > > > + Specifies if the library in question has ABI versioned functions. If it > > > + has, this value should be set to ensure that the C files are compiled > > > + twice with suitable parameters for each of shared or static library > > > + builds. > > > + > > > > Maybe a different name for this option? In general an "ideal > > theoretical" solution would be for build system to figure out on its own > > that separate build is necessary automatically - but that might incur > > some performance penalty (additional grep'ing of sources or so). > > I was thinking about that, and how we can do it automatically. The trouble > is that for correctness we would need to recheck every file after it had > changed, and since the result of the check means that we have different > build steps it would basically mean doing a full reconfiguration for every > file change. That's not really practical, hence this proposed solution. > I've not made any changes here for the V3. However, if we want to reduce the number of changes required, we could always switch to having the rte_function_versioning.h header file included on the basis of the flag in the meson.build file. Having the C flag compile vary based on the meson one is normal, having the inverse is problematic because of what I explained above - you'd basically need to reconfigure to check after each file change. Personally, I don't think changing things to auto-include the header is worth it, hence the fact of no-change in v3. Regards, /Bruce ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support 2019-09-27 19:49 [dpdk-dev] [PATCH 0/2] Improve function versioning meson support Bruce Richardson ` (2 preceding siblings ...) 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 0/2] Improve function versioning meson support Bruce Richardson @ 2019-10-07 15:45 ` Bruce Richardson 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 1/2] eal: split compat header file Bruce Richardson ` (2 more replies) 2019-10-09 22:59 ` [dpdk-dev] [PATCH " Stephen Hemminger 4 siblings, 3 replies; 17+ messages in thread From: Bruce Richardson @ 2019-10-07 15:45 UTC (permalink / raw) To: dev; +Cc: Andrzej Ostruszka, Neil Horman, bluca, thomas, Bruce Richardson Adding support for LTO has exposed some issues with how the functions versioning was supported by meson, which was always set to build both shared and static libraries. For plain C code, so long as the -fPIC compiler flag was passed, the output is identical whether or not the code is to be included in a static library or a dynamic one. Unfortunately, when using function versioning that no longer held as different macros were used for the versioned functions depending on which type of build it was. This means that any files that use versioning need to be built twice, with different defines in each case. While the trivial solution here is just to rebuild everything twice, that involves a lot of unnecessary work when building DPDK. A better option is to identify those files or components which need multiple builds and rebuild only those. To do this, we add a new meson.build setting for libraries "use_function_versioning" and when that is set, we rebuild all source files twice, initially for static library and then with -DRTE_BUILD_SHARED_LIB for the shared library. If the flag is not set, then the static versioning setting only is used, which could lead to the build succeeding but later causing problems. To avoid that, we add a new define which must be set when the versioning header is included. This addition while solving 1 problem raises 2 other, more minor problems: * what to do with make builds? since make only builds one library type, we can just always define the new value. * what about files that include rte_compat.h for the macro for "experimental"? To solve this, we can split compat.h in two, since the versioning macro should be internal only to DPDK (as no public header should expose anything but the latest APIs), while the experimental macros are primarily for public use. V3: following feedback, moved the define for make from the common_base build config file to one of the global makefiles, as it's not user tunable. V2: added in file that missed a "git add" when doing V1 Bruce Richardson (2): eal: split compat header file build: support building ABI versioned files twice config/rte_config.h | 3 - doc/api/doxy-api-index.md | 3 +- doc/guides/contributing/coding_style.rst | 7 ++ doc/guides/contributing/versioning.rst | 4 +- lib/librte_distributor/meson.build | 1 + lib/librte_distributor/rte_distributor.c | 2 +- lib/librte_distributor/rte_distributor_v20.c | 2 +- lib/librte_eal/common/Makefile | 1 + lib/librte_eal/common/include/rte_compat.h | 70 ---------------- .../common/include/rte_function_versioning.h | 83 +++++++++++++++++++ lib/librte_lpm/meson.build | 1 + lib/librte_lpm/rte_lpm.c | 1 + lib/librte_lpm/rte_lpm.h | 1 - lib/librte_lpm/rte_lpm6.c | 1 + lib/librte_timer/meson.build | 1 + lib/librte_timer/rte_timer.c | 2 +- lib/meson.build | 16 +++- mk/target/generic/rte.vars.mk | 8 ++ 18 files changed, 124 insertions(+), 83 deletions(-) create mode 100644 lib/librte_eal/common/include/rte_function_versioning.h -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v3 1/2] eal: split compat header file 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Bruce Richardson @ 2019-10-07 15:45 ` Bruce Richardson 2019-10-27 9:49 ` Thomas Monjalon 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 2/2] build: support building ABI versioned files twice Bruce Richardson 2019-10-23 10:19 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Andrzej Ostruszka 2 siblings, 1 reply; 17+ messages in thread From: Bruce Richardson @ 2019-10-07 15:45 UTC (permalink / raw) To: dev; +Cc: Andrzej Ostruszka, Neil Horman, bluca, thomas, Bruce Richardson The compat.h header file provided macros for two purposes: 1. it provided the macros for marking functions as rte_experimental 2. it provided the macros for doing function versioning Although these were in the same file, #1 is something that is for use by public header files, which #2 is for internal use only. Therefore, we can split these into two headers, keeping #1 in rte_compat.h and #2 in a new file rte_function_versioning.h. For "make" builds, since internal objects pick up the headers from the "include/" folder, we need to add the new header to the installation list, but for "meson" builds it does not need to be installed as it's not for public use. The rework also serves to allow the use of the function versioning macros to files that actually need them, so the use of experimental functions does not need including of the versioning code. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- doc/api/doxy-api-index.md | 3 +- doc/guides/contributing/versioning.rst | 4 +- lib/librte_distributor/rte_distributor.c | 2 +- lib/librte_distributor/rte_distributor_v20.c | 2 +- lib/librte_eal/common/Makefile | 1 + lib/librte_eal/common/include/rte_compat.h | 70 ---------------- .../common/include/rte_function_versioning.h | 79 +++++++++++++++++++ lib/librte_lpm/rte_lpm.c | 1 + lib/librte_lpm/rte_lpm.h | 1 - lib/librte_lpm/rte_lpm6.c | 1 + lib/librte_timer/rte_timer.c | 2 +- 11 files changed, 89 insertions(+), 77 deletions(-) create mode 100644 lib/librte_eal/common/include/rte_function_versioning.h diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index 6c2d888ee..9acf36ba1 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -171,5 +171,6 @@ The public API headers are grouped by topics: - **misc**: [EAL config] (@ref rte_eal.h), [common] (@ref rte_common.h), - [ABI compat] (@ref rte_compat.h), + [experimental APIs] (@ref rte_compat.h), + [ABI versioning] (@ref rte_function_versioning.h), [version] (@ref rte_version.h) diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst index 3ab2c4346..64984c54e 100644 --- a/doc/guides/contributing/versioning.rst +++ b/doc/guides/contributing/versioning.rst @@ -206,7 +206,7 @@ functionality or behavior. When that occurs, it is desirable to allow for backward compatibility for a time with older binaries that are dynamically linked to the DPDK. -To support backward compatibility the ``rte_compat.h`` +To support backward compatibility the ``rte_function_versioning.h`` header file provides macros to use when updating exported functions. These macros are used in conjunction with the ``rte_<library>_version.map`` file for a given library to allow multiple versions of a symbol to exist in a shared @@ -362,7 +362,7 @@ the function, we add this line of code VERSION_SYMBOL(rte_acl_create, _v20, 2.0); -Remembering to also add the rte_compat.h header to the requisite c file where +Remembering to also add the rte_function_versioning.h header to the requisite c file where these changes are being made. The above macro instructs the linker to create a new symbol ``rte_acl_create@DPDK_2.0``, which matches the symbol created in older builds, but now points to the above newly named function. We have now mapped diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c index 21eb1fb0a..6d1e971a9 100644 --- a/lib/librte_distributor/rte_distributor.c +++ b/lib/librte_distributor/rte_distributor.c @@ -8,7 +8,7 @@ #include <rte_mbuf.h> #include <rte_memory.h> #include <rte_cycles.h> -#include <rte_compat.h> +#include <rte_function_versioning.h> #include <rte_memzone.h> #include <rte_errno.h> #include <rte_string_fns.h> diff --git a/lib/librte_distributor/rte_distributor_v20.c b/lib/librte_distributor/rte_distributor_v20.c index cdc0969a8..64c611fa9 100644 --- a/lib/librte_distributor/rte_distributor_v20.c +++ b/lib/librte_distributor/rte_distributor_v20.c @@ -9,7 +9,7 @@ #include <rte_memory.h> #include <rte_memzone.h> #include <rte_errno.h> -#include <rte_compat.h> +#include <rte_function_versioning.h> #include <rte_string_fns.h> #include <rte_eal_memconfig.h> #include <rte_pause.h> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile index a00d4fcad..d70f84fd7 100644 --- a/lib/librte_eal/common/Makefile +++ b/lib/librte_eal/common/Makefile @@ -4,6 +4,7 @@ include $(RTE_SDK)/mk/rte.vars.mk INC := rte_branch_prediction.h rte_common.h rte_compat.h +INC += rte_function_versioning.h INC += rte_debug.h rte_eal.h rte_eal_interrupts.h INC += rte_errno.h rte_launch.h rte_lcore.h INC += rte_log.h rte_memory.h rte_memzone.h diff --git a/lib/librte_eal/common/include/rte_compat.h b/lib/librte_eal/common/include/rte_compat.h index 92ff28faf..3eb33784b 100644 --- a/lib/librte_eal/common/include/rte_compat.h +++ b/lib/librte_eal/common/include/rte_compat.h @@ -5,76 +5,6 @@ #ifndef _RTE_COMPAT_H_ #define _RTE_COMPAT_H_ -#include <rte_common.h> - -#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 accommodate - * new functionality, behavior, etc. When that occurs, it is desirable 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. - * - * Refer to the guidelines document in the docs subdirectory for details on the - * use of these macros - */ - -/* - * Macro Parameters: - * b - function base name - * e - function version extension, to be concatenated with base name - * n - function symbol version string to be applied - * f - function prototype - * p - full function symbol name - */ - -/* - * VERSION_SYMBOL - * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the internal - * function name <b>_<e> - */ -#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n)) - -/* - * BIND_DEFAULT_SYMBOL - * Creates a symbol version entry instructing the linker to bind references to - * symbol <b> to the internal symbol <b>_<e> - */ -#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n)) -#define __vsym __attribute__((used)) - -/* - * MAP_STATIC_SYMBOL - * If a function has been bifurcated into multiple versions, none of which - * are defined as the exported symbol name in the map file, this macro can be - * used to alias a specific version of the symbol to its exported name. For - * example, if you have 2 versions of a function foo_v1 and foo_v2, where the - * former is mapped to foo@DPDK_1 and the latter is mapped to foo@DPDK_2 when - * building a shared library, this macro can be used to map either foo_v1 or - * foo_v2 to the symbol foo when building a static library, e.g.: - * MAP_STATIC_SYMBOL(void foo(), foo_v2); - */ -#define MAP_STATIC_SYMBOL(f, p) - -#else -/* - * No symbol versioning in use - */ -#define VERSION_SYMBOL(b, e, n) -#define __vsym -#define BIND_DEFAULT_SYMBOL(b, e, n) -#define MAP_STATIC_SYMBOL(f, p) f __attribute__((alias(RTE_STR(p)))) -/* - * RTE_BUILD_SHARED_LIB=n - */ -#endif #ifndef ALLOW_EXPERIMENTAL_API diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h new file mode 100644 index 000000000..ce963d4b1 --- /dev/null +++ b/lib/librte_eal/common/include/rte_function_versioning.h @@ -0,0 +1,79 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2015 Neil Horman <nhorman@tuxdriver.com>. + * All rights reserved. + */ + +#ifndef _RTE_FUNCTION_VERSIONING_H_ +#define _RTE_FUNCTION_VERSIONING_H_ +#include <rte_common.h> + +#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 accommodate + * new functionality, behavior, etc. When that occurs, it is desirable 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. + * + * Refer to the guidelines document in the docs subdirectory for details on the + * use of these macros + */ + +/* + * Macro Parameters: + * b - function base name + * e - function version extension, to be concatenated with base name + * n - function symbol version string to be applied + * f - function prototype + * p - full function symbol name + */ + +/* + * VERSION_SYMBOL + * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the internal + * function name <b>_<e> + */ +#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n)) + +/* + * BIND_DEFAULT_SYMBOL + * Creates a symbol version entry instructing the linker to bind references to + * symbol <b> to the internal symbol <b>_<e> + */ +#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n)) +#define __vsym __attribute__((used)) + +/* + * MAP_STATIC_SYMBOL + * If a function has been bifurcated into multiple versions, none of which + * are defined as the exported symbol name in the map file, this macro can be + * used to alias a specific version of the symbol to its exported name. For + * example, if you have 2 versions of a function foo_v1 and foo_v2, where the + * former is mapped to foo@DPDK_1 and the latter is mapped to foo@DPDK_2 when + * building a shared library, this macro can be used to map either foo_v1 or + * foo_v2 to the symbol foo when building a static library, e.g.: + * MAP_STATIC_SYMBOL(void foo(), foo_v2); + */ +#define MAP_STATIC_SYMBOL(f, p) + +#else +/* + * No symbol versioning in use + */ +#define VERSION_SYMBOL(b, e, n) +#define __vsym +#define BIND_DEFAULT_SYMBOL(b, e, n) +#define MAP_STATIC_SYMBOL(f, p) f __attribute__((alias(RTE_STR(p)))) +/* + * RTE_BUILD_SHARED_LIB=n + */ +#endif + +#endif /* _RTE_FUNCTION_VERSIONING_H_ */ diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index 3a929a1b1..c96395e26 100644 --- a/lib/librte_lpm/rte_lpm.c +++ b/lib/librte_lpm/rte_lpm.c @@ -22,6 +22,7 @@ #include <rte_rwlock.h> #include <rte_spinlock.h> #include <rte_tailq.h> +#include <rte_function_versioning.h> #include "rte_lpm.h" diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index 906ec4483..26303e628 100644 --- a/lib/librte_lpm/rte_lpm.h +++ b/lib/librte_lpm/rte_lpm.h @@ -20,7 +20,6 @@ #include <rte_memory.h> #include <rte_common.h> #include <rte_vect.h> -#include <rte_compat.h> #ifdef __cplusplus extern "C" { diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c index 9b8aeb972..e20f82460 100644 --- a/lib/librte_lpm/rte_lpm6.c +++ b/lib/librte_lpm/rte_lpm6.c @@ -25,6 +25,7 @@ #include <assert.h> #include <rte_jhash.h> #include <rte_tailq.h> +#include <rte_function_versioning.h> #include "rte_lpm6.h" diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index bdcf05d06..3834c9473 100644 --- a/lib/librte_timer/rte_timer.c +++ b/lib/librte_timer/rte_timer.c @@ -25,8 +25,8 @@ #include <rte_pause.h> #include <rte_memzone.h> #include <rte_malloc.h> -#include <rte_compat.h> #include <rte_errno.h> +#include <rte_function_versioning.h> #include "rte_timer.h" -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/2] eal: split compat header file 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 1/2] eal: split compat header file Bruce Richardson @ 2019-10-27 9:49 ` Thomas Monjalon 0 siblings, 0 replies; 17+ messages in thread From: Thomas Monjalon @ 2019-10-27 9:49 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, Andrzej Ostruszka, Neil Horman, bluca 07/10/2019 17:45, Bruce Richardson: > The compat.h header file provided macros for two purposes: > 1. it provided the macros for marking functions as rte_experimental > 2. it provided the macros for doing function versioning [...] > lib/librte_eal/common/include/rte_compat.h | 70 ---------------- > .../common/include/rte_function_versioning.h | 79 +++++++++++++++++++ I take this opportunity to add (while merging) the files in MAINTAINERS, in the section "ABI versioning" of Neil Horman, so it is clear what are the pieces of ABI code. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v3 2/2] build: support building ABI versioned files twice 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Bruce Richardson 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 1/2] eal: split compat header file Bruce Richardson @ 2019-10-07 15:45 ` Bruce Richardson 2019-10-23 10:19 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Andrzej Ostruszka 2 siblings, 0 replies; 17+ messages in thread From: Bruce Richardson @ 2019-10-07 15:45 UTC (permalink / raw) To: dev; +Cc: Andrzej Ostruszka, Neil Horman, bluca, thomas, Bruce Richardson Any file with ABI versioned functions needs different macros for shared and static builds, so we need to accomodate that. Rather than building everything twice, we just flag to the build system which libraries need that handling, by setting use_function_versioning in the meson.build files. To ensure we don't get silent errors at build time due to this meson flag being missed, we add an explicit error to the function versioning header file if a known C macro is not defined. Since "make" builds always only build one of shared or static libraries, this define can be always set, and so is added to the global CFLAGS. For meson, the build flag - and therefore the C define - is set for the three libraries that need the function versioning: "distributor", "lpm" and "timer". Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Tested-by: Andrzej Ostruszka <amo@semihalf.com> --- v3: move define from common_base to generic/rte.vars.mk --- config/rte_config.h | 3 --- doc/guides/contributing/coding_style.rst | 7 +++++++ lib/librte_distributor/meson.build | 1 + .../common/include/rte_function_versioning.h | 4 ++++ lib/librte_lpm/meson.build | 1 + lib/librte_timer/meson.build | 1 + lib/meson.build | 16 +++++++++++++--- mk/target/generic/rte.vars.mk | 8 ++++++++ 8 files changed, 35 insertions(+), 6 deletions(-) diff --git a/config/rte_config.h b/config/rte_config.h index 0bbbe274f..b63a2fdea 100644 --- a/config/rte_config.h +++ b/config/rte_config.h @@ -31,9 +31,6 @@ /****** library defines ********/ -/* compat defines */ -#define RTE_BUILD_SHARED_LIB - /* EAL defines */ #define RTE_MAX_HEAPS 32 #define RTE_MAX_MEMSEG_LISTS 128 diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst index 449b33494..e95a1a2be 100644 --- a/doc/guides/contributing/coding_style.rst +++ b/doc/guides/contributing/coding_style.rst @@ -948,6 +948,13 @@ reason built. For missing dependencies this should be of the form ``'missing dependency, "libname"'``. +use_function_versioning + **Default Value = false**. + Specifies if the library in question has ABI versioned functions. If it + has, this value should be set to ensure that the C files are compiled + twice with suitable parameters for each of shared or static library + builds. + version **Default Value = 1**. Specifies the ABI version of the library, and is used as the major diff --git a/lib/librte_distributor/meson.build b/lib/librte_distributor/meson.build index dba7e3b2a..5149f9bf5 100644 --- a/lib/librte_distributor/meson.build +++ b/lib/librte_distributor/meson.build @@ -9,3 +9,4 @@ else endif headers = files('rte_distributor.h') deps += ['mbuf'] +use_function_versioning = true diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h index ce963d4b1..55e88ffae 100644 --- a/lib/librte_eal/common/include/rte_function_versioning.h +++ b/lib/librte_eal/common/include/rte_function_versioning.h @@ -7,6 +7,10 @@ #define _RTE_FUNCTION_VERSIONING_H_ #include <rte_common.h> +#ifndef RTE_USE_FUNCTION_VERSIONING +#error Use of function versioning disabled, is "use_function_versioning=true" in meson.build? +#endif + #ifdef RTE_BUILD_SHARED_LIB /* diff --git a/lib/librte_lpm/meson.build b/lib/librte_lpm/meson.build index a5176d8ae..4e3920660 100644 --- a/lib/librte_lpm/meson.build +++ b/lib/librte_lpm/meson.build @@ -8,3 +8,4 @@ headers = files('rte_lpm.h', 'rte_lpm6.h') # without worrying about which architecture we actually need headers += files('rte_lpm_altivec.h', 'rte_lpm_neon.h', 'rte_lpm_sse.h') deps += ['hash'] +use_function_versioning = true diff --git a/lib/librte_timer/meson.build b/lib/librte_timer/meson.build index d3b828ce9..b7edfe2e7 100644 --- a/lib/librte_timer/meson.build +++ b/lib/librte_timer/meson.build @@ -4,3 +4,4 @@ sources = files('rte_timer.c') headers = files('rte_timer.h') allow_experimental_apis = true +use_function_versioning = true diff --git a/lib/meson.build b/lib/meson.build index e5ff83893..1b0ed767c 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -47,6 +47,7 @@ foreach l:libraries name = l version = 1 allow_experimental_apis = false + use_function_versioning = false sources = [] headers = [] includes = [] @@ -96,6 +97,9 @@ foreach l:libraries if allow_experimental_apis cflags += '-DALLOW_EXPERIMENTAL_API' endif + if use_function_versioning + cflags += '-DRTE_USE_FUNCTION_VERSIONING' + endif if get_option('per_library_versions') lib_version = '@0@.1'.format(version) @@ -117,9 +121,15 @@ foreach l:libraries include_directories: includes, dependencies: static_deps) - # then use pre-build objects to build shared lib - sources = [] - objs += static_lib.extract_all_objects(recursive: false) + if not use_function_versioning + # use pre-build objects to build shared lib + sources = [] + objs += static_lib.extract_all_objects(recursive: false) + else + # for compat we need to rebuild with + # RTE_BUILD_SHARED_LIB defined + cflags += '-DRTE_BUILD_SHARED_LIB' + endif version_map = '@0@/@1@/rte_@2@_version.map'.format( meson.current_source_dir(), dir_name, name) implib = dir_name + '.dll.a' diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.mk index 5f00a0bfa..374722173 100644 --- a/mk/target/generic/rte.vars.mk +++ b/mk/target/generic/rte.vars.mk @@ -90,6 +90,14 @@ ASFLAGS += $(TARGET_ASFLAGS) CFLAGS += -I$(RTE_OUTPUT)/include LDFLAGS += -L$(RTE_OUTPUT)/lib +# add in flag for supporting function versioning. The define is used in meson +# builds to ensure that the user has properly flagged the unit in question as +# using function versioning so it can be built twice - once for static lib and +# then a second time for the shared lib. Since make only builds one library +# type at a time, such precautions aren't necessary, so we can globally define +# the flag +CFLAGS += -DRTE_USE_FUNCTION_VERSIONING + # always include rte_config.h: the one in $(RTE_OUTPUT)/include is # the configuration of SDK when $(BUILDING_RTE_SDK) is true, or the # configuration of the application if $(BUILDING_RTE_SDK) is not -- 2.21.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Bruce Richardson 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 1/2] eal: split compat header file Bruce Richardson 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 2/2] build: support building ABI versioned files twice Bruce Richardson @ 2019-10-23 10:19 ` Andrzej Ostruszka 2019-10-27 10:26 ` Thomas Monjalon 2 siblings, 1 reply; 17+ messages in thread From: Andrzej Ostruszka @ 2019-10-23 10:19 UTC (permalink / raw) To: Bruce Richardson, dev; +Cc: Neil Horman, bluca, thomas On 10/7/19 5:45 PM, Bruce Richardson wrote: > Adding support for LTO has exposed some issues with how the functions > versioning was supported by meson, which was always set to build both > shared and static libraries. [...] > V3: following feedback, moved the define for make from the common_base > build config file to one of the global makefiles, as it's not user > tunable. > V2: added in file that missed a "git add" when doing V1 Reviewed-by: Andrzej Ostruszka <aostruszka@marvell.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support 2019-10-23 10:19 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Andrzej Ostruszka @ 2019-10-27 10:26 ` Thomas Monjalon 0 siblings, 0 replies; 17+ messages in thread From: Thomas Monjalon @ 2019-10-27 10:26 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, Andrzej Ostruszka, Neil Horman, bluca 23/10/2019 12:19, Andrzej Ostruszka: > On 10/7/19 5:45 PM, Bruce Richardson wrote: > > Adding support for LTO has exposed some issues with how the functions > > versioning was supported by meson, which was always set to build both > > shared and static libraries. > [...] > > V3: following feedback, moved the define for make from the common_base > > build config file to one of the global makefiles, as it's not user > > tunable. > > V2: added in file that missed a "git add" when doing V1 > > Reviewed-by: Andrzej Ostruszka <aostruszka@marvell.com> Applied, thanks ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 0/2] Improve function versioning meson support 2019-09-27 19:49 [dpdk-dev] [PATCH 0/2] Improve function versioning meson support Bruce Richardson ` (3 preceding siblings ...) 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Bruce Richardson @ 2019-10-09 22:59 ` Stephen Hemminger 4 siblings, 0 replies; 17+ messages in thread From: Stephen Hemminger @ 2019-10-09 22:59 UTC (permalink / raw) To: Bruce Richardson Cc: dev, Andrzej Ostruszka, Thomas Monjalon, Ray Kinsella, Neil Horman On Fri, 27 Sep 2019 20:49:30 +0100 Bruce Richardson <bruce.richardson@intel.com> wrote: > Adding support for LTO has exposed some issues with how the functions > versioning was supported by meson, which was always set to build both > shared and static libraries. > > For plain C code, so long as the -fPIC compiler flag was passed, the > output is identical whether or not the code is to be included in a > static library or a dynamic one. Unfortunately, when using function > versioning that no longer held as different macros were used for the > versioned functions depending on which type of build it was. This means > that any files that use versioning need to be built twice, with > different defines in each case. > > While the trivial solution here is just to rebuild everything twice, > that involves a lot of unnecessary work when building DPDK. A better > option is to identify those files or components which need multiple > builds and rebuild only those. To do this, we add a new meson.build > setting for libraries "use_function_versioning" and when that is set, we > rebuild all source files twice, initially for static library and then > with -DRTE_BUILD_SHARED_LIB for the shared library. > > If the flag is not set, then the static versioning setting only is used, > which could lead to the build succeeding but later causing problems. To > avoid that, we add a new define which must be set when the versioning > header is included. This addition while solving 1 problem raises 2 > other, more minor problems: > * what to do with make builds? since make only builds one library type, > we can just always define the new value. > * what about files that include rte_compat.h for the macro for > "experimental"? To solve this, we can split compat.h in two, since the > versioning macro should be internal only to DPDK (as no public header > should expose anything but the latest APIs), while the experimental > macros are primarily for public use. > > Bruce Richardson (2): > eal: split compat header file > build: support building ABI versioned files twice > > config/common_base | 1 + > config/rte_config.h | 3 --- > doc/api/doxy-api-index.md | 3 ++- > doc/guides/contributing/coding_style.rst | 7 ++++++ > doc/guides/contributing/versioning.rst | 4 ++-- > lib/librte_distributor/meson.build | 1 + > lib/librte_distributor/rte_distributor.c | 2 +- > lib/librte_distributor/rte_distributor_v20.c | 2 +- > lib/librte_eal/common/Makefile | 1 + > ...rte_compat.h => rte_function_versioning.h} | 23 ++++++------------- > lib/librte_lpm/meson.build | 1 + > lib/librte_lpm/rte_lpm.c | 1 + > lib/librte_lpm/rte_lpm.h | 1 - > lib/librte_lpm/rte_lpm6.c | 1 + > lib/librte_timer/meson.build | 1 + > lib/librte_timer/rte_timer.c | 2 +- > lib/meson.build | 16 ++++++++++--- > 17 files changed, 41 insertions(+), 29 deletions(-) > rename lib/librte_eal/common/include/{rte_compat.h => rte_function_versioning.h} (89%) > Looks fine. Acked-by: Stephen Hemminger <stephen@networkplumber.org> ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-10-27 10:27 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-27 19:49 [dpdk-dev] [PATCH 0/2] Improve function versioning meson support Bruce Richardson 2019-09-27 19:49 ` [dpdk-dev] [PATCH 1/2] eal: split compat header file Bruce Richardson 2019-09-27 20:48 ` Bruce Richardson 2019-09-27 19:49 ` [dpdk-dev] [PATCH 2/2] build: support building ABI versioned files twice Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 0/2] Improve function versioning meson support Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 1/2] eal: split compat header file Bruce Richardson 2019-09-27 20:59 ` [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice Bruce Richardson 2019-10-01 13:23 ` Andrzej Ostruszka 2019-10-01 16:53 ` Bruce Richardson 2019-10-07 15:57 ` Bruce Richardson 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Bruce Richardson 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 1/2] eal: split compat header file Bruce Richardson 2019-10-27 9:49 ` Thomas Monjalon 2019-10-07 15:45 ` [dpdk-dev] [PATCH v3 2/2] build: support building ABI versioned files twice Bruce Richardson 2019-10-23 10:19 ` [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support Andrzej Ostruszka 2019-10-27 10:26 ` Thomas Monjalon 2019-10-09 22:59 ` [dpdk-dev] [PATCH " Stephen Hemminger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).