DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org, thomas@monjalon.net, andremue@linux.microsoft.com,
	 Tyler Retzlaff <roretzla@linux.microsoft.com>,
	Jasvinder Singh <jasvinder.singh@intel.com>
Subject: Re: [RFC v3 3/8] eal: rework function versioning macros
Date: Thu, 13 Mar 2025 18:09:26 +0100	[thread overview]
Message-ID: <CAJFAV8y3D3aqHwfDZQg+248OBOMB4SVYeL9qsQxFMFTT+SSnxg@mail.gmail.com> (raw)
In-Reply-To: <Z9MNiiyVYXrRBpJx@bricha3-mobl1.ger.corp.intel.com>

On Thu, Mar 13, 2025 at 5:54 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > diff --git a/doc/guides/contributing/abi_versioning.rst b/doc/guides/contributing/abi_versioning.rst
> > index 7afd1c1886..88dd776b4c 100644
> > --- a/doc/guides/contributing/abi_versioning.rst
> > +++ b/doc/guides/contributing/abi_versioning.rst
> > @@ -138,27 +138,20 @@ 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
> >  library so that older binaries need not be immediately recompiled.
> >
> > -The macros exported are:
> > +The macros are:
> >
> > -* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> > -  versioned symbol ``b@DPDK_n`` to the internal function ``be``.
> > +* ``RTE_VERSION_SYMBOL(ver, type, name, args``: Creates a symbol version table
>
> Missing closing brace .........................^ here
>
> > +  entry binding symbol ``<name>@DPDK_<ver>`` to the internal function name
> > +  ``<name>_v<ver>``.
> >
> > -* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry instructing
> > -  the linker to bind references to symbol ``b`` to the internal symbol
> > -  ``be``.
> > +* ``RTE_DEFAULT_SYMBO(ver, type, name, args)``: Creates a symbol version entry
>
> s/SYMBO/SYMBOL/

Good catch thanks...

>
> > +  instructing the linker to bind references to symbol ``<name>`` to the internal
> > +  symbol ``<name>_v<ver>``.
> >
> > -* ``MAP_STATIC_SYMBOL(f, p)``: Declare the prototype ``f``, and map it to the
> > -  fully qualified function ``p``, so that if a symbol becomes versioned, it
> > -  can still be mapped back to the public symbol name.
> > -
> > -* ``__vsym``:  Annotation to be used in a declaration of the internal symbol
> > -  ``be`` to signal that it is being used as an implementation of a particular
> > -  version of symbol ``b``.
> > -
> > -* ``VERSION_SYMBOL_EXPERIMENTAL(b, e)``: Creates a symbol version table entry
> > -  binding versioned symbol ``b@EXPERIMENTAL`` to the internal function ``be``.
> > -  The macro is used when a symbol matures to become part of the stable ABI, to
> > -  provide an alias to experimental until the next major ABI version.
> > +* ``RTE_VERSION_EXPERIMENTAL_SYMBOL(type, name, args)``:  Similar to RTE_VERSION_SYMBOL
> > +  but for experimental API symbols. The macro is used when a symbol matures
> > +  to become part of the stable ABI, to provide an alias to experimental
> > +  until the next major ABI version.
>
> Just to clarify - this is where we create two names/aliases for the one
> function, so it can be found either as an experimental version or a stable
> one, right? In that way it's actually quite different from
> RTE_VERSION_SYMBOL which is used to define a *NEW" version of an existing
> function, i.e. two functions rather than one function with two names.

I did not change the behavior, it is still mapping a symbol to an
actual implementation (there is an example later in this doc).
The previous description about an alias was probably inaccurate.


>
> >
> >  .. _example_abi_macro_usage:
> >
> > @@ -277,49 +270,36 @@ 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
> >  symbol at which versions.  First, at the site of the initial symbol definition,
> > -we need to update the function so that it is uniquely named, and not in conflict
> > -with the public symbol name
> > +we wrap the function with ``RTE_VERSION_SYMBOL``, passing the current ABI version,
> > +the function return type, and the function name and its arguments.
> >
> >  .. code-block:: c
> >
> >   -struct rte_acl_ctx *
> >   -rte_acl_create(const struct rte_acl_param *param)
> > - +struct rte_acl_ctx * __vsym
> > - +rte_acl_create_v21(const struct rte_acl_param *param)
> > + +RTE_VERSION_SYMBOL(21, struct rte_acl_ctx *, rte_acl_create, (const struct rte_acl_param *param))
> >   {
> >          size_t sz;
> >          struct rte_acl_ctx *ctx;
> >          ...
> > -
> > -Note that the base name of the symbol was kept intact, as this is conducive to
> > -the macros used for versioning symbols and we have annotated the function as
> > -``__vsym``, an implementation of a versioned symbol . That is our next step,
> > -mapping this new symbol name to the initial symbol name at version node 21.
> > -Immediately after the function, we add the VERSION_SYMBOL macro.
> > -
> > -.. code-block:: c
> > -
> > -   #include <rte_function_versioning.h>
> > -
> > -   ...
> > -   VERSION_SYMBOL(rte_acl_create, _v21, 21);
> > + }
> >
> >  Remembering to also add the rte_function_versioning.h header to the requisite c
> >  file where these changes are being made. The macro instructs the linker to
> >  create a new symbol ``rte_acl_create@DPDK_21``, which matches the symbol created
> > -in older builds, but now points to the above newly named function. We have now
> > -mapped the original rte_acl_create symbol to the original function (but with a
> > -new name).
> > +in older builds, but now points to the above newly named function ``rte_acl_create_v21``.
> > +We have now mapped the original rte_acl_create symbol to the original function
> > +(but with a new name).
> >
> >  Please see the section :ref:`Enabling versioning macros
> >  <enabling_versioning_macros>` to enable this macro in the meson/ninja build.
> > -Next, we need to create the new ``v22`` version of the symbol. We create a new
> > -function name, with the ``v22`` suffix, and implement it appropriately.
> > +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``.
> >
> >  .. code-block:: c
> >
> > -   struct rte_acl_ctx * __vsym
> > -   rte_acl_create_v22(const struct rte_acl_param *param, int debug);
> > +   RTE_DEFAULT_SYMBOL(22, struct rte_acl_ctx *, rte_acl_create, (const struct rte_acl_param *param,
> > +        int debug))
>
> Not directly relevant to the changes in this patch, but since this is
> documentation which doesn't actually need to be based on a real-life
> example, we should maybe come up with example functions which are short and
> don't need line wrapping. This example would be just as
> effective/instructive with a return value of "int" and parameter type
> without a "const" qualifier. :-)

Yep, I'll simplify the example.


>
> >     {
> >          struct rte_acl_ctx *ctx = rte_acl_create_v21(param);
> >
> > @@ -328,35 +308,9 @@ function name, with the ``v22`` suffix, and implement it appropriately.
> >          return ctx;
> >     }
> >
> > -This code serves as our new API call. Its the same as our old call, but adds the
> > -new parameter in place. Next we need to map this function to the new default
> > -symbol ``rte_acl_create@DPDK_22``. To do this, immediately after the function,
> > -we add the BIND_DEFAULT_SYMBOL macro.
> > -
> > -.. code-block:: c
> > -
> > -   #include <rte_function_versioning.h>
> > -
> > -   ...
> > -   BIND_DEFAULT_SYMBOL(rte_acl_create, _v22, 22);
> > -
> >  The macro instructs the linker to create the new default symbol
> > -``rte_acl_create@DPDK_22``, which points to the above newly named function.
> > -
> > -We finally modify the prototype of the call in the public header file,
> > -such that it contains both versions of the symbol and the public API.
> > -
> > -.. code-block:: c
> > -
> > -   struct rte_acl_ctx *
> > -   rte_acl_create(const struct rte_acl_param *param);
> > -
> > -   struct rte_acl_ctx * __vsym
> > -   rte_acl_create_v21(const struct rte_acl_param *param);
> > -
> > -   struct rte_acl_ctx * __vsym
> > -   rte_acl_create_v22(const struct rte_acl_param *param, int debug);
> > -
> > +``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,
> > @@ -365,43 +319,10 @@ and a new DPDK_22 version, used by future built applications.
> >  .. note::
> >
> >     **Before you leave**, please take care reviewing the sections on
> > -   :ref:`mapping static symbols <mapping_static_symbols>`,
> >     :ref:`enabling versioning macros <enabling_versioning_macros>`,
> >     and :ref:`ABI deprecation <abi_deprecation>`.
> >
> >
> > -.. _mapping_static_symbols:
> > -
> > -Mapping static symbols
> > -______________________
> > -
> > -Now we've taken what was a public symbol, and duplicated it into two uniquely
> > -and differently named symbols. We've then mapped each of those back to the
> > -public symbol ``rte_acl_create`` with different version tags. This only applies
> > -to dynamic linking, as static linking has no notion of versioning. That leaves
> > -this code in a position of no longer having a symbol simply named
> > -``rte_acl_create`` and a static build will fail on that missing symbol.
> > -
> > -To correct this, we can simply map a function of our choosing back to the public
> > -symbol in the static build with the ``MAP_STATIC_SYMBOL`` macro.  Generally the
> > -assumption is that the most recent version of the symbol is the one you want to
> > -map.  So, back in the C file where, immediately after ``rte_acl_create_v22`` is
> > -defined, we add this
> > -
> > -
> > -.. code-block:: c
> > -
> > -   struct rte_acl_ctx * __vsym
> > -   rte_acl_create_v22(const struct rte_acl_param *param, int debug)
> > -   {
> > -        ...
> > -   }
> > -   MAP_STATIC_SYMBOL(struct rte_acl_ctx *rte_acl_create(const struct rte_acl_param *param, int debug), rte_acl_create_v22);
> > -
> > -That tells the compiler that, when building a static library, any calls to the
> > -symbol ``rte_acl_create`` should be linked to ``rte_acl_create_v22``
> > -
> > -
> >  .. _enabling_versioning_macros:
> >
> >  Enabling versioning macros
> > @@ -519,26 +440,17 @@ and ``DPDK_22`` version nodes.
> >      * Create an acl context object for apps to
> >      * manipulate
> >      */
> > -   struct rte_acl_ctx *
> > -   rte_acl_create(const struct rte_acl_param *param)
> > +   RTE_DEFAULT_SYMBOL(22, struct rte_acl_ctx *, rte_acl_create,
> > +        (const struct rte_acl_param *param))
> >     {
> >     ...
> >     }
> >
> > -   __rte_experimental
> > -   struct rte_acl_ctx *
> > -   rte_acl_create_e(const struct rte_acl_param *param)
> > -   {
> > -      return rte_acl_create(param);
> > -   }
> > -   VERSION_SYMBOL_EXPERIMENTAL(rte_acl_create, _e);
> > -
> > -   struct rte_acl_ctx *
> > -   rte_acl_create_v22(const struct rte_acl_param *param)
> > +   RTE_VERSION_EXPERIMENTAL_SYMBOL(struct rte_acl_ctx *, rte_acl_create,
> > +        (const struct rte_acl_param *param))
> >     {
> >        return rte_acl_create(param);
> >     }
> > -   BIND_DEFAULT_SYMBOL(rte_acl_create, _v22, 22);
> >
> >  In the map file, we map the symbol to both the ``EXPERIMENTAL``
> >  and ``DPDK_22`` version nodes.
> > @@ -564,13 +476,6 @@ and ``DPDK_22`` version nodes.
> >          rte_acl_create;
> >     };
> >
> > -.. note::
> > -
> > -   Please note, similar to :ref:`symbol versioning <example_abi_macro_usage>`,
> > -   when aliasing to experimental you will also need to take care of
> > -   :ref:`mapping static symbols <mapping_static_symbols>`.
> > -
> > -
> >  .. _abi_deprecation:
> >
> >  Deprecating part of a public API
> > @@ -616,10 +521,10 @@ Next remove the corresponding versioned export.
> >
> >  .. code-block:: c
> >
> > - -VERSION_SYMBOL(rte_acl_create, _v21, 21);
> > + -RTE_VERSION_SYMBOL(21, struct rte_acl_ctx *, rte_acl_create, (const struct rte_acl_param *param))
> >
> >
> > -Note that the internal function definition could also be removed, but its used
> > +Note that the internal function definition must also be removed, but its used
>
> its -> it's (or "it is" if you want the longer version).

Ok, I'll fix this too.

>
> >  in our example by the newer version ``v22``, so we leave it in place and declare
> >  it as static. This is a coding style choice.
> >

[snip]

> > diff --git a/lib/eal/include/rte_function_versioning.h b/lib/eal/include/rte_function_versioning.h
> > index eb6dd2bc17..0020ce4885 100644
> > --- a/lib/eal/include/rte_function_versioning.h
> > +++ b/lib/eal/include/rte_function_versioning.h
> > @@ -11,8 +11,6 @@
> >  #error Use of function versioning disabled, is "use_function_versioning=true" in meson.build?
> >  #endif
> >
> > -#ifdef RTE_BUILD_SHARED_LIB
> > -
> >  /*
> >   * Provides backwards compatibility when updating exported functions.
> >   * When a symbol is exported from a library to provide an API, it also provides a
> > @@ -20,80 +18,54 @@
> >   * 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
> > - * 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
> > + * dynamically linked to the dpdk.
> >   */
> >
> > -/*
> > - * 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
> > - */
> > +#ifdef RTE_BUILD_SHARED_LIB
> >
> >  /*
> > - * VERSION_SYMBOL
> > - * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the internal
> > - * function name <b><e>
> > + * RTE_VERSION_SYMBOL
> > + * Creates a symbol version table entry binding symbol <name>@DPDK_<ver> to the internal
> > + * function name <name>_v<ver>.
> >   */
> > -#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
> > +#define RTE_VERSION_SYMBOL(ver, type, name, args) \
> > +__asm__(".symver " RTE_STR(name) "_v" RTE_STR(ver) ", " RTE_STR(name) "@DPDK_" RTE_STR(ver)); \
> > +__rte_used type name ## _v ## ver args; \
> > +type name ## _v ## ver args
> >
> >  /*
> > - * VERSION_SYMBOL_EXPERIMENTAL
> > - * Creates a symbol version table entry binding the symbol <b>@EXPERIMENTAL to the internal
> > - * function name <b><e>. The macro is used when a symbol matures to become part of the stable ABI,
> > - * to provide an alias to experimental for some time.
> > + * RTE_VERSION_EXPERIMENTAL_SYMBOL
> > + * Similar to RTE_VERSION_SYMBOL but for experimental API symbols.
> > + * This is mainly used for keeping compatibility for symbols that get promoted to stable ABI.
> >   */
> > -#define VERSION_SYMBOL_EXPERIMENTAL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@EXPERIMENTAL")
> > +#define RTE_VERSION_EXPERIMENTAL_SYMBOL(type, name, args) \
> > +__asm__(".symver " RTE_STR(name) "_exp, " RTE_STR(name) "@EXPERIMENTAL") \
> > +__rte_used type name ## _exp args; \
> > +type name ## _exp args
> >
> >  /*
> > - * BIND_DEFAULT_SYMBOL
> > + * RTE_DEFAULT_SYMBOL
> >   * Creates a symbol version entry instructing the linker to bind references to
> > - * symbol <b> to the internal symbol <b><e>
> > + * symbol <name> to the internal symbol <name>_v<ver>.
> >   */
> > -#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n))
> > +#define RTE_DEFAULT_SYMBOL(ver, type, name, args) \
> > +__asm__(".symver " RTE_STR(name) "_v" RTE_STR(ver) ", " RTE_STR(name) "@@DPDK_" RTE_STR(ver)); \
> > +__rte_used type name ## _v ## ver args; \
> > +type name ## _v ## ver args
> >
> > -/*
> > - * __vsym
> > - * Annotation to be used in declaration of the internal symbol <b><e> to signal
> > - * that it is being used as an implementation of a particular version of symbol
> > - * <b>.
> > - */
> > -#define __vsym __rte_used
> > +#else /* !RTE_BUILD_SHARED_LIB */
> >
> > -/*
> > - * 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)
> > +#define RTE_VERSION_SYMBOL(ver, type, name, args) \
> > +type name ## _v ## ver args; \
> > +type name ## _v ## ver args
> >
> > -#else
> > -/*
> > - * No symbol versioning in use
> > - */
> > -#define VERSION_SYMBOL(b, e, n)
> > -#define VERSION_SYMBOL_EXPERIMENTAL(b, e)
> > -#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
> > +#define RTE_VERSION_EXPERIMENTAL_SYMBOL(type, name, args) \
> > +type name ## _exp args; \
> > +type name ## _exp args
> > +
> > +#define RTE_DEFAULT_SYMBOL(ver, type, name, args) \
> > +type name args
> > +
> > +#endif /* RTE_BUILD_SHARED_LIB */
> >
> >  #endif /* _RTE_FUNCTION_VERSIONING_H_ */
>
> Changes to this file look ok to me.

Thank you Bruce.


-- 
David Marchand


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

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-05 21:23 [RFC] eal: add new " 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 [this message]
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
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=CAJFAV8y3D3aqHwfDZQg+248OBOMB4SVYeL9qsQxFMFTT+SSnxg@mail.gmail.com \
    --to=david.marchand@redhat.com \
    --cc=andremue@linux.microsoft.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=jasvinder.singh@intel.com \
    --cc=roretzla@linux.microsoft.com \
    --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).