[dpdk-dev] [PATCH 0/2] Improve function versioning meson support

[dpdk-dev] [PATCH 0/2] Improve function versioning meson support

[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

[dpdk-dev] [PATCH 1/2] eal: split compat header file

[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

[dpdk-dev] [PATCH 2/2] build: support building ABI versioned files twice

[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

Re: [dpdk-dev] [PATCH 1/2] eal: split compat header file

[Bruce Richardson]

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.

[dpdk-dev] [PATCH v2 0/2] Improve function versioning meson support

[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

[dpdk-dev] [PATCH v2 1/2] eal: split compat header file

[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

[dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice

[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

Re: [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice

[Andrzej Ostruszka]

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>

Re: [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice

[Bruce Richardson]

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

[dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support

[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

[dpdk-dev] [PATCH v3 1/2] eal: split compat header file

[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

[dpdk-dev] [PATCH v3 2/2] build: support building ABI versioned files twice

[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

Re: [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice

[Bruce Richardson]

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

Re: [dpdk-dev] [PATCH 0/2] Improve function versioning meson support

[Stephen Hemminger]

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>

Re: [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support

[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>

Re: [dpdk-dev] [PATCH v3 1/2] eal: split compat header file

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCH v3 0/2] Improve function versioning meson support

[Thomas Monjalon]

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