DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: David Marchand <david.marchand@redhat.com>
Cc: <dev@dpdk.org>, <thomas@monjalon.net>, <andremue@linux.microsoft.com>
Subject: Re: [RFC v3 5/8] build: generate symbol maps
Date: Thu, 13 Mar 2025 17:26:47 +0000	[thread overview]
Message-ID: <Z9MVVzHCtYaVy2AV@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <20250311095609.194523-6-david.marchand@redhat.com>

On Tue, Mar 11, 2025 at 10:56:03AM +0100, David Marchand wrote:
> Rather than maintain a file in parallel of the code, symbols to be
> exported can be marked with a token RTE_EXPORT_*SYMBOL.
> 
> From those marks, the build framework generates map files only for
> symbols actually compiled (which means that the WINDOWS_NO_EXPORT hack
> becomes unnecessary).
> 
> The build framework directly creates a map file in the format that the
> linker expects (rather than converting from GNU linker to MSVC linker).
> 
> Empty maps are allowed again as a replacement for drivers/version.map.
> 
> The symbol check is updated to only support the new format.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Some comments inline below.
/Bruce

> ---
> Changes since RFC v2:
> - because of MSVC limitations wrt macro passed via cmdline,
>   used an internal header for defining RTE_EXPORT_* macros,
> - updated documentation and tooling,
> 
> ---
>  MAINTAINERS                                |   2 +
>  buildtools/gen-version-map.py              | 111 ++++++++++
>  buildtools/map-list-symbol.sh              |  10 +-
>  buildtools/meson.build                     |   1 +
>  config/meson.build                         |   2 +
>  config/rte_export.h                        |  16 ++
>  devtools/check-symbol-change.py            |  90 +++++++++
>  devtools/check-symbol-maps.sh              |  14 --
>  devtools/checkpatches.sh                   |   2 +-
>  doc/guides/contributing/abi_versioning.rst | 224 ++-------------------
>  drivers/meson.build                        |  94 +++++----
>  drivers/version.map                        |   3 -
>  lib/meson.build                            |  91 ++++++---
>  13 files changed, 371 insertions(+), 289 deletions(-)
>  create mode 100755 buildtools/gen-version-map.py
>  create mode 100644 config/rte_export.h
>  create mode 100755 devtools/check-symbol-change.py
>  delete mode 100644 drivers/version.map
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 312e6fcee5..04772951d3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -95,6 +95,7 @@ F: devtools/check-maintainers.sh
>  F: devtools/check-forbidden-tokens.awk
>  F: devtools/check-git-log.sh
>  F: devtools/check-spdx-tag.sh
> +F: devtools/check-symbol-change.py
>  F: devtools/check-symbol-change.sh
>  F: devtools/check-symbol-maps.sh
>  F: devtools/checkpatches.sh
> @@ -127,6 +128,7 @@ F: config/
>  F: buildtools/check-symbols.sh
>  F: buildtools/chkincs/
>  F: buildtools/call-sphinx-build.py
> +F: buildtools/gen-version-map.py
>  F: buildtools/get-cpu-count.py
>  F: buildtools/get-numa-count.py
>  F: buildtools/list-dir-globs.py
> diff --git a/buildtools/gen-version-map.py b/buildtools/gen-version-map.py
> new file mode 100755
> index 0000000000..b160aa828b
> --- /dev/null
> +++ b/buildtools/gen-version-map.py
> @@ -0,0 +1,111 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2024 Red Hat, Inc.
> +
> +"""Generate a version map file used by GNU or MSVC linker."""
> +

While it's an internal build script not to be run by users directly, I
believe a short one-line usage here might be useful, since the code below
is directly referencing sys.argv[N] values. That makes it easier for the
user to know what they are.

Alternatively, assign them to proper names at the top of the script e.g.:
	scriptname, link_mode, abi_version_file, output, *input = sys.argv

Final alternative (which may be a bit overkill) is to use argparse.

> +import re
> +import sys
> +
> +# From rte_export.h
> +export_exp_sym_regexp = re.compile(r"^RTE_EXPORT_EXPERIMENTAL_SYMBOL\(([^,]+), ([0-9]+.[0-9]+)\)")
> +export_int_sym_regexp = re.compile(r"^RTE_EXPORT_INTERNAL_SYMBOL\(([^)]+)\)")
> +export_sym_regexp = re.compile(r"^RTE_EXPORT_SYMBOL\(([^)]+)\)")
> +# From rte_function_versioning.h
> +ver_sym_regexp = re.compile(r"^RTE_VERSION_SYMBOL\(([^,]+), [^,]+, ([^,]+),")
> +ver_exp_sym_regexp = re.compile(r"^RTE_VERSION_EXPERIMENTAL_SYMBOL\([^,]+, ([^,]+),")
> +default_sym_regexp = re.compile(r"^RTE_DEFAULT_SYMBOL\(([^,]+), [^,]+, ([^,]+),")
> +
> +with open(sys.argv[2]) as f:
> +    abi = 'DPDK_{}'.format(re.match("([0-9]+).[0-9]", f.readline()).group(1))
> +
> +symbols = {}
> +
> +for file in sys.argv[4:]:
> +    with open(file, encoding="utf-8") as f:
> +        for ln in f.readlines():
> +            node = None
> +            symbol = None
> +            comment = None
> +            if export_exp_sym_regexp.match(ln):
> +                node = 'EXPERIMENTAL'
> +                symbol = export_exp_sym_regexp.match(ln).group(1)
> +                comment = ' # added in {}'.format(export_exp_sym_regexp.match(ln).group(2))
> +            elif export_int_sym_regexp.match(ln):
> +                node = 'INTERNAL'
> +                symbol = export_int_sym_regexp.match(ln).group(1)
> +            elif export_sym_regexp.match(ln):
> +                node = abi
> +                symbol = export_sym_regexp.match(ln).group(1)
> +            elif ver_sym_regexp.match(ln):
> +                node = 'DPDK_{}'.format(ver_sym_regexp.match(ln).group(1))
> +                symbol = ver_sym_regexp.match(ln).group(2)
> +            elif ver_exp_sym_regexp.match(ln):
> +                node = 'EXPERIMENTAL'
> +                symbol = ver_exp_sym_regexp.match(ln).group(1)
> +            elif default_sym_regexp.match(ln):
> +                node = 'DPDK_{}'.format(default_sym_regexp.match(ln).group(1))
> +                symbol = default_sym_regexp.match(ln).group(2)
> +
> +            if not symbol:
> +                continue
> +
> +            if node not in symbols:
> +                symbols[node] = {}
> +            symbols[node][symbol] = comment
> +
> +if sys.argv[1] == 'msvc':
> +    with open(sys.argv[3], "w") as outfile:
> +        outfile.writelines(f"EXPORTS\n")
> +        for key in (abi, 'EXPERIMENTAL', 'INTERNAL'):
> +            if key not in symbols:
> +                continue
> +            for symbol in sorted(symbols[key].keys()):
> +                outfile.writelines(f"\t{symbol}\n")
> +            del symbols[key]
> +else:
> +    with open(sys.argv[3], "w") as outfile:
> +        local_token = False
> +        for key in (abi, 'EXPERIMENTAL', 'INTERNAL'):
> +            if key not in symbols:
> +                continue
> +            outfile.writelines(f"{key} {{\n\tglobal:\n\n")
> +            for symbol in sorted(symbols[key].keys()):
> +                if sys.argv[1] == 'mingw' and symbol.startswith('per_lcore'):
> +                    prefix = '__emutls_v.'
> +                else:
> +                    prefix = ''
> +                outfile.writelines(f"\t{prefix}{symbol};")
> +                comment = symbols[key][symbol]
> +                if comment:
> +                    outfile.writelines(f"{comment}")
> +                outfile.writelines("\n")

How about using "" rather than None for the default comment so you can
always just do a print of "{prefix}{symbol};{comment}\n". The fact that
writelines doesn't output a "\n" is a little confusing here, so maybe use
"print" instead.

	print("f\t{prefix}{symbol};{comment}", file=outfile)

> +            outfile.writelines("\n")
> +            if not local_token:
> +                outfile.writelines("\tlocal: *;\n")
> +                local_token = True
> +            outfile.writelines("};\n")
> +            del symbols[key]
> +        for key in sorted(symbols.keys()):
> +            outfile.writelines(f"{key} {{\n\tglobal:\n\n")
> +            for symbol in sorted(symbols[key].keys()):
> +                if sys.argv[1] == 'mingw' and symbol.startswith('per_lcore'):
> +                    prefix = '__emutls_v.'
> +                else:
> +                    prefix = ''
> +                outfile.writelines(f"\t{prefix}{symbol};")
> +                comment = symbols[key][symbol]
> +                if comment:
> +                    outfile.writelines(f"{comment}")
> +                outfile.writelines("\n")
> +            outfile.writelines(f"}} {abi};\n")
> +            if not local_token:
> +                outfile.writelines("\tlocal: *;\n")
> +                local_token = True
> +            del symbols[key]
> +        # No exported symbol, add a catch all
> +        if not local_token:
> +            outfile.writelines(f"{abi} {{\n")
> +            outfile.writelines("\tlocal: *;\n")
> +            local_token = True
> +            outfile.writelines("};\n")
> diff --git a/buildtools/map-list-symbol.sh b/buildtools/map-list-symbol.sh
> index eb98451d8e..0829df4be5 100755
> --- a/buildtools/map-list-symbol.sh
> +++ b/buildtools/map-list-symbol.sh
> @@ -62,10 +62,14 @@ for file in $@; do
>  		if (current_section == "") {
>  			next;
>  		}
> +		symbol_version = current_version
> +		if (/^[^}].*[^:*]; # added in /) {
> +			symbol_version = $5
> +		}
>  		if ("'$version'" != "") {
> -			if ("'$version'" == "unset" && current_version != "") {
> +			if ("'$version'" == "unset" && symbol_version != "") {
>  				next;
> -			} else if ("'$version'" != "unset" && "'$version'" != current_version) {
> +			} else if ("'$version'" != "unset" && "'$version'" != symbol_version) {
>  				next;
>  			}
>  		}
> @@ -73,7 +77,7 @@ for file in $@; do
>  		if ("'$symbol'" == "all" || $1 == "'$symbol'") {
>  			ret = 0;
>  			if ("'$quiet'" == "") {
> -				print "'$file' "current_section" "$1" "current_version;
> +				print "'$file' "current_section" "$1" "symbol_version;
>  			}
>  			if ("'$symbol'" != "all") {
>  				exit 0;
> diff --git a/buildtools/meson.build b/buildtools/meson.build
> index 4e2c1217a2..b745e9afa4 100644
> --- a/buildtools/meson.build
> +++ b/buildtools/meson.build
> @@ -16,6 +16,7 @@ else
>      py3 = ['meson', 'runpython']
>  endif
>  echo = py3 + ['-c', 'import sys; print(*sys.argv[1:])']
> +gen_version_map = py3 + files('gen-version-map.py')
>  list_dir_globs = py3 + files('list-dir-globs.py')
>  map_to_win_cmd = py3 + files('map_to_win.py')
>  sphinx_wrapper = py3 + files('call-sphinx-build.py')
> diff --git a/config/meson.build b/config/meson.build
> index f31fef216c..54657055fb 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -303,8 +303,10 @@ endif
>  # add -include rte_config to cflags
>  if is_ms_compiler
>      add_project_arguments('/FI', 'rte_config.h', language: 'c')
> +    add_project_arguments('/FI', 'rte_export.h', language: 'c')
>  else
>      add_project_arguments('-include', 'rte_config.h', language: 'c')
> +    add_project_arguments('-include', 'rte_export.h', language: 'c')
>  endif
>  
>  # enable extra warnings and disable any unwanted warnings
> diff --git a/config/rte_export.h b/config/rte_export.h
> new file mode 100644
> index 0000000000..83d871fe11
> --- /dev/null
> +++ b/config/rte_export.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2025 Red Hat, Inc.
> + */
> +
> +#ifndef RTE_EXPORT_H
> +#define RTE_EXPORT_H
> +
> +/* *Internal* macros for exporting symbols, used by the build system.
> + * For RTE_EXPORT_EXPERIMENTAL_SYMBOL, ver indicates the
> + * version this symbol was introduced in.
> + */
> +#define RTE_EXPORT_EXPERIMENTAL_SYMBOL(a, ver)
> +#define RTE_EXPORT_INTERNAL_SYMBOL(a)
> +#define RTE_EXPORT_SYMBOL(a)
> +
> +#endif /* RTE_EXPORT_H */
> diff --git a/devtools/check-symbol-change.py b/devtools/check-symbol-change.py
> new file mode 100755
> index 0000000000..09709e4f06
> --- /dev/null
> +++ b/devtools/check-symbol-change.py
> @@ -0,0 +1,90 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2025 Red Hat, Inc.
> +
> +"""Check exported symbols change in a patch."""
> +
> +import re
> +import sys
> +
> +file_header_regexp = re.compile(r"^(\-\-\-|\+\+\+) [ab]/(lib|drivers)/([^/]+)/([^/]+)")
> +# From rte_export.h
> +export_exp_sym_regexp = re.compile(r"^.RTE_EXPORT_EXPERIMENTAL_SYMBOL\(([^,]+),")
> +export_int_sym_regexp = re.compile(r"^.RTE_EXPORT_INTERNAL_SYMBOL\(([^)]+)\)")
> +export_sym_regexp = re.compile(r"^.RTE_EXPORT_SYMBOL\(([^)]+)\)")
> +# TODO, handle versioned symbols from rte_function_versioning.h
> +# ver_sym_regexp = re.compile(r"^.RTE_VERSION_SYMBOL\(([^,]+), [^,]+, ([^,]+),")
> +# ver_exp_sym_regexp = re.compile(r"^.RTE_VERSION_EXPERIMENTAL_SYMBOL\([^,]+, ([^,]+),")
> +# default_sym_regexp = re.compile(r"^.RTE_DEFAULT_SYMBOL\(([^,]+), [^,]+, ([^,]+),")
> +
> +symbols = {}
> +
> +for file in sys.argv[1:]:
> +    with open(file, encoding="utf-8") as f:
> +        for ln in f.readlines():
> +            if file_header_regexp.match(ln):
> +                if file_header_regexp.match(ln).group(2) == "lib":
> +                    lib = '/'.join(file_header_regexp.match(ln).group(2, 3))
> +                elif file_header_regexp.match(ln).group(3) == "intel":
> +                    lib = '/'.join(file_header_regexp.match(ln).group(2, 3, 4))
> +                else:
> +                    lib = '/'.join(file_header_regexp.match(ln).group(2, 3))
> +
> +                if lib not in symbols:
> +                    symbols[lib] = {}
> +                continue
> +
> +            if export_exp_sym_regexp.match(ln):
> +                symbol = export_exp_sym_regexp.match(ln).group(1)
> +                node = 'EXPERIMENTAL'
> +            elif export_int_sym_regexp.match(ln):
> +                node = 'INTERNAL'
> +                symbol = export_int_sym_regexp.match(ln).group(1)
> +            elif export_sym_regexp.match(ln):
> +                symbol = export_sym_regexp.match(ln).group(1)
> +                node = 'stable'
> +            else:
> +                continue
> +
> +            if symbol not in symbols[lib]:
> +                symbols[lib][symbol] = {}
> +            added = ln[0] == '+'
> +            if added and 'added' in symbols[lib][symbol] and node != symbols[lib][symbol]['added']:
> +                print(f"{symbol} in {lib} was found in multiple ABI, please check.")
> +            if not added and 'removed' in symbols[lib][symbol] and node != symbols[lib][symbol]['removed']:
> +                print(f"{symbol} in {lib} was found in multiple ABI, please check.")
> +            if added:
> +                symbols[lib][symbol]['added'] = node
> +            else:
> +                symbols[lib][symbol]['removed'] = node
> +
> +    for lib in sorted(symbols.keys()):
> +        error = False
> +        for symbol in sorted(symbols[lib].keys()):
> +            if 'removed' not in symbols[lib][symbol]:
> +                # Symbol addition
> +                node = symbols[lib][symbol]['added']
> +                if node == 'stable':
> +                    print(f"ERROR: {symbol} in {lib} has been added directly to stable ABI.")
> +                    error = True
> +                else:
> +                    print(f"INFO: {symbol} in {lib} has been added to {node} ABI.")
> +                continue
> +
> +            if 'added' not in symbols[lib][symbol]:
> +                # Symbol removal
> +                node = symbols[lib][symbol]['added']
> +                if node == 'stable':
> +                    print(f"INFO: {symbol} in {lib} has been removed from stable ABI.")
> +                    print(f"Please check it has gone though the deprecation process.")
> +                continue
> +
> +            if symbols[lib][symbol]['added'] == symbols[lib][symbol]['removed']:
> +                # Symbol was moved around
> +                continue
> +
> +            # Symbol modifications
> +            added = symbols[lib][symbol]['added']
> +            removed = symbols[lib][symbol]['removed']
> +            print(f"INFO: {symbol} in {lib} is moving from {removed} to {added}")
> +            print(f"Please check it has gone though the deprecation process.")
> diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
> index 6121f78ec6..fcd3931e5d 100755
> --- a/devtools/check-symbol-maps.sh
> +++ b/devtools/check-symbol-maps.sh
> @@ -60,20 +60,6 @@ if [ -n "$local_miss_maps" ] ; then
>      ret=1
>  fi
>  
> -find_empty_maps ()
> -{
> -    for map in $@ ; do
> -        [ $(buildtools/map-list-symbol.sh $map | wc -l) != '0' ] || echo $map
> -    done
> -}
> -
> -empty_maps=$(find_empty_maps $@)
> -if [ -n "$empty_maps" ] ; then
> -    echo "Found empty maps:"
> -    echo "$empty_maps"
> -    ret=1
> -fi
> -
>  find_bad_format_maps ()
>  {
>      abi_version=$(cut -d'.' -f 1 ABI_VERSION)
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 003bb49e04..7dcac7c8c9 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,STRLCPY,\
>  PREFER_KERNEL_TYPES,PREFER_FALLTHROUGH,BIT_MACRO,CONST_STRUCT,\
>  SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
>  LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
> -NEW_TYPEDEFS,COMPARISON_TO_NULL,AVOID_BUG"
> +NEW_TYPEDEFS,COMPARISON_TO_NULL,AVOID_BUG,EXPORT_SYMBOL"
>  options="$options $DPDK_CHECKPATCH_OPTIONS"
>  
>  print_usage () {
> diff --git a/doc/guides/contributing/abi_versioning.rst b/doc/guides/contributing/abi_versioning.rst
> index 88dd776b4c..addbb24b9e 100644
> --- a/doc/guides/contributing/abi_versioning.rst
> +++ b/doc/guides/contributing/abi_versioning.rst
> @@ -58,12 +58,12 @@ persists over multiple releases.
>  
>  .. code-block:: none
>  
> - $ head ./lib/acl/version.map
> + $ head ./build/lib/librte_acl_exports.map

I must admit I'm not a fan of these long filenames. How about just
"acl_exports.map"?

>   DPDK_21 {
>          global:
>   ...
>  
> - $ head ./lib/eal/version.map
> + $ head ./build/lib/librte_eal_exports.map
>   DPDK_21 {
>          global:
>   ...
> @@ -77,7 +77,7 @@ that library.
>  
>  .. code-block:: none
>  
> - $ head ./lib/acl/version.map
> + $ head ./build/lib/librte_acl_exports.map
>   DPDK_21 {
>          global:
>   ...
> @@ -88,7 +88,7 @@ that library.
>   } DPDK_21;
>   ...
>  
> - $ head ./lib/eal/version.map
> + $ head ./build/lib/librte_eal_exports.map
>   DPDK_21 {
>          global:
>   ...
> @@ -100,12 +100,12 @@ how this may be done.
>  
>  .. code-block:: none
>  
> - $ head ./lib/acl/version.map
> + $ head ./build/lib/librte_acl_exports.map
>   DPDK_22 {
>          global:
>   ...
>  
> - $ head ./lib/eal/version.map
> + $ head ./build/lib/librte_eal_exports.map
>   DPDK_22 {
>          global:
>   ...
> @@ -134,8 +134,7 @@ linked to the DPDK.
>  
>  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 ``version.map`` file for
> -a given library to allow multiple versions of a symbol to exist in a shared
> +macros allow multiple versions of a symbol to exist in a shared
>  library so that older binaries need not be immediately recompiled.
>  
>  The macros are:
> @@ -169,6 +168,7 @@ Assume we have a function as follows
>    * Create an acl context object for apps to
>    * manipulate
>    */
> + RTE_EXPORT_SYMBOL(rte_acl_create)
>   struct rte_acl_ctx *
>   rte_acl_create(const struct rte_acl_param *param)
>   {
> @@ -187,6 +187,7 @@ private, is safe), but it also requires modifying the code as follows
>    * Create an acl context object for apps to
>    * manipulate
>    */
> + RTE_EXPORT_SYMBOL(rte_acl_create)
>   struct rte_acl_ctx *
>   rte_acl_create(const struct rte_acl_param *param, int debug)
>   {
> @@ -203,78 +204,16 @@ The addition of a parameter to the function is ABI breaking as the function is
>  public, and existing application may use it in its current form. However, the
>  compatibility macros in DPDK allow a developer to use symbol versioning so that
>  multiple functions can be mapped to the same public symbol based on when an
> -application was linked to it. To see how this is done, we start with the
> -requisite libraries version map file. Initially the version map file for the acl
> -library looks like this
> +application was linked to it.
>  
> -.. code-block:: none
> -
> -   DPDK_21 {
> -        global:
> -
> -        rte_acl_add_rules;
> -        rte_acl_build;
> -        rte_acl_classify;
> -        rte_acl_classify_alg;
> -        rte_acl_classify_scalar;
> -        rte_acl_create;
> -        rte_acl_dump;
> -        rte_acl_find_existing;
> -        rte_acl_free;
> -        rte_acl_ipv4vlan_add_rules;
> -        rte_acl_ipv4vlan_build;
> -        rte_acl_list_dump;
> -        rte_acl_reset;
> -        rte_acl_reset_rules;
> -        rte_acl_set_ctx_classify;
> -
> -        local: *;
> -   };
> -
> -This file needs to be modified as follows
> -
> -.. code-block:: none
> -
> -   DPDK_21 {
> -        global:
> -
> -        rte_acl_add_rules;
> -        rte_acl_build;
> -        rte_acl_classify;
> -        rte_acl_classify_alg;
> -        rte_acl_classify_scalar;
> -        rte_acl_create;
> -        rte_acl_dump;
> -        rte_acl_find_existing;
> -        rte_acl_free;
> -        rte_acl_ipv4vlan_add_rules;
> -        rte_acl_ipv4vlan_build;
> -        rte_acl_list_dump;
> -        rte_acl_reset;
> -        rte_acl_reset_rules;
> -        rte_acl_set_ctx_classify;
> -
> -        local: *;
> -   };
> -
> -   DPDK_22 {
> -        global:
> -        rte_acl_create;
> -
> -   } DPDK_21;
> -
> -The addition of the new block tells the linker that a new version node
> -``DPDK_22`` is available, which contains the symbol rte_acl_create, and inherits
> -the symbols from the DPDK_21 node. This list is directly translated into a
> -list of exported symbols when DPDK is compiled as a shared library.
> -
> -Next, we need to specify in the code which function maps to the rte_acl_create
> +We need to specify in the code which function maps to the rte_acl_create
>  symbol at which versions.  First, at the site of the initial symbol definition,
>  we wrap the function with ``RTE_VERSION_SYMBOL``, passing the current ABI version,
> -the function return type, and the function name and its arguments.
> +the function return type, the function name and its arguments.

Good fix, though technically not relevant to this patch.

>  
>  .. code-block:: c
>  
> + -RTE_EXPORT_SYMBOL(rte_acl_create)
>   -struct rte_acl_ctx *
>   -rte_acl_create(const struct rte_acl_param *param)
>   +RTE_VERSION_SYMBOL(21, struct rte_acl_ctx *, rte_acl_create, (const struct rte_acl_param *param))
> @@ -293,6 +232,7 @@ We have now mapped the original rte_acl_create symbol to the original function
>  
>  Please see the section :ref:`Enabling versioning macros
>  <enabling_versioning_macros>` to enable this macro in the meson/ninja build.
> +

Ditto.

>  Next, we need to create the new version of the symbol. We create a new
>  function name and implement it appropriately, then wrap it in a call to ``RTE_DEFAULT_SYMBOL``.
>  
> @@ -312,9 +252,9 @@ The macro instructs the linker to create the new default symbol
>  ``rte_acl_create@DPDK_22``, which points to the function named ``rte_acl_create_v22``
>  (declared by the macro).
>  
> -And that's it, on the next shared library rebuild, there will be two versions of
> -rte_acl_create, an old DPDK_21 version, used by previously built applications,
> -and a new DPDK_22 version, used by future built applications.
> +And that's it. On the next shared library rebuild, there will be two versions of rte_acl_create,
> +an old DPDK_21 version, used by previously built applications, and a new DPDK_22 version,
> +used by future built applications.

nit: not sure what others think but "future built" sounds strange to me?
How about "later built" or "newly built"?

>  
>  .. note::
>  
> @@ -364,6 +304,7 @@ Assume we have an experimental function ``rte_acl_create`` as follows:
>      * Create an acl context object for apps to
>      * manipulate
>      */
<snip>

  reply	other threads:[~2025-03-13 17:27 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-05 21:23 [RFC] eal: add new function versioning macros David Marchand
2025-03-06  2:57 ` Patrick Robb
2025-03-06 10:23 ` Bruce Richardson
2025-03-06 12:50 ` [RFC v2 1/2] " David Marchand
2025-03-06 12:50   ` [RFC v2 2/2] build: generate symbol maps David Marchand
2025-03-06 15:45   ` [RFC v2 1/2] eal: add new function versioning macros Andre Muezerie
2025-03-11  9:55 ` [RFC v3 0/8] Symbol versioning and export rework David Marchand
2025-03-11  9:55   ` [RFC v3 1/8] lib: remove incorrect exported symbols David Marchand
2025-03-11  9:56   ` [RFC v3 2/8] drivers: " David Marchand
2025-03-11  9:56   ` [RFC v3 3/8] eal: rework function versioning macros David Marchand
2025-03-13 16:53     ` Bruce Richardson
2025-03-13 17:09       ` David Marchand
2025-03-11  9:56   ` [RFC v3 4/8] buildtools: display version when listing symbols David Marchand
2025-03-11  9:56   ` [RFC v3 5/8] build: generate symbol maps David Marchand
2025-03-13 17:26     ` Bruce Richardson [this message]
2025-03-14 15:38       ` David Marchand
2025-03-14 14:24     ` Thomas Monjalon
2025-03-14 15:38       ` David Marchand
2025-03-14 15:27     ` Andre Muezerie
2025-03-14 15:51       ` David Marchand
2025-03-11  9:56   ` [RFC v3 6/8] build: mark exported symbols David Marchand
2025-03-13 17:30     ` Bruce Richardson
2025-03-14 16:14       ` David Marchand
2025-03-14 16:23         ` Bruce Richardson
2025-03-14 16:53           ` David Marchand
2025-03-14 17:21             ` David Marchand
2025-03-14 17:28             ` Bruce Richardson
2025-03-14 17:39               ` David Marchand
2025-03-14 17:51                 ` Bruce Richardson
2025-03-11  9:56   ` [RFC v3 7/8] build: use dynamically generated version maps David Marchand
2025-03-11  9:56   ` [RFC v3 8/8] build: remove static " David Marchand
2025-03-11 10:18   ` [RFC v3 0/8] Symbol versioning and export rework Morten Brørup
2025-03-11 13:43     ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z9MVVzHCtYaVy2AV@bricha3-mobl1.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=andremue@linux.microsoft.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).