DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
@ 2015-06-23 19:33 Neil Horman
  2015-06-23 19:33 ` [dpdk-dev] [PATCH 2/2] ABI: Add some documentation Neil Horman
                   ` (5 more replies)
  0 siblings, 6 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-23 19:33 UTC (permalink / raw)
  To: dev

Clean up some macro definition typos and comments

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas.monjalon@6wind.com
---
 lib/librte_compat/rte_compat.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index fb0dc19..75920a1 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -54,7 +54,7 @@
  * foo is exported as a global symbol.
  *
  * 2) rename the existing function int foo(char *string) to
- *	int __vsym foo_v20(char *string)
+ *	int foo_v20(char *string)
  *
  * 3) Add this macro immediately below the function
  *	VERSION_SYMBOL(foo, _v20, 2.0);
@@ -63,7 +63,7 @@
  *	char foo(int value, int otherval) { ...}
  *
  * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, 2.1);
+ *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
  *
  */
 
@@ -79,21 +79,21 @@
  * 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))
+#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
 
 /*
  * BASE_SYMBOL
  * Creates a symbol version table entry binding unversioned symbol <b>
  * to the internal function <b>_<e>
  */
-#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@")
+#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")
 
 /*
- * BNID_DEFAULT_SYMBOL
+ * 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 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))
 
 #else
@@ -103,7 +103,7 @@
 #define VERSION_SYMBOL(b, e, v)
 #define __vsym
 #define BASE_SYMBOL(b, n)
-#define BIND_DEFAULT_SYMBOL(b, v)
+#define BIND_DEFAULT_SYMBOL(b, e, n)
 
 /*
  * RTE_BUILD_SHARED_LIB=n
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCH 2/2] ABI: Add some documentation
  2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
@ 2015-06-23 19:33 ` Neil Horman
  2015-06-24 11:21   ` Mcnamara, John
  2015-06-24 11:23 ` [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Mcnamara, John
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-23 19:33 UTC (permalink / raw)
  To: dev

People have been asking for ways to use the ABI macros, heres some docs to
clarify their use.  Included is:

* An overview of what ABI is
* Details of the ABI deprecation process
* Details of the versioning macros
* Examples of their use
* Details of how to use the ABI validator

Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
working on it.  Much of the introductory material was gathered and cleaned up by
him

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: john.mcnamara@intel.com
CC: thomas.monjalon@6wind.com
---
 doc/guides/guidelines/index.rst      |   1 +
 doc/guides/guidelines/versioning.rst | 456 +++++++++++++++++++++++++++++++++++
 2 files changed, 457 insertions(+)
 create mode 100644 doc/guides/guidelines/versioning.rst

diff --git a/doc/guides/guidelines/index.rst b/doc/guides/guidelines/index.rst
index b2b0a92..251062e 100644
--- a/doc/guides/guidelines/index.rst
+++ b/doc/guides/guidelines/index.rst
@@ -6,3 +6,4 @@ Guidelines
     :numbered:
 
     coding_style
+    versioning
diff --git a/doc/guides/guidelines/versioning.rst b/doc/guides/guidelines/versioning.rst
new file mode 100644
index 0000000..79af924
--- /dev/null
+++ b/doc/guides/guidelines/versioning.rst
@@ -0,0 +1,456 @@
+Managing ABI updates
+====================
+
+Description
+-----------
+
+This document details some methods for handling ABI management in the DPDK.
+Note this document is not exhaustive, in that C library versioning is flexible
+allowing multiple methods to acheive various goals, but it will provide the user
+with some introductory methods
+
+General Guidelines
+------------------
+
+#. Whenever possible, ABI should be preserved
+#. The addition of symbols is generally not problematic
+#. The modification of symbols can generally be managed with versioning
+#. The removal of symbols generally is an ABI break and requires bumping of the
+   LIBABIVER macro
+
+What is an ABI
+--------------
+
+An ABI (Application Binary Interface) is the set of runtime interfaces exposed
+by a library. It is similar to an API (Application Programming Interface) but
+is the result of compilation.  It is also effectively cloned when applications
+link to dynamic libraries.  That is to say when an application is compiled to
+link against dynamic libraries, it is assumed that the ABI remains constant
+between the time the application is compiled/linked, and the time that it runs.
+Therefore, in the case of dynamic linking, it is critical that an ABI is
+preserved, or (when modified), done in such a way that the application is unable
+to behave improperly or in an unexpected fashion.
+
+The DPDK ABI policy
+-------------------
+
+ABI versions are set at the time of major release labeling, and the ABI may
+change multiple times, without warning, between the last release label and the
+HEAD label of the git tree.
+
+ABI versions, once released, are available until such time as their
+deprecation has been noted in the Release Notes for at least one major release
+cycle. For example consider the case where the ABI for DPDK 2.0 has been
+shipped and then a decision is made to modify it during the development of
+DPDK 2.1. The decision will be recorded in the Release Notes for the DPDK 2.1
+release and the modification will be made available in the DPDK 2.2 release.
+
+ABI versions may be deprecated in whole or in part as needed by a given
+update.
+
+Some ABI changes may be too significant to reasonably maintain multiple
+versions. In those cases ABI's may be updated without backward compatibility
+being provided. The requirements for doing so are:
+
+#. At least 3 acknowledgments of the need to do so must be made on the
+   dpdk.org mailing list.
+
+#. A full deprecation cycle, as explained above, must be made to offer
+   downstream consumers sufficient warning of the change.
+
+#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
+   incorporated must be incremented in parallel with the ABI changes
+   themselves.
+
+Note that the above process for ABI deprecation should not be undertaken
+lightly. ABI stability is extremely important for downstream consumers of the
+DPDK, especially when distributed in shared object form. Every effort should
+be made to preserve the ABI whenever possible. The ABI should only be changed
+for significant reasons, such as performance enhancements. ABI breakage due to
+changes such as reorganizing public structure fields for aesthetic or
+readability purposes should be avoided.
+
+Examples of Deprecation Notices
+-------------------------------
+
+The following are some examples of ABI deprecation notices which would be
+added to the Release Notes:
+
+* The Macro ``#RTE_FOO`` is deprecated and will be removed with version 2.0,
+  to be replaced with the inline function ``rte_foo()``.
+
+* The function ``rte_mbuf_grok()`` has been updated to include a new parameter
+  in version 2.0. Backwards compatibility will be maintained for this function
+  until the release of version 2.1
+
+* The members of ``struct rte_foo`` have been reorganized in release 2.0 for
+  performance reasons. Existing binary applications will have backwards
+  compatibility in release 2.0, while newly built binaries will need to
+  reference the new structure variant ``struct rte_foo2``. Compatibility will
+  be removed in release 2.2, and all applications will require updating and
+  rebuilding to the new structure at that time, which will be renamed to the
+  original ``struct rte_foo``.
+
+* Significant ABI changes are planned for the ``librte_dostuff`` library. The
+  upcoming release 2.0 will not contain these changes, but release 2.1 will,
+  and no backwards compatibility is planned due to the extensive nature of
+  these changes. Binaries using this library built prior to version 2.1 will
+  require updating and recompilation.
+
+Versioning Macros
+-----------------
+
+When a symbol is exported from a library to provide an API, it also provides a
+calling convention (ABI) that is embodied in its name, return type and
+arguments. Occasionally that function may need to change to accommodate new
+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 ``lib/librte_compat/rte_compat.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
+library so that older binaries need not be immediately recompiled.
+
+The macros exported are:
+
+* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
+  unversioned symbol ``b`` to the internal function ``b_e``.
+
+
+* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
+  unversioned symbol ``b`` to the internal function ``b_e``.
+
+* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry instructing
+  the linker to bind references to symbol ``b`` to the internal symbol
+  ``b_e``.
+
+
+Examples of ABI Macro use
+-------------------------
+
+Updating a public API
+~~~~~~~~~~~~~~~~~~~~~
+
+Assume we have a function as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param)
+ {
+        ...
+ }
+
+
+Assume that struct rte_acl_ctx is a private structure, and that a developer
+wishes to enhance the acl api so that a debugging flag can be enabled on a
+per-context basis.  This requires an addition to the structure (which, being
+private, is safe), but it also requries modifying the code as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param, int debug)
+ {
+        ...
+ }
+
+
+Note also that, being a public function, the header file prototype must also be
+changed, as must all the call sites, to reflect the new ABI footprint.  We will
+maintain previous ABI versions that are accessible only to previously compiled
+binaries
+
+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 alow 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
+
+.. code-block:: none 
+
+   DPDK_2.0 {
+        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_2.0 {
+        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_2.1 {
+        global:
+        rte_acl_create;
+
+ } DPDK_2.0;
+
+The addition of the new block tells the linker that a new version node is
+available (DPDK_2.1), whcih contains the symbol rte_acl_create, and inherits the
+symbols from the DPDK_2.0 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 map 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
+
+.. code-block:: c
+
+  struct rte_acl_ctx *
+ -rte_acl_create(const struct rte_acl_param *param)
+ +rte_acl_create_v20(const struct rte_acl_param *param)
+ {
+        size_t sz;
+        struct rte_acl_ctx *ctx;
+        ...
+
+Note that the base name of the symbol was kept in tact, as this is condusive to
+the macros used for versioning symbols.  That is our next step, mapping this new
+symbol name to the initial symbol name at version node 2.0.  Immediately after
+the function, we add this line of code
+
+.. code-block:: c
+
+   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+Remembering to also add the rte_compat.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
+the origional rte_acl_create symbol to the origional function (but with a new
+name)
+
+Next, we need to create the 2.1 version of the symbol.  We create a new function
+name, with a different suffix, and  implement it appropriately
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   rte_acl_create_v21(const struct rte_acl_param *param, int debug);
+   {
+        struct rte_acl_ctx *ctx = rte_acl_create_v20(param);
+
+        ctx->debug = debug;
+
+        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 symbol
+rte_acl_create@DPDK_2.1.  To do this, we modify the public prototype of the call
+in the header file, adding the macro there to inform all including applications,
+that on re-link, the default rte_acl_create symbol should point to this
+function.  Note that we could do this by simply naming the fuction above
+rte_acl_create, and the linker would chose the most recent version tag to apply
+in the version script, but we can also do this in the header file
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   -rte_acl_create(const struct rte_acl_param *param);
+   +rte_acl_create(const struct rte_acl_param *param, int debug);
+   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+The BIND_DEFAULT_SYMBOL macro explicitly tells applications that include this
+header, to link to the rte_acl_create_v21 function and apply the DPDK_2.1
+version node to it.  This method is more explicit and flexible than just
+reimplementing the exact symbol name, and allows for other features (such as
+linking to the old symbol version by default, when the new ABI is to be opt-in
+for a period.
+
+Thats it, on the next shared library rebuild, there will be two versions of
+rte_acl_create, an old DPDK_2.0 version, used by previously built applications,
+and a new DPDK_2.1 version, used by future built applications.
+
+
+Deprecating part of a public API
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Lets assume that you've done the above update, and after a few releases have
+passed you decide you would like to retire the old version of the function.
+After having gone through the ABI deprecation annoucement process, removal is
+easy.  Start by removing the symbol from the requisite version map file:
+
+.. code-block:: none
+
+   DPDK_2.0 {
+        global:
+
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+ -      rte_acl_create
+        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_2.1 {
+        global:
+        rte_acl_create;
+ } DPDK_2.0;
+
+
+Next remove the corresponding versioned export
+.. code-block:: c
+
+ -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+
+Note that the internal function definition could also be removed, but its used
+in our example by the newer version _v21, so we leave it in place.  This is a
+coding style choice.
+
+Lastly, we need to bump the LIBABIVER number for this library in the Makefile to
+indicate to applications doing dynamic linking that this is a later, and
+possibly incompatible library version:
+
+.. code-block:: c
+
+   -LIBABIVER := 1
+   +LIBABIVER := 2
+
+Deprecating an entire ABI version
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+While removing a symbol from and ABI may be useful, it is often more practical
+to remove an entire version node at once.  If a version node completely
+specifies an API, then removing part of it, typically makes it incomplete.  In
+those cases it is better to remove the entire node
+ 
+To do this, start by modifying the version map file, such that all symbols from
+the node to be removed are merged into the next node in the map
+
+In the case of our map above, it would transform to look as follows
+
+.. code-block:: none
+
+   DPDK_2.1 {              
+        global:
+              
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+        rte_acl_create
+        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: *;
+ };           
+
+Then any uses of BIND_DEFAULT_SYMBOL that ponited to the old node should be
+updated to point to the new version node in any header files for all affected
+symbols.
+
+.. code-block:: c
+
+ -BIND_DEFAULT_SYMBOL(rte_acl_create, _v20, 2.0);
+ +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+Lastly, any VERSION_SYMBOL macros that point to the old version node should be
+removed, taking care to keep, where neeed old code in place to support newer
+versions of the symbol.
+
+Running the ABI Validator
+-------------------------
+
+The ``scripts`` directory in the DPDK source tree contains a utility program,
+``validate-abi.sh``, for validating the DPDK ABI based on the Linux `ABI
+Compliance Checker
+<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_.
+
+This has a dependency on the ``abi-compliance-checker`` and ``and abi-dumper``
+utilities which can be installed via a package manager. For example::
+
+   sudo yum install abi-compliance-checker
+   sudo yum install abi-dumper
+
+The syntax of the ``validate-abi.sh`` utility is::
+
+   ./scripts/validate-abi.sh <TAG1> <TAG2> <TARGET>
+
+Where ``TAG1`` and ``TAG2`` are valid git tags on the local repo and target is
+the usual DPDK compilation target.
+
+For example to test the current committed HEAD against a previous release tag
+we could add a temporary tag and run the utility as follows::
+
+   git tag MY_TEMP_TAG
+   ./scripts/validate-abi.sh v2.0.0 MY_TEMP_TAG x86_64-native-linuxapp-gcc
+
+After the validation script completes (it can take a while since it need to
+compile both tags) it will create compatibility reports in the
+``./compat_report`` directory. Listed incompatibilities can be found as
+follows::
+
+  grep -lr Incompatible compat_reports/
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] ABI: Add some documentation
  2015-06-23 19:33 ` [dpdk-dev] [PATCH 2/2] ABI: Add some documentation Neil Horman
@ 2015-06-24 11:21   ` Mcnamara, John
  0 siblings, 0 replies; 41+ messages in thread
From: Mcnamara, John @ 2015-06-24 11:21 UTC (permalink / raw)
  To: Neil Horman, dev

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Tuesday, June 23, 2015 8:34 PM
> To: dev@dpdk.org
> Cc: Neil Horman; Mcnamara, John; thomas.monjalon@6wind.com
> Subject: [PATCH 2/2] ABI: Add some documentation
> 
> People have been asking for ways to use the ABI macros, heres some docs to
> clarify their use.  Included is:

Hi,

Thanks for this.

There are a few minor comments on the RST structure below.

Also, there is a conflict in the doc/guides/guidelines/index.rst file with an addition that just got merged. I just needs a rebase.


> +This file needs to be modified as follows
> +
> +.. code-block:: none
> +
> +   DPDK_2.0 {
> +        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_2.1 {
> +        global:
> +        rte_acl_create;
> +
> + } DPDK_2.0;

The last 7 lines of this verbatim section should be indented to the same level as the rest of the section. In general the code blocks should be indented at least 3 spaces to keep the various RST converters happy. That applies in a few places.


> +Note that the base name of the symbol was kept in tact, as this is
> +condusive to the macros used for versioning symbols.  That is our next
> +step, mapping this new symbol name to the initial symbol name at
> +version node 2.0.  Immediately after the function, we add this line of
> +code
> +
> +.. code-block:: c
> +
> +   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);VERSION_SYMBOL(rte_acl_create, _v20, 2.0);

The is a duplicate macro here.


> +
> +Remembering to also add the rte_compat.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

Could you enclose the symbol in RST backquotes ``rte_acl_create@DPDK_2.0`` since some of the renderers treat this as an email address! There is another one a few paragraphs down.


> +the symbol created in older builds, but now points to the above newly
> +named function.  We have now mapped the origional rte_acl_create symbol
> +to the origional function 

There are a few minor typos here and there.


> + };
> +
> + DPDK_2.1 {
> +        global:
> +        rte_acl_create;
> + } DPDK_2.0;

Same comment as above on indentation.


John.
-- 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
  2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
  2015-06-23 19:33 ` [dpdk-dev] [PATCH 2/2] ABI: Add some documentation Neil Horman
@ 2015-06-24 11:23 ` Mcnamara, John
  2015-06-24 18:06   ` Neil Horman
  2015-06-24 18:34 ` [dpdk-dev] [PATCHv2 " Neil Horman
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 41+ messages in thread
From: Mcnamara, John @ 2015-06-24 11:23 UTC (permalink / raw)
  To: Neil Horman, dev

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Tuesday, June 23, 2015 8:34 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> 
> Clean up some macro definition typos and comments

Hi,

There is a similar patch here:

    http://dpdk.org/dev/patchwork/patch/5709/

The other patch also removes the #ifdef RTE_BUILD_SHARED_LIB.

John.
-- 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
  2015-06-24 11:23 ` [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Mcnamara, John
@ 2015-06-24 18:06   ` Neil Horman
  0 siblings, 0 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-24 18:06 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev

On Wed, Jun 24, 2015 at 11:23:26AM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Tuesday, June 23, 2015 8:34 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> > 
> > Clean up some macro definition typos and comments
> 
> Hi,
> 
> There is a similar patch here:
> 
>     http://dpdk.org/dev/patchwork/patch/5709/
> 
> The other patch also removes the #ifdef RTE_BUILD_SHARED_LIB.
> 
> John.
> -- 
> 
> 
Yeah, I just NAK'ed that one, as it improperly removes the empty macro
definitions for use when you are doing a static build.  We need to keep that in
place so we dont' go aliasing symbols when were not supposed to


Neil

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos
  2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
  2015-06-23 19:33 ` [dpdk-dev] [PATCH 2/2] ABI: Add some documentation Neil Horman
  2015-06-24 11:23 ` [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Mcnamara, John
@ 2015-06-24 18:34 ` Neil Horman
  2015-06-24 18:34   ` [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation Neil Horman
  2015-06-24 19:41   ` [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos Thomas Monjalon
  2015-06-25  7:37 ` [dpdk-dev] [PATCH " Gajdzica, MaciejX T
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-24 18:34 UTC (permalink / raw)
  To: dev

Clean up some macro definition typos and comments

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas.monjalon@6wind.com
---
 lib/librte_compat/rte_compat.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index fb0dc19..75920a1 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -54,7 +54,7 @@
  * foo is exported as a global symbol.
  *
  * 2) rename the existing function int foo(char *string) to
- *	int __vsym foo_v20(char *string)
+ *	int foo_v20(char *string)
  *
  * 3) Add this macro immediately below the function
  *	VERSION_SYMBOL(foo, _v20, 2.0);
@@ -63,7 +63,7 @@
  *	char foo(int value, int otherval) { ...}
  *
  * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, 2.1);
+ *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
  *
  */
 
@@ -79,21 +79,21 @@
  * 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))
+#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
 
 /*
  * BASE_SYMBOL
  * Creates a symbol version table entry binding unversioned symbol <b>
  * to the internal function <b>_<e>
  */
-#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@")
+#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")
 
 /*
- * BNID_DEFAULT_SYMBOL
+ * 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 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))
 
 #else
@@ -103,7 +103,7 @@
 #define VERSION_SYMBOL(b, e, v)
 #define __vsym
 #define BASE_SYMBOL(b, n)
-#define BIND_DEFAULT_SYMBOL(b, v)
+#define BIND_DEFAULT_SYMBOL(b, e, n)
 
 /*
  * RTE_BUILD_SHARED_LIB=n
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-24 18:34 ` [dpdk-dev] [PATCHv2 " Neil Horman
@ 2015-06-24 18:34   ` Neil Horman
  2015-06-24 21:09     ` Thomas Monjalon
  2015-06-25  7:19     ` Zhang, Helin
  2015-06-24 19:41   ` [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos Thomas Monjalon
  1 sibling, 2 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-24 18:34 UTC (permalink / raw)
  To: dev

People have been asking for ways to use the ABI macros, heres some docs to
clarify their use.  Included is:

* An overview of what ABI is
* Details of the ABI deprecation process
* Details of the versioning macros
* Examples of their use
* Details of how to use the ABI validator

Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
working on it.  Much of the introductory material was gathered and cleaned up by
him

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: john.mcnamara@intel.com
CC: thomas.monjalon@6wind.com

Change notes:

v2)
     * Fixed RST indentations and spelling errors
     * Rebased to upstream to fix index.rst conflict
---
 doc/guides/guidelines/index.rst      |   1 +
 doc/guides/guidelines/versioning.rst | 456 +++++++++++++++++++++++++++++++++++
 2 files changed, 457 insertions(+)
 create mode 100644 doc/guides/guidelines/versioning.rst

diff --git a/doc/guides/guidelines/index.rst b/doc/guides/guidelines/index.rst
index 0ee9ab3..bfb9fa3 100644
--- a/doc/guides/guidelines/index.rst
+++ b/doc/guides/guidelines/index.rst
@@ -7,3 +7,4 @@ Guidelines
 
     coding_style
     design
+    versioning
diff --git a/doc/guides/guidelines/versioning.rst b/doc/guides/guidelines/versioning.rst
new file mode 100644
index 0000000..2aef526
--- /dev/null
+++ b/doc/guides/guidelines/versioning.rst
@@ -0,0 +1,456 @@
+Managing ABI updates
+====================
+
+Description
+-----------
+
+This document details some methods for handling ABI management in the DPDK.
+Note this document is not exhaustive, in that C library versioning is flexible
+allowing multiple methods to achieve various goals, but it will provide the user
+with some introductory methods
+
+General Guidelines
+------------------
+
+#. Whenever possible, ABI should be preserved
+#. The addition of symbols is generally not problematic
+#. The modification of symbols can generally be managed with versioning
+#. The removal of symbols generally is an ABI break and requires bumping of the
+   LIBABIVER macro
+
+What is an ABI
+--------------
+
+An ABI (Application Binary Interface) is the set of runtime interfaces exposed
+by a library. It is similar to an API (Application Programming Interface) but
+is the result of compilation.  It is also effectively cloned when applications
+link to dynamic libraries.  That is to say when an application is compiled to
+link against dynamic libraries, it is assumed that the ABI remains constant
+between the time the application is compiled/linked, and the time that it runs.
+Therefore, in the case of dynamic linking, it is critical that an ABI is
+preserved, or (when modified), done in such a way that the application is unable
+to behave improperly or in an unexpected fashion.
+
+The DPDK ABI policy
+-------------------
+
+ABI versions are set at the time of major release labeling, and the ABI may
+change multiple times, without warning, between the last release label and the
+HEAD label of the git tree.
+
+ABI versions, once released, are available until such time as their
+deprecation has been noted in the Release Notes for at least one major release
+cycle. For example consider the case where the ABI for DPDK 2.0 has been
+shipped and then a decision is made to modify it during the development of
+DPDK 2.1. The decision will be recorded in the Release Notes for the DPDK 2.1
+release and the modification will be made available in the DPDK 2.2 release.
+
+ABI versions may be deprecated in whole or in part as needed by a given
+update.
+
+Some ABI changes may be too significant to reasonably maintain multiple
+versions. In those cases ABI's may be updated without backward compatibility
+being provided. The requirements for doing so are:
+
+#. At least 3 acknowledgments of the need to do so must be made on the
+   dpdk.org mailing list.
+
+#. A full deprecation cycle, as explained above, must be made to offer
+   downstream consumers sufficient warning of the change.
+
+#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
+   incorporated must be incremented in parallel with the ABI changes
+   themselves.
+
+Note that the above process for ABI deprecation should not be undertaken
+lightly. ABI stability is extremely important for downstream consumers of the
+DPDK, especially when distributed in shared object form. Every effort should
+be made to preserve the ABI whenever possible. The ABI should only be changed
+for significant reasons, such as performance enhancements. ABI breakage due to
+changes such as reorganizing public structure fields for aesthetic or
+readability purposes should be avoided.
+
+Examples of Deprecation Notices
+-------------------------------
+
+The following are some examples of ABI deprecation notices which would be
+added to the Release Notes:
+
+* The Macro ``#RTE_FOO`` is deprecated and will be removed with version 2.0,
+  to be replaced with the inline function ``rte_foo()``.
+
+* The function ``rte_mbuf_grok()`` has been updated to include a new parameter
+  in version 2.0. Backwards compatibility will be maintained for this function
+  until the release of version 2.1
+
+* The members of ``struct rte_foo`` have been reorganized in release 2.0 for
+  performance reasons. Existing binary applications will have backwards
+  compatibility in release 2.0, while newly built binaries will need to
+  reference the new structure variant ``struct rte_foo2``. Compatibility will
+  be removed in release 2.2, and all applications will require updating and
+  rebuilding to the new structure at that time, which will be renamed to the
+  original ``struct rte_foo``.
+
+* Significant ABI changes are planned for the ``librte_dostuff`` library. The
+  upcoming release 2.0 will not contain these changes, but release 2.1 will,
+  and no backwards compatibility is planned due to the extensive nature of
+  these changes. Binaries using this library built prior to version 2.1 will
+  require updating and recompilation.
+
+Versioning Macros
+-----------------
+
+When a symbol is exported from a library to provide an API, it also provides a
+calling convention (ABI) that is embodied in its name, return type and
+arguments. Occasionally that function may need to change to accommodate new
+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 ``lib/librte_compat/rte_compat.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
+library so that older binaries need not be immediately recompiled.
+
+The macros exported are:
+
+* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
+  unversioned symbol ``b`` to the internal function ``b_e``.
+
+
+* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
+  unversioned symbol ``b`` to the internal function ``b_e``.
+
+* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry instructing
+  the linker to bind references to symbol ``b`` to the internal symbol
+  ``b_e``.
+
+
+Examples of ABI Macro use
+-------------------------
+
+Updating a public API
+~~~~~~~~~~~~~~~~~~~~~
+
+Assume we have a function as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param)
+ {
+        ...
+ }
+
+
+Assume that struct rte_acl_ctx is a private structure, and that a developer
+wishes to enhance the acl api so that a debugging flag can be enabled on a
+per-context basis.  This requires an addition to the structure (which, being
+private, is safe), but it also requires modifying the code as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param, int debug)
+ {
+        ...
+ }
+
+
+Note also that, being a public function, the header file prototype must also be
+changed, as must all the call sites, to reflect the new ABI footprint.  We will
+maintain previous ABI versions that are accessible only to previously compiled
+binaries
+
+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
+
+.. code-block:: none 
+
+   DPDK_2.0 {
+        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_2.0 {
+        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_2.1 {
+        global:
+        rte_acl_create;
+
+   } DPDK_2.0;
+
+The addition of the new block tells the linker that a new version node is
+available (DPDK_2.1), which contains the symbol rte_acl_create, and inherits the
+symbols from the DPDK_2.0 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 map 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
+
+.. code-block:: c
+
+  struct rte_acl_ctx *
+ -rte_acl_create(const struct rte_acl_param *param)
+ +rte_acl_create_v20(const struct rte_acl_param *param)
+ {
+        size_t sz;
+        struct rte_acl_ctx *ctx;
+        ...
+
+Note that the base name of the symbol was kept in tact, as this is condusive to
+the macros used for versioning symbols.  That is our next step, mapping this new
+symbol name to the initial symbol name at version node 2.0.  Immediately after
+the function, we add this line of code
+
+.. code-block:: c
+
+   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+Remembering to also add the rte_compat.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
+the original rte_acl_create symbol to the original function (but with a new
+name)
+
+Next, we need to create the 2.1 version of the symbol.  We create a new function
+name, with a different suffix, and  implement it appropriately
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   rte_acl_create_v21(const struct rte_acl_param *param, int debug);
+   {
+        struct rte_acl_ctx *ctx = rte_acl_create_v20(param);
+
+        ctx->debug = debug;
+
+        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 symbol
+``rte_acl_create@DPDK_2.1``.  To do this, we modify the public prototype of the call
+in the header file, adding the macro there to inform all including applications,
+that on re-link, the default rte_acl_create symbol should point to this
+function.  Note that we could do this by simply naming the function above
+rte_acl_create, and the linker would chose the most recent version tag to apply
+in the version script, but we can also do this in the header file
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   -rte_acl_create(const struct rte_acl_param *param);
+   +rte_acl_create(const struct rte_acl_param *param, int debug);
+   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+The BIND_DEFAULT_SYMBOL macro explicitly tells applications that include this
+header, to link to the rte_acl_create_v21 function and apply the DPDK_2.1
+version node to it.  This method is more explicit and flexible than just
+re-implementing the exact symbol name, and allows for other features (such as
+linking to the old symbol version by default, when the new ABI is to be opt-in
+for a period.
+
+That's it, on the next shared library rebuild, there will be two versions of
+rte_acl_create, an old DPDK_2.0 version, used by previously built applications,
+and a new DPDK_2.1 version, used by future built applications.
+
+
+Deprecating part of a public API
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Lets assume that you've done the above update, and after a few releases have
+passed you decide you would like to retire the old version of the function.
+After having gone through the ABI deprecation announcement process, removal is
+easy.  Start by removing the symbol from the requisite version map file:
+
+.. code-block:: none
+
+   DPDK_2.0 {
+        global:
+
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+ -      rte_acl_create
+        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_2.1 {
+        global:
+        rte_acl_create;
+   } DPDK_2.0;
+
+
+Next remove the corresponding versioned export
+.. code-block:: c
+
+ -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+
+Note that the internal function definition could also be removed, but its used
+in our example by the newer version _v21, so we leave it in place.  This is a
+coding style choice.
+
+Lastly, we need to bump the LIBABIVER number for this library in the Makefile to
+indicate to applications doing dynamic linking that this is a later, and
+possibly incompatible library version:
+
+.. code-block:: c
+
+   -LIBABIVER := 1
+   +LIBABIVER := 2
+
+Deprecating an entire ABI version
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+While removing a symbol from and ABI may be useful, it is often more practical
+to remove an entire version node at once.  If a version node completely
+specifies an API, then removing part of it, typically makes it incomplete.  In
+those cases it is better to remove the entire node
+ 
+To do this, start by modifying the version map file, such that all symbols from
+the node to be removed are merged into the next node in the map
+
+In the case of our map above, it would transform to look as follows
+
+.. code-block:: none
+
+   DPDK_2.1 {              
+        global:
+              
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+        rte_acl_create
+        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: *;
+ };           
+
+Then any uses of BIND_DEFAULT_SYMBOL that pointed to the old node should be
+updated to point to the new version node in any header files for all affected
+symbols.
+
+.. code-block:: c
+
+ -BIND_DEFAULT_SYMBOL(rte_acl_create, _v20, 2.0);
+ +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+Lastly, any VERSION_SYMBOL macros that point to the old version node should be
+removed, taking care to keep, where need old code in place to support newer
+versions of the symbol.
+
+Running the ABI Validator
+-------------------------
+
+The ``scripts`` directory in the DPDK source tree contains a utility program,
+``validate-abi.sh``, for validating the DPDK ABI based on the Linux `ABI
+Compliance Checker
+<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_.
+
+This has a dependency on the ``abi-compliance-checker`` and ``and abi-dumper``
+utilities which can be installed via a package manager. For example::
+
+   sudo yum install abi-compliance-checker
+   sudo yum install abi-dumper
+
+The syntax of the ``validate-abi.sh`` utility is::
+
+   ./scripts/validate-abi.sh <TAG1> <TAG2> <TARGET>
+
+Where ``TAG1`` and ``TAG2`` are valid git tags on the local repo and target is
+the usual DPDK compilation target.
+
+For example to test the current committed HEAD against a previous release tag
+we could add a temporary tag and run the utility as follows::
+
+   git tag MY_TEMP_TAG
+   ./scripts/validate-abi.sh v2.0.0 MY_TEMP_TAG x86_64-native-linuxapp-gcc
+
+After the validation script completes (it can take a while since it need to
+compile both tags) it will create compatibility reports in the
+``./compat_report`` directory. Listed incompatibilities can be found as
+follows::
+
+  grep -lr Incompatible compat_reports/
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos
  2015-06-24 18:34 ` [dpdk-dev] [PATCHv2 " Neil Horman
  2015-06-24 18:34   ` [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation Neil Horman
@ 2015-06-24 19:41   ` Thomas Monjalon
  2015-06-24 20:15     ` Neil Horman
  1 sibling, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-24 19:41 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-24 14:34, Neil Horman:
> @@ -103,7 +103,7 @@
>  #define VERSION_SYMBOL(b, e, v)

Should be (b, e, n)

>  #define __vsym
>  #define BASE_SYMBOL(b, n)

Should be (b, e)

> -#define BIND_DEFAULT_SYMBOL(b, v)
> +#define BIND_DEFAULT_SYMBOL(b, e, n)

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos
  2015-06-24 19:41   ` [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos Thomas Monjalon
@ 2015-06-24 20:15     ` Neil Horman
  2015-06-24 20:49       ` Thomas Monjalon
  0 siblings, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-24 20:15 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Wed, Jun 24, 2015 at 09:41:08PM +0200, Thomas Monjalon wrote:
> 2015-06-24 14:34, Neil Horman:
> > @@ -103,7 +103,7 @@
> >  #define VERSION_SYMBOL(b, e, v)
> 
> Should be (b, e, n)
> 
> >  #define __vsym
> >  #define BASE_SYMBOL(b, n)
> 
> Should be (b, e)
> 
> > -#define BIND_DEFAULT_SYMBOL(b, v)
> > +#define BIND_DEFAULT_SYMBOL(b, e, n)
> 
> 
> 
Agreed, though thats really just cosmetic, given that the error is in the empty
defines.  Can you fix it in patchwork by hand, or would you prefer a repost?

Neil

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos
  2015-06-24 20:15     ` Neil Horman
@ 2015-06-24 20:49       ` Thomas Monjalon
  0 siblings, 0 replies; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-24 20:49 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-24 16:15, Neil Horman:
> On Wed, Jun 24, 2015 at 09:41:08PM +0200, Thomas Monjalon wrote:
> > 2015-06-24 14:34, Neil Horman:
> > > @@ -103,7 +103,7 @@
> > >  #define VERSION_SYMBOL(b, e, v)
> > 
> > Should be (b, e, n)
> > 
> > >  #define __vsym
> > >  #define BASE_SYMBOL(b, n)
> > 
> > Should be (b, e)
> > 
> > > -#define BIND_DEFAULT_SYMBOL(b, v)
> > > +#define BIND_DEFAULT_SYMBOL(b, e, n)
> > 
> > 
> > 
> Agreed, though thats really just cosmetic, given that the error is in the empty
> defines.  Can you fix it in patchwork by hand, or would you prefer a repost?

I can manage it but I think you'll need a repost for the patch 2/2 anyway.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-24 18:34   ` [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation Neil Horman
@ 2015-06-24 21:09     ` Thomas Monjalon
  2015-06-25 11:35       ` Neil Horman
  2015-06-25  7:19     ` Zhang, Helin
  1 sibling, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-24 21:09 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-24 14:34, Neil Horman:
> +Some ABI changes may be too significant to reasonably maintain multiple
> +versions. In those cases ABI's may be updated without backward compatibility
> +being provided. The requirements for doing so are:
> +
> +#. At least 3 acknowledgments of the need to do so must be made on the
> +   dpdk.org mailing list.
> +
> +#. A full deprecation cycle, as explained above, must be made to offer
> +   downstream consumers sufficient warning of the change.
> +
> +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
> +   incorporated must be incremented in parallel with the ABI changes
> +   themselves.

The proposal was to provide the old and the new ABI in the same source code
during the deprecation cycle. The old ABI would be the default and people
can build the new one by enabling the NEXT_ABI config option.
So the migration to the new ABI is smoother.


[...]
> +The macros exported are:
> +
> +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.

The definition is the same as BASE_SYMBOL.

> +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.


[...]
> +   DPDK_2.0 {
> +        global:
> +
> +        rte_acl_add_rules;
> +        rte_acl_build;
> +        rte_acl_classify;
> +        rte_acl_classify_alg;
> +        rte_acl_classify_scalar;
> +        rte_acl_create;

So it's declared twice, right?
I think it should be explicit.

> +        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_2.1 {
> +        global:
> +        rte_acl_create;
> +
> +   } DPDK_2.0;

[...]
> +Note that the base name of the symbol was kept in tact, as this is condusive to

s/in tact/intact/?

[...]
> +the macros used for versioning symbols.  That is our next step, mapping this new
> +symbol name to the initial symbol name at version node 2.0.  Immediately after
> +the function, we add this line of code
> +
> +.. code-block:: c
> +
> +   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);

Can it be declared before the function?

[...]
> +Remembering to also add the rte_compat.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
> +the original rte_acl_create symbol to the original function (but with a new
> +name)

Could we use VERSION_SYMBOL(rte_acl_create, , 2.0);
when introducing the function in DPDK 2.0 (before any ABI breakage)?
It could help to generate the .map file.

When do we need to use BASE_SYMBOL?

[...]
> +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 symbol
> +``rte_acl_create@DPDK_2.1``.  To do this, we modify the public prototype of the call
> +in the header file, adding the macro there to inform all including applications,
> +that on re-link, the default rte_acl_create symbol should point to this
> +function.  Note that we could do this by simply naming the function above
> +rte_acl_create, and the linker would chose the most recent version tag to apply
> +in the version script, but we can also do this in the header file
> +
> +.. code-block:: c
> +
> +   struct rte_acl_ctx *
> +   -rte_acl_create(const struct rte_acl_param *param);
> +   +rte_acl_create(const struct rte_acl_param *param, int debug);
> +   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);

Will it work with static library?

> +Next remove the corresponding versioned export
> +.. code-block:: c
> +
> + -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
> +
> +
> +Note that the internal function definition could also be removed, but its used
> +in our example by the newer version _v21, so we leave it in place.  This is a
> +coding style choice.
> +
> +Lastly, we need to bump the LIBABIVER number for this library in the Makefile to
> +indicate to applications doing dynamic linking that this is a later, and
> +possibly incompatible library version:
> +
> +.. code-block:: c
> +
> +   -LIBABIVER := 1
> +   +LIBABIVER := 2

Very well explained, thanks.

[...]
> +        rte_acl_add_rules;
> +        rte_acl_build;
> +        rte_acl_classify;
> +        rte_acl_classify_alg;
> +        rte_acl_classify_scalar;
> +        rte_acl_dump;
> +        rte_acl_create

Not in alphabetical order.


As you copy a part of abi.rst, it should be removed from the original doc.

Thanks Neil

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-24 18:34   ` [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation Neil Horman
  2015-06-24 21:09     ` Thomas Monjalon
@ 2015-06-25  7:19     ` Zhang, Helin
  2015-06-25  7:42       ` Gonzalez Monroy, Sergio
  2015-06-25 12:25       ` Neil Horman
  1 sibling, 2 replies; 41+ messages in thread
From: Zhang, Helin @ 2015-06-25  7:19 UTC (permalink / raw)
  To: Neil Horman, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Thursday, June 25, 2015 2:35 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
> 
> People have been asking for ways to use the ABI macros, heres some docs to
> clarify their use.  Included is:
> 
> * An overview of what ABI is
> * Details of the ABI deprecation process
> * Details of the versioning macros
> * Examples of their use
> * Details of how to use the ABI validator
> 
> Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
> working on it.  Much of the introductory material was gathered and cleaned up
> by him
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: john.mcnamara@intel.com
> CC: thomas.monjalon@6wind.com
> 
> Change notes:
> 
> v2)
>      * Fixed RST indentations and spelling errors
>      * Rebased to upstream to fix index.rst conflict
> ---
>  doc/guides/guidelines/index.rst      |   1 +
>  doc/guides/guidelines/versioning.rst | 456
> +++++++++++++++++++++++++++++++++++
>  2 files changed, 457 insertions(+)
>  create mode 100644 doc/guides/guidelines/versioning.rst
> 
> diff --git a/doc/guides/guidelines/index.rst b/doc/guides/guidelines/index.rst
> index 0ee9ab3..bfb9fa3 100644
> --- a/doc/guides/guidelines/index.rst
> +++ b/doc/guides/guidelines/index.rst
> @@ -7,3 +7,4 @@ Guidelines
> 
>      coding_style
>      design
> +    versioning
> diff --git a/doc/guides/guidelines/versioning.rst
> b/doc/guides/guidelines/versioning.rst
> new file mode 100644
> index 0000000..2aef526
> --- /dev/null
> +++ b/doc/guides/guidelines/versioning.rst
> @@ -0,0 +1,456 @@
> +Managing ABI updates
> +====================
> +
> +Description
> +-----------
> +
> +This document details some methods for handling ABI management in the
> DPDK.
> +Note this document is not exhaustive, in that C library versioning is
> +flexible allowing multiple methods to achieve various goals, but it
> +will provide the user with some introductory methods
> +
> +General Guidelines
> +------------------
> +
> +#. Whenever possible, ABI should be preserved #. The addition of
> +symbols is generally not problematic #. The modification of symbols can
> +generally be managed with versioning #. The removal of symbols
> +generally is an ABI break and requires bumping of the
> +   LIBABIVER macro
> +
> +What is an ABI
> +--------------
> +
> +An ABI (Application Binary Interface) is the set of runtime interfaces
> +exposed by a library. It is similar to an API (Application Programming
> +Interface) but is the result of compilation.  It is also effectively
> +cloned when applications link to dynamic libraries.  That is to say
> +when an application is compiled to link against dynamic libraries, it
> +is assumed that the ABI remains constant between the time the application is
> compiled/linked, and the time that it runs.
> +Therefore, in the case of dynamic linking, it is critical that an ABI
> +is preserved, or (when modified), done in such a way that the
> +application is unable to behave improperly or in an unexpected fashion.
> +
> +The DPDK ABI policy
> +-------------------
> +
> +ABI versions are set at the time of major release labeling, and the ABI
> +may change multiple times, without warning, between the last release
> +label and the HEAD label of the git tree.
> +
> +ABI versions, once released, are available until such time as their
> +deprecation has been noted in the Release Notes for at least one major
> +release cycle. For example consider the case where the ABI for DPDK 2.0
> +has been shipped and then a decision is made to modify it during the
> +development of DPDK 2.1. The decision will be recorded in the Release
> +Notes for the DPDK 2.1 release and the modification will be made available in
> the DPDK 2.2 release.
> +
> +ABI versions may be deprecated in whole or in part as needed by a given
> +update.
> +
> +Some ABI changes may be too significant to reasonably maintain multiple
> +versions. In those cases ABI's may be updated without backward
> +compatibility being provided. The requirements for doing so are:
> +
> +#. At least 3 acknowledgments of the need to do so must be made on the
> +   dpdk.org mailing list.
> +
> +#. A full deprecation cycle, as explained above, must be made to offer
> +   downstream consumers sufficient warning of the change.
> +
> +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
> +   incorporated must be incremented in parallel with the ABI changes
> +   themselves.
> +
> +Note that the above process for ABI deprecation should not be
> +undertaken lightly. ABI stability is extremely important for downstream
> +consumers of the DPDK, especially when distributed in shared object
> +form. Every effort should be made to preserve the ABI whenever
> +possible. The ABI should only be changed for significant reasons, such
> +as performance enhancements. ABI breakage due to changes such as
> +reorganizing public structure fields for aesthetic or readability purposes should
> be avoided.
> +
> +Examples of Deprecation Notices
> +-------------------------------
> +
> +The following are some examples of ABI deprecation notices which would
> +be added to the Release Notes:
> +
> +* The Macro ``#RTE_FOO`` is deprecated and will be removed with version
> +2.0,
> +  to be replaced with the inline function ``rte_foo()``.
> +
> +* The function ``rte_mbuf_grok()`` has been updated to include a new
> +parameter
> +  in version 2.0. Backwards compatibility will be maintained for this
> +function
> +  until the release of version 2.1
> +
> +* The members of ``struct rte_foo`` have been reorganized in release
> +2.0 for
> +  performance reasons. Existing binary applications will have backwards
> +  compatibility in release 2.0, while newly built binaries will need to
> +  reference the new structure variant ``struct rte_foo2``.
> +Compatibility will
> +  be removed in release 2.2, and all applications will require updating
> +and
> +  rebuilding to the new structure at that time, which will be renamed
> +to the
> +  original ``struct rte_foo``.
> +
> +* Significant ABI changes are planned for the ``librte_dostuff``
> +library. The
> +  upcoming release 2.0 will not contain these changes, but release 2.1
> +will,
> +  and no backwards compatibility is planned due to the extensive nature
> +of
> +  these changes. Binaries using this library built prior to version 2.1
> +will
> +  require updating and recompilation.
> +
> +Versioning Macros
> +-----------------
> +
> +When a symbol is exported from a library to provide an API, it also
> +provides a calling convention (ABI) that is embodied in its name,
> +return type and arguments. Occasionally that function may need to
> +change to accommodate new 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
> +``lib/librte_compat/rte_compat.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 library so that older binaries
> need not be immediately recompiled.
> +
> +The macros exported are:
> +
> +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry
> +binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.
> +
> +
> +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.
> +
> +* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry
> +instructing
> +  the linker to bind references to symbol ``b`` to the internal symbol
> +  ``b_e``.
> +
> +
> +Examples of ABI Macro use
> +-------------------------
> +
> +Updating a public API
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +Assume we have a function as follows
> +
> +.. code-block:: c
> +
> + /*
> +  * Create an acl context object for apps to
> +  * manipulate
> +  */
> + struct rte_acl_ctx *
> + rte_acl_create(const struct rte_acl_param *param) {
> +        ...
> + }
> +
> +
> +Assume that struct rte_acl_ctx is a private structure, and that a
> +developer wishes to enhance the acl api so that a debugging flag can be
> +enabled on a per-context basis.  This requires an addition to the
> +structure (which, being private, is safe), but it also requires
> +modifying the code as follows
> +
> +.. code-block:: c
> +
> + /*
> +  * Create an acl context object for apps to
> +  * manipulate
> +  */
> + struct rte_acl_ctx *
> + rte_acl_create(const struct rte_acl_param *param, int debug) {
> +        ...
> + }
> +
> +
> +Note also that, being a public function, the header file prototype must
> +also be changed, as must all the call sites, to reflect the new ABI
> +footprint.  We will maintain previous ABI versions that are accessible
> +only to previously compiled binaries
> +
> +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
> +
> +.. code-block:: none
> +
> +   DPDK_2.0 {
> +        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_2.0 {
> +        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_2.1 {
> +        global:
> +        rte_acl_create;
One question, does it need a line of "local: *;", like it did in
librte_ether/rte_ether_version.map?

> +
> +   } DPDK_2.0;
> +
> +The addition of the new block tells the linker that a new version node
> +is available (DPDK_2.1), which contains the symbol rte_acl_create, and
> +inherits the symbols from the DPDK_2.0 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 map 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
> +
> +.. code-block:: c
> +
> +  struct rte_acl_ctx *
> + -rte_acl_create(const struct rte_acl_param *param)
> + +rte_acl_create_v20(const struct rte_acl_param *param)
> + {
> +        size_t sz;
> +        struct rte_acl_ctx *ctx;
> +        ...
> +
> +Note that the base name of the symbol was kept in tact, as this is
> +condusive to the macros used for versioning symbols.  That is our next
> +step, mapping this new symbol name to the initial symbol name at
> +version node 2.0.  Immediately after the function, we add this line of
> +code
> +
> +.. code-block:: c
> +
> +   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
> +
> +Remembering to also add the rte_compat.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 the original rte_acl_create
> +symbol to the original function (but with a new
> +name)
> +
> +Next, we need to create the 2.1 version of the symbol.  We create a new
> +function name, with a different suffix, and  implement it appropriately
> +
> +.. code-block:: c
> +
> +   struct rte_acl_ctx *
> +   rte_acl_create_v21(const struct rte_acl_param *param, int debug);
> +   {
> +        struct rte_acl_ctx *ctx = rte_acl_create_v20(param);
> +
> +        ctx->debug = debug;
> +
> +        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 symbol ``rte_acl_create@DPDK_2.1``.  To do this, we modify the
> +public prototype of the call in the header file, adding the macro there
> +to inform all including applications, that on re-link, the default
> +rte_acl_create symbol should point to this function.  Note that we
> +could do this by simply naming the function above rte_acl_create, and
> +the linker would chose the most recent version tag to apply in the
> +version script, but we can also do this in the header file
> +
> +.. code-block:: c
> +
> +   struct rte_acl_ctx *
> +   -rte_acl_create(const struct rte_acl_param *param);
> +   +rte_acl_create(const struct rte_acl_param *param, int debug);
> +   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
> +
> +The BIND_DEFAULT_SYMBOL macro explicitly tells applications that
> +include this header, to link to the rte_acl_create_v21 function and
> +apply the DPDK_2.1 version node to it.  This method is more explicit
> +and flexible than just re-implementing the exact symbol name, and
> +allows for other features (such as linking to the old symbol version by
> +default, when the new ABI is to be opt-in for a period.
> +
> +That's it, on the next shared library rebuild, there will be two
> +versions of rte_acl_create, an old DPDK_2.0 version, used by previously
> +built applications, and a new DPDK_2.1 version, used by future built
> applications.
> +
> +
> +Deprecating part of a public API
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Lets assume that you've done the above update, and after a few releases
> +have passed you decide you would like to retire the old version of the function.
> +After having gone through the ABI deprecation announcement process,
> +removal is easy.  Start by removing the symbol from the requisite version map
> file:
> +
> +.. code-block:: none
> +
> +   DPDK_2.0 {
> +        global:
> +
> +        rte_acl_add_rules;
> +        rte_acl_build;
> +        rte_acl_classify;
> +        rte_acl_classify_alg;
> +        rte_acl_classify_scalar;
> +        rte_acl_dump;
> + -      rte_acl_create
> +        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_2.1 {
> +        global:
> +        rte_acl_create;
> +   } DPDK_2.0;
> +
> +
> +Next remove the corresponding versioned export .. code-block:: c
> +
> + -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
> +
> +
> +Note that the internal function definition could also be removed, but
> +its used in our example by the newer version _v21, so we leave it in
> +place.  This is a coding style choice.
> +
> +Lastly, we need to bump the LIBABIVER number for this library in the
> +Makefile to indicate to applications doing dynamic linking that this is
> +a later, and possibly incompatible library version:
> +
> +.. code-block:: c
> +
> +   -LIBABIVER := 1
> +   +LIBABIVER := 2
> +
> +Deprecating an entire ABI version
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +While removing a symbol from and ABI may be useful, it is often more
> +practical to remove an entire version node at once.  If a version node
> +completely specifies an API, then removing part of it, typically makes
> +it incomplete.  In those cases it is better to remove the entire node
> +
> +To do this, start by modifying the version map file, such that all
> +symbols from the node to be removed are merged into the next node in
> +the map
> +
> +In the case of our map above, it would transform to look as follows
> +
> +.. code-block:: none
> +
> +   DPDK_2.1 {
> +        global:
> +
> +        rte_acl_add_rules;
> +        rte_acl_build;
> +        rte_acl_classify;
> +        rte_acl_classify_alg;
> +        rte_acl_classify_scalar;
> +        rte_acl_dump;
> +        rte_acl_create
> +        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: *;
> + };
> +
> +Then any uses of BIND_DEFAULT_SYMBOL that pointed to the old node
> +should be updated to point to the new version node in any header files
> +for all affected symbols.
> +
> +.. code-block:: c
> +
> + -BIND_DEFAULT_SYMBOL(rte_acl_create, _v20, 2.0);
> + +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
> +
> +Lastly, any VERSION_SYMBOL macros that point to the old version node
> +should be removed, taking care to keep, where need old code in place to
> +support newer versions of the symbol.
> +
> +Running the ABI Validator
> +-------------------------
> +
> +The ``scripts`` directory in the DPDK source tree contains a utility
> +program, ``validate-abi.sh``, for validating the DPDK ABI based on the
> +Linux `ABI Compliance Checker
> +<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_.
> +
> +This has a dependency on the ``abi-compliance-checker`` and ``and
> +abi-dumper`` utilities which can be installed via a package manager. For
> example::
> +
> +   sudo yum install abi-compliance-checker
> +   sudo yum install abi-dumper
> +
> +The syntax of the ``validate-abi.sh`` utility is::
> +
> +   ./scripts/validate-abi.sh <TAG1> <TAG2> <TARGET>
> +
> +Where ``TAG1`` and ``TAG2`` are valid git tags on the local repo and
> +target is the usual DPDK compilation target.
> +
> +For example to test the current committed HEAD against a previous
> +release tag we could add a temporary tag and run the utility as follows::
> +
> +   git tag MY_TEMP_TAG
> +   ./scripts/validate-abi.sh v2.0.0 MY_TEMP_TAG
> + x86_64-native-linuxapp-gcc
> +
> +After the validation script completes (it can take a while since it
> +need to compile both tags) it will create compatibility reports in the
> +``./compat_report`` directory. Listed incompatibilities can be found as
> +follows::
> +
> +  grep -lr Incompatible compat_reports/
> --
> 2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
  2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
                   ` (2 preceding siblings ...)
  2015-06-24 18:34 ` [dpdk-dev] [PATCHv2 " Neil Horman
@ 2015-06-25  7:37 ` Gajdzica, MaciejX T
  2015-06-25 12:28   ` Neil Horman
  2015-06-25 14:35 ` [dpdk-dev] [PATCHv3 1/3] " Neil Horman
  2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
  5 siblings, 1 reply; 41+ messages in thread
From: Gajdzica, MaciejX T @ 2015-06-25  7:37 UTC (permalink / raw)
  To: Neil Horman, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Tuesday, June 23, 2015 9:34 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> 
> Clean up some macro definition typos and comments
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: thomas.monjalon@6wind.com
> ---
>  lib/librte_compat/rte_compat.h | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
> index fb0dc19..75920a1 100644
> --- a/lib/librte_compat/rte_compat.h
> +++ b/lib/librte_compat/rte_compat.h
> @@ -54,7 +54,7 @@
>   * foo is exported as a global symbol.
>   *
>   * 2) rename the existing function int foo(char *string) to
> - *	int __vsym foo_v20(char *string)
> + *	int foo_v20(char *string)
>   *
>   * 3) Add this macro immediately below the function
>   *	VERSION_SYMBOL(foo, _v20, 2.0);
> @@ -63,7 +63,7 @@
>   *	char foo(int value, int otherval) { ...}
>   *
>   * 5) Mark the newest version as the default version
> - *	BIND_DEFAULT_SYMBOL(foo, 2.1);
> + *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
>   *
>   */
> 
> @@ -79,21 +79,21 @@
>   * 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))
> +#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> +RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
> 
>  /*
>   * BASE_SYMBOL
>   * Creates a symbol version table entry binding unversioned symbol <b>
>   * to the internal function <b>_<e>
>   */
> -#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ",
> "RTE_STR(b)"@")
> +#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "
> +RTE_STR(b)"@")
> 
>  /*
> - * BNID_DEFAULT_SYMBOL
> + * 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 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))
> 
>  #else
> @@ -103,7 +103,7 @@
>  #define VERSION_SYMBOL(b, e, v)
>  #define __vsym
>  #define BASE_SYMBOL(b, n)
> -#define BIND_DEFAULT_SYMBOL(b, v)
> +#define BIND_DEFAULT_SYMBOL(b, e, n)
> 
>  /*
>   * RTE_BUILD_SHARED_LIB=n
> --
> 2.1.0

This patch doesn't solves the issue with static build.

You have function:
int foo(int val)

And you want to create new version of it. So after edit you will have:
int foo_v20(int val)
{
[...]
}
VERSION_SYMBOL(foo, _v20, 2.0);

int foo_v21(int val1, int val2)
{
[...]
}
BIND_DEFAULT_SYMBOL (foo, _v21, 2.1);

You have also external application that uses foo function. You try to compile this app with dpdk
compiled as shared and static. In first case everything will work fine, but in second linker won't
find definition of foo because it doesn't exist. There are only definitions of foo_v20 and foo_v21.

Best Regards
Maciek

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-25  7:19     ` Zhang, Helin
@ 2015-06-25  7:42       ` Gonzalez Monroy, Sergio
  2015-06-25  8:00         ` Gonzalez Monroy, Sergio
  2015-06-25 12:25       ` Neil Horman
  1 sibling, 1 reply; 41+ messages in thread
From: Gonzalez Monroy, Sergio @ 2015-06-25  7:42 UTC (permalink / raw)
  To: Zhang, Helin, Neil Horman, dev

On 25/06/2015 08:19, Zhang, Helin wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
>> Sent: Thursday, June 25, 2015 2:35 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
>>
>> People have been asking for ways to use the ABI macros, heres some docs to
>> clarify their use.  Included is:
>>
>> * An overview of what ABI is
>> * Details of the ABI deprecation process
>> * Details of the versioning macros
>> * Examples of their use
>> * Details of how to use the ABI validator
>>
>> Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
>> working on it.  Much of the introductory material was gathered and cleaned up
>> by him
>>
>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>> CC: john.mcnamara@intel.com
>> CC: thomas.monjalon@6wind.com
>>
>> Change notes:
>>
>> v2)
>>       * Fixed RST indentations and spelling errors
>>       * Rebased to upstream to fix index.rst conflict
>> ---
>>   doc/guides/guidelines/index.rst      |   1 +
>>   doc/guides/guidelines/versioning.rst | 456
>> +++++++++++++++++++++++++++++++++++
>>   2 files changed, 457 insertions(+)
>>   create mode 100644 doc/guides/guidelines/versioning.rst
>>
>> diff --git a/doc/guides/guidelines/index.rst b/doc/guides/guidelines/index.rst
>> index 0ee9ab3..bfb9fa3 100644
>> --- a/doc/guides/guidelines/index.rst
>> +++ b/doc/guides/guidelines/index.rst
>> @@ -7,3 +7,4 @@ Guidelines
>>
>>       coding_style
>>       design
>> +    versioning
>> diff --git a/doc/guides/guidelines/versioning.rst
>> b/doc/guides/guidelines/versioning.rst
>> new file mode 100644
>> index 0000000..2aef526
>> --- /dev/null
>> +++ b/doc/guides/guidelines/versioning.rst
>> @@ -0,0 +1,456 @@
>> +Managing ABI updates
>> +====================
>> +
>> +Description
>> +-----------
>> +
>> +This document details some methods for handling ABI management in the
>> DPDK.
>> +Note this document is not exhaustive, in that C library versioning is
>> +flexible allowing multiple methods to achieve various goals, but it
>> +will provide the user with some introductory methods
>> +
>> +General Guidelines
>> +------------------
>> +
>> +#. Whenever possible, ABI should be preserved #. The addition of
>> +symbols is generally not problematic #. The modification of symbols can
>> +generally be managed with versioning #. The removal of symbols
>> +generally is an ABI break and requires bumping of the
>> +   LIBABIVER macro
>> +
>> +What is an ABI
>> +--------------
>> +
>> +An ABI (Application Binary Interface) is the set of runtime interfaces
>> +exposed by a library. It is similar to an API (Application Programming
>> +Interface) but is the result of compilation.  It is also effectively
>> +cloned when applications link to dynamic libraries.  That is to say
>> +when an application is compiled to link against dynamic libraries, it
>> +is assumed that the ABI remains constant between the time the application is
>> compiled/linked, and the time that it runs.
>> +Therefore, in the case of dynamic linking, it is critical that an ABI
>> +is preserved, or (when modified), done in such a way that the
>> +application is unable to behave improperly or in an unexpected fashion.
>> +
>> +The DPDK ABI policy
>> +-------------------
>> +
>> +ABI versions are set at the time of major release labeling, and the ABI
>> +may change multiple times, without warning, between the last release
>> +label and the HEAD label of the git tree.
>> +
>> +ABI versions, once released, are available until such time as their
>> +deprecation has been noted in the Release Notes for at least one major
>> +release cycle. For example consider the case where the ABI for DPDK 2.0
>> +has been shipped and then a decision is made to modify it during the
>> +development of DPDK 2.1. The decision will be recorded in the Release
>> +Notes for the DPDK 2.1 release and the modification will be made available in
>> the DPDK 2.2 release.
>> +
>> +ABI versions may be deprecated in whole or in part as needed by a given
>> +update.
>> +
>> +Some ABI changes may be too significant to reasonably maintain multiple
>> +versions. In those cases ABI's may be updated without backward
>> +compatibility being provided. The requirements for doing so are:
>> +
>> +#. At least 3 acknowledgments of the need to do so must be made on the
>> +   dpdk.org mailing list.
>> +
>> +#. A full deprecation cycle, as explained above, must be made to offer
>> +   downstream consumers sufficient warning of the change.
>> +
>> +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
>> +   incorporated must be incremented in parallel with the ABI changes
>> +   themselves.
>> +
>> +Note that the above process for ABI deprecation should not be
>> +undertaken lightly. ABI stability is extremely important for downstream
>> +consumers of the DPDK, especially when distributed in shared object
>> +form. Every effort should be made to preserve the ABI whenever
>> +possible. The ABI should only be changed for significant reasons, such
>> +as performance enhancements. ABI breakage due to changes such as
>> +reorganizing public structure fields for aesthetic or readability purposes should
>> be avoided.
>> +
>> +Examples of Deprecation Notices
>> +-------------------------------
>> +
>> +The following are some examples of ABI deprecation notices which would
>> +be added to the Release Notes:
>> +
>> +* The Macro ``#RTE_FOO`` is deprecated and will be removed with version
>> +2.0,
>> +  to be replaced with the inline function ``rte_foo()``.
>> +
>> +* The function ``rte_mbuf_grok()`` has been updated to include a new
>> +parameter
>> +  in version 2.0. Backwards compatibility will be maintained for this
>> +function
>> +  until the release of version 2.1
>> +
>> +* The members of ``struct rte_foo`` have been reorganized in release
>> +2.0 for
>> +  performance reasons. Existing binary applications will have backwards
>> +  compatibility in release 2.0, while newly built binaries will need to
>> +  reference the new structure variant ``struct rte_foo2``.
>> +Compatibility will
>> +  be removed in release 2.2, and all applications will require updating
>> +and
>> +  rebuilding to the new structure at that time, which will be renamed
>> +to the
>> +  original ``struct rte_foo``.
>> +
>> +* Significant ABI changes are planned for the ``librte_dostuff``
>> +library. The
>> +  upcoming release 2.0 will not contain these changes, but release 2.1
>> +will,
>> +  and no backwards compatibility is planned due to the extensive nature
>> +of
>> +  these changes. Binaries using this library built prior to version 2.1
>> +will
>> +  require updating and recompilation.
>> +
>> +Versioning Macros
>> +-----------------
>> +
>> +When a symbol is exported from a library to provide an API, it also
>> +provides a calling convention (ABI) that is embodied in its name,
>> +return type and arguments. Occasionally that function may need to
>> +change to accommodate new 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
>> +``lib/librte_compat/rte_compat.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 library so that older binaries
>> need not be immediately recompiled.
>> +
>> +The macros exported are:
>> +
>> +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry
>> +binding
>> +  unversioned symbol ``b`` to the internal function ``b_e``.
>> +
>> +
>> +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
>> +  unversioned symbol ``b`` to the internal function ``b_e``.
>> +
>> +* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry
>> +instructing
>> +  the linker to bind references to symbol ``b`` to the internal symbol
>> +  ``b_e``.
>> +
>> +
>> +Examples of ABI Macro use
>> +-------------------------
>> +
>> +Updating a public API
>> +~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Assume we have a function as follows
>> +
>> +.. code-block:: c
>> +
>> + /*
>> +  * Create an acl context object for apps to
>> +  * manipulate
>> +  */
>> + struct rte_acl_ctx *
>> + rte_acl_create(const struct rte_acl_param *param) {
>> +        ...
>> + }
>> +
>> +
>> +Assume that struct rte_acl_ctx is a private structure, and that a
>> +developer wishes to enhance the acl api so that a debugging flag can be
>> +enabled on a per-context basis.  This requires an addition to the
>> +structure (which, being private, is safe), but it also requires
>> +modifying the code as follows
>> +
>> +.. code-block:: c
>> +
>> + /*
>> +  * Create an acl context object for apps to
>> +  * manipulate
>> +  */
>> + struct rte_acl_ctx *
>> + rte_acl_create(const struct rte_acl_param *param, int debug) {
>> +        ...
>> + }
>> +
>> +
>> +Note also that, being a public function, the header file prototype must
>> +also be changed, as must all the call sites, to reflect the new ABI
>> +footprint.  We will maintain previous ABI versions that are accessible
>> +only to previously compiled binaries
>> +
>> +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
>> +
>> +.. code-block:: none
>> +
>> +   DPDK_2.0 {
>> +        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_2.0 {
>> +        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_2.1 {
>> +        global:
>> +        rte_acl_create;
> One question, does it need a line of "local: *;", like it did in
> librte_ether/rte_ether_version.map?
No, it does not. You only need to specify 'local' in the default/base 
node, which
in this case/example is DPDK_2.0.

Sergio
>> +
>> +   } DPDK_2.0;
>> +
>>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-25  7:42       ` Gonzalez Monroy, Sergio
@ 2015-06-25  8:00         ` Gonzalez Monroy, Sergio
  0 siblings, 0 replies; 41+ messages in thread
From: Gonzalez Monroy, Sergio @ 2015-06-25  8:00 UTC (permalink / raw)
  To: Zhang, Helin, Neil Horman, dev

On 25/06/2015 08:42, Gonzalez Monroy, Sergio wrote:
> On 25/06/2015 08:19, Zhang, Helin wrote:
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
>>> Sent: Thursday, June 25, 2015 2:35 AM
>>> To: dev@dpdk.org
>>> Subject: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
>>>
>>> People have been asking for ways to use the ABI macros, heres some 
>>> docs to
>>> clarify their use.  Included is:
>>>
>>> * An overview of what ABI is
>>> * Details of the ABI deprecation process
>>> * Details of the versioning macros
>>> * Examples of their use
>>> * Details of how to use the ABI validator
>>>
>>> Thanks to John Mcnamara, who duplicated much of this effort at Intel 
>>> while I was
>>> working on it.  Much of the introductory material was gathered and 
>>> cleaned up
>>> by him
>>>
>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>> CC: john.mcnamara@intel.com
>>> CC: thomas.monjalon@6wind.com
>>>
>>> Change notes:
>>>
>>> v2)
>>>       * Fixed RST indentations and spelling errors
>>>       * Rebased to upstream to fix index.rst conflict
>>> ---
>>>   doc/guides/guidelines/index.rst      |   1 +
>>>   doc/guides/guidelines/versioning.rst | 456
>>> +++++++++++++++++++++++++++++++++++
>>>   2 files changed, 457 insertions(+)
>>>   create mode 100644 doc/guides/guidelines/versioning.rst
>>>
>>> diff --git a/doc/guides/guidelines/index.rst 
>>> b/doc/guides/guidelines/index.rst
>>> index 0ee9ab3..bfb9fa3 100644
>>> --- a/doc/guides/guidelines/index.rst
>>> +++ b/doc/guides/guidelines/index.rst
>>> @@ -7,3 +7,4 @@ Guidelines
>>>
>>>       coding_style
>>>       design
>>> +    versioning
>>> diff --git a/doc/guides/guidelines/versioning.rst
>>> b/doc/guides/guidelines/versioning.rst
>>> new file mode 100644
>>> index 0000000..2aef526
>>> --- /dev/null
>>> +++ b/doc/guides/guidelines/versioning.rst
>>> @@ -0,0 +1,456 @@
>>> +Managing ABI updates
>>> +====================
>>> +
>>> +Description
>>> +-----------
>>> +
>>> +This document details some methods for handling ABI management in the
>>> DPDK.
>>> +Note this document is not exhaustive, in that C library versioning is
>>> +flexible allowing multiple methods to achieve various goals, but it
>>> +will provide the user with some introductory methods
>>> +
>>> +General Guidelines
>>> +------------------
>>> +
>>> +#. Whenever possible, ABI should be preserved #. The addition of
>>> +symbols is generally not problematic #. The modification of symbols 
>>> can
>>> +generally be managed with versioning #. The removal of symbols
>>> +generally is an ABI break and requires bumping of the
>>> +   LIBABIVER macro
>>> +
>>> +What is an ABI
>>> +--------------
>>> +
>>> +An ABI (Application Binary Interface) is the set of runtime interfaces
>>> +exposed by a library. It is similar to an API (Application Programming
>>> +Interface) but is the result of compilation.  It is also effectively
>>> +cloned when applications link to dynamic libraries.  That is to say
>>> +when an application is compiled to link against dynamic libraries, it
>>> +is assumed that the ABI remains constant between the time the 
>>> application is
>>> compiled/linked, and the time that it runs.
>>> +Therefore, in the case of dynamic linking, it is critical that an ABI
>>> +is preserved, or (when modified), done in such a way that the
>>> +application is unable to behave improperly or in an unexpected 
>>> fashion.
>>> +
>>> +The DPDK ABI policy
>>> +-------------------
>>> +
>>> +ABI versions are set at the time of major release labeling, and the 
>>> ABI
>>> +may change multiple times, without warning, between the last release
>>> +label and the HEAD label of the git tree.
>>> +
>>> +ABI versions, once released, are available until such time as their
>>> +deprecation has been noted in the Release Notes for at least one major
>>> +release cycle. For example consider the case where the ABI for DPDK 
>>> 2.0
>>> +has been shipped and then a decision is made to modify it during the
>>> +development of DPDK 2.1. The decision will be recorded in the Release
>>> +Notes for the DPDK 2.1 release and the modification will be made 
>>> available in
>>> the DPDK 2.2 release.
>>> +
>>> +ABI versions may be deprecated in whole or in part as needed by a 
>>> given
>>> +update.
>>> +
>>> +Some ABI changes may be too significant to reasonably maintain 
>>> multiple
>>> +versions. In those cases ABI's may be updated without backward
>>> +compatibility being provided. The requirements for doing so are:
>>> +
>>> +#. At least 3 acknowledgments of the need to do so must be made on the
>>> +   dpdk.org mailing list.
>>> +
>>> +#. A full deprecation cycle, as explained above, must be made to offer
>>> +   downstream consumers sufficient warning of the change.
>>> +
>>> +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI 
>>> changes are
>>> +   incorporated must be incremented in parallel with the ABI changes
>>> +   themselves.
>>> +
>>> +Note that the above process for ABI deprecation should not be
>>> +undertaken lightly. ABI stability is extremely important for 
>>> downstream
>>> +consumers of the DPDK, especially when distributed in shared object
>>> +form. Every effort should be made to preserve the ABI whenever
>>> +possible. The ABI should only be changed for significant reasons, such
>>> +as performance enhancements. ABI breakage due to changes such as
>>> +reorganizing public structure fields for aesthetic or readability 
>>> purposes should
>>> be avoided.
>>> +
>>> +Examples of Deprecation Notices
>>> +-------------------------------
>>> +
>>> +The following are some examples of ABI deprecation notices which would
>>> +be added to the Release Notes:
>>> +
>>> +* The Macro ``#RTE_FOO`` is deprecated and will be removed with 
>>> version
>>> +2.0,
>>> +  to be replaced with the inline function ``rte_foo()``.
>>> +
>>> +* The function ``rte_mbuf_grok()`` has been updated to include a new
>>> +parameter
>>> +  in version 2.0. Backwards compatibility will be maintained for this
>>> +function
>>> +  until the release of version 2.1
>>> +
>>> +* The members of ``struct rte_foo`` have been reorganized in release
>>> +2.0 for
>>> +  performance reasons. Existing binary applications will have 
>>> backwards
>>> +  compatibility in release 2.0, while newly built binaries will 
>>> need to
>>> +  reference the new structure variant ``struct rte_foo2``.
>>> +Compatibility will
>>> +  be removed in release 2.2, and all applications will require 
>>> updating
>>> +and
>>> +  rebuilding to the new structure at that time, which will be renamed
>>> +to the
>>> +  original ``struct rte_foo``.
>>> +
>>> +* Significant ABI changes are planned for the ``librte_dostuff``
>>> +library. The
>>> +  upcoming release 2.0 will not contain these changes, but release 2.1
>>> +will,
>>> +  and no backwards compatibility is planned due to the extensive 
>>> nature
>>> +of
>>> +  these changes. Binaries using this library built prior to version 
>>> 2.1
>>> +will
>>> +  require updating and recompilation.
>>> +
>>> +Versioning Macros
>>> +-----------------
>>> +
>>> +When a symbol is exported from a library to provide an API, it also
>>> +provides a calling convention (ABI) that is embodied in its name,
>>> +return type and arguments. Occasionally that function may need to
>>> +change to accommodate new 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
>>> +``lib/librte_compat/rte_compat.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 library so that 
>>> older binaries
>>> need not be immediately recompiled.
>>> +
>>> +The macros exported are:
>>> +
>>> +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry
>>> +binding
>>> +  unversioned symbol ``b`` to the internal function ``b_e``.
>>> +
>>> +
>>> +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
>>> +  unversioned symbol ``b`` to the internal function ``b_e``.
>>> +
>>> +* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry
>>> +instructing
>>> +  the linker to bind references to symbol ``b`` to the internal symbol
>>> +  ``b_e``.
>>> +
>>> +
>>> +Examples of ABI Macro use
>>> +-------------------------
>>> +
>>> +Updating a public API
>>> +~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +Assume we have a function as follows
>>> +
>>> +.. code-block:: c
>>> +
>>> + /*
>>> +  * Create an acl context object for apps to
>>> +  * manipulate
>>> +  */
>>> + struct rte_acl_ctx *
>>> + rte_acl_create(const struct rte_acl_param *param) {
>>> +        ...
>>> + }
>>> +
>>> +
>>> +Assume that struct rte_acl_ctx is a private structure, and that a
>>> +developer wishes to enhance the acl api so that a debugging flag 
>>> can be
>>> +enabled on a per-context basis.  This requires an addition to the
>>> +structure (which, being private, is safe), but it also requires
>>> +modifying the code as follows
>>> +
>>> +.. code-block:: c
>>> +
>>> + /*
>>> +  * Create an acl context object for apps to
>>> +  * manipulate
>>> +  */
>>> + struct rte_acl_ctx *
>>> + rte_acl_create(const struct rte_acl_param *param, int debug) {
>>> +        ...
>>> + }
>>> +
>>> +
>>> +Note also that, being a public function, the header file prototype 
>>> must
>>> +also be changed, as must all the call sites, to reflect the new ABI
>>> +footprint.  We will maintain previous ABI versions that are accessible
>>> +only to previously compiled binaries
>>> +
>>> +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
>>> +
>>> +.. code-block:: none
>>> +
>>> +   DPDK_2.0 {
>>> +        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_2.0 {
>>> +        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_2.1 {
>>> +        global:
>>> +        rte_acl_create;
>> One question, does it need a line of "local: *;", like it did in
>> librte_ether/rte_ether_version.map?
> No, it does not. You only need to specify 'local' in the default/base 
> node, which
> in this case/example is DPDK_2.0.
>
> Sergio
Just to be clear, as I think I may have misused the term 'default' here, 
it is recommended
to just specify 'local: *;' in just one node (it could confuse the 
linker) and it doesn't really
matter which one.

Quoting http://www.akkadia.org/drepper/symbol-versioning:
"It makes no sense at all to associate versions with symbols which are 
not exported. Therefore the `local:' sections of all but the base 
version are empty and the `local:' section of the base version simply 
contains `*'. This will match all symbols which are not explicitly 
mentioned in any `global:' list."

Sergio

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-24 21:09     ` Thomas Monjalon
@ 2015-06-25 11:35       ` Neil Horman
  2015-06-25 13:22         ` Thomas Monjalon
  0 siblings, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-25 11:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Wed, Jun 24, 2015 at 11:09:29PM +0200, Thomas Monjalon wrote:
> 2015-06-24 14:34, Neil Horman:
> > +Some ABI changes may be too significant to reasonably maintain multiple
> > +versions. In those cases ABI's may be updated without backward compatibility
> > +being provided. The requirements for doing so are:
> > +
> > +#. At least 3 acknowledgments of the need to do so must be made on the
> > +   dpdk.org mailing list.
> > +
> > +#. A full deprecation cycle, as explained above, must be made to offer
> > +   downstream consumers sufficient warning of the change.
> > +
> > +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
> > +   incorporated must be incremented in parallel with the ABI changes
> > +   themselves.
> 
> The proposal was to provide the old and the new ABI in the same source code
> during the deprecation cycle. The old ABI would be the default and people
> can build the new one by enabling the NEXT_ABI config option.
> So the migration to the new ABI is smoother.
> 
Yes....I'm not sure what you're saying here.  The ABI doesn't 'Change' until the
old ABI is removed (i.e. old applications are forced to adopt a new ABI), and so
LIBABIVER has to be updated in parallel with that removal

> 
> [...]
> > +The macros exported are:
> > +
> > +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> > +  unversioned symbol ``b`` to the internal function ``b_e``.
> 
> The definition is the same as BASE_SYMBOL.
> 
No, they're different.  VERSION_SYMBOL is defined as:
VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))

while BASE_SYMBOL is
#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")

> > +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> > +  unversioned symbol ``b`` to the internal function ``b_e``.
> 
> 
> [...]
> > +   DPDK_2.0 {
> > +        global:
> > +
> > +        rte_acl_add_rules;
> > +        rte_acl_build;
> > +        rte_acl_classify;
> > +        rte_acl_classify_alg;
> > +        rte_acl_classify_scalar;
> > +        rte_acl_create;
> 
> So it's declared twice, right?
> I think it should be explicit.
> 
Yes, its listed once for each version node, so 2 delcarations.  I thought that
was made explicit by the use of the code block.  What else would you like to
see?

> > +        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_2.1 {
> > +        global:
> > +        rte_acl_create;
> > +
> > +   } DPDK_2.0;
> 
> [...]
> > +Note that the base name of the symbol was kept in tact, as this is condusive to
> 
> s/in tact/intact/?
> 
Hmm, thats odd, aspell explicitly changed that.  Though your right, it should be
intact.  I'll fix it.

> [...]
> > +the macros used for versioning symbols.  That is our next step, mapping this new
> > +symbol name to the initial symbol name at version node 2.0.  Immediately after
> > +the function, we add this line of code
> > +
> > +.. code-block:: c
> > +
> > +   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
> 
> Can it be declared before the function?
> 
Strictly speaking yes, though its a bit odd from a sylistic point to declare
versioned aliases for a symbol prior to defining the symbol itself (its like a
forward declaration)

> [...]
> > +Remembering to also add the rte_compat.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
> > +the original rte_acl_create symbol to the original function (but with a new
> > +name)
> 
> Could we use VERSION_SYMBOL(rte_acl_create, , 2.0);
> when introducing the function in DPDK 2.0 (before any ABI breakage)?
> It could help to generate the .map file.
> 
I've honestly not tried.  I think its possible, but the example you give above I
don't think will work, because it will result in an error indicating
rte_acl_create is declared twice.  You would have to rename rte_acl_create to
something uniqe prior to versioning it.

> When do we need to use BASE_SYMBOL?
> 
For our purposes you currently don't, because there are no unversioned symbols
in DPDK (since we use a map file).  I've just included it here for completeness
in the header file should it ever be needed in the future.

> [...]
> > +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 symbol
> > +``rte_acl_create@DPDK_2.1``.  To do this, we modify the public prototype of the call
> > +in the header file, adding the macro there to inform all including applications,
> > +that on re-link, the default rte_acl_create symbol should point to this
> > +function.  Note that we could do this by simply naming the function above
> > +rte_acl_create, and the linker would chose the most recent version tag to apply
> > +in the version script, but we can also do this in the header file
> > +
> > +.. code-block:: c
> > +
> > +   struct rte_acl_ctx *
> > +   -rte_acl_create(const struct rte_acl_param *param);
> > +   +rte_acl_create(const struct rte_acl_param *param, int debug);
> > +   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
> 
> Will it work with static library?
> 
hmm, this example in particular?  No, I didn't think of that.  To work with a
static build, you still need to define the unversioned symbol.  Thats easy
enough to do though, by either defining rte_acl_create as a public api and
calling the appropriate versioned function, or by creating a macro to point to
the right version via an alias.  I can fix that easily enough.

> > +Next remove the corresponding versioned export
> > +.. code-block:: c
> > +
> > + -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
> > +
> > +
> > +Note that the internal function definition could also be removed, but its used
> > +in our example by the newer version _v21, so we leave it in place.  This is a
> > +coding style choice.
> > +
> > +Lastly, we need to bump the LIBABIVER number for this library in the Makefile to
> > +indicate to applications doing dynamic linking that this is a later, and
> > +possibly incompatible library version:
> > +
> > +.. code-block:: c
> > +
> > +   -LIBABIVER := 1
> > +   +LIBABIVER := 2
> 
> Very well explained, thanks.
> 
> [...]
> > +        rte_acl_add_rules;
> > +        rte_acl_build;
> > +        rte_acl_classify;
> > +        rte_acl_classify_alg;
> > +        rte_acl_classify_scalar;
> > +        rte_acl_dump;
> > +        rte_acl_create
> 
> Not in alphabetical order.
> 
No, none of them are, but that can be adjusted, though I'd like to do that
separately from this documentation.

> 
> As you copy a part of abi.rst, it should be removed from the original doc.
> 
Sure
> Thanks Neil
> 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-25  7:19     ` Zhang, Helin
  2015-06-25  7:42       ` Gonzalez Monroy, Sergio
@ 2015-06-25 12:25       ` Neil Horman
  2015-06-29 16:35         ` [dpdk-dev] [PATCH] lib: remove redundant definition of local symbols Thomas Monjalon
  1 sibling, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-25 12:25 UTC (permalink / raw)
  To: Zhang, Helin; +Cc: dev

On Thu, Jun 25, 2015 at 07:19:49AM +0000, Zhang, Helin wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Thursday, June 25, 2015 2:35 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
> > 
> > People have been asking for ways to use the ABI macros, heres some docs to
> > clarify their use.  Included is:
> > 
> > * An overview of what ABI is
> > * Details of the ABI deprecation process
> > * Details of the versioning macros
> > * Examples of their use
> > * Details of how to use the ABI validator
> > 
> > Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
> > working on it.  Much of the introductory material was gathered and cleaned up
> > by him
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: john.mcnamara@intel.com
> > CC: thomas.monjalon@6wind.com
> > 
> > Change notes:
> > 
> > v2)
> >      * Fixed RST indentations and spelling errors
> >      * Rebased to upstream to fix index.rst conflict
> > ---
> >  doc/guides/guidelines/index.rst      |   1 +
> >  doc/guides/guidelines/versioning.rst | 456
> > +++++++++++++++++++++++++++++++++++
> >  2 files changed, 457 insertions(+)
> >  create mode 100644 doc/guides/guidelines/versioning.rst
> > 
> > diff --git a/doc/guides/guidelines/index.rst b/doc/guides/guidelines/index.rst
> > index 0ee9ab3..bfb9fa3 100644
> > --- a/doc/guides/guidelines/index.rst
> > +++ b/doc/guides/guidelines/index.rst
> > @@ -7,3 +7,4 @@ Guidelines
> > 
> >      coding_style
> >      design
> > +    versioning
> > diff --git a/doc/guides/guidelines/versioning.rst
> > b/doc/guides/guidelines/versioning.rst
> > new file mode 100644
> > index 0000000..2aef526
> > --- /dev/null
> > +++ b/doc/guides/guidelines/versioning.rst
> > @@ -0,0 +1,456 @@
> > +Managing ABI updates
> > +====================
> > +
> > +Description
> > +-----------
> > +
> > +This document details some methods for handling ABI management in the
> > DPDK.
> > +Note this document is not exhaustive, in that C library versioning is
> > +flexible allowing multiple methods to achieve various goals, but it
> > +will provide the user with some introductory methods
> > +
> > +General Guidelines
> > +------------------
> > +
> > +#. Whenever possible, ABI should be preserved #. The addition of
> > +symbols is generally not problematic #. The modification of symbols can
> > +generally be managed with versioning #. The removal of symbols
> > +generally is an ABI break and requires bumping of the
> > +   LIBABIVER macro
> > +
> > +What is an ABI
> > +--------------
> > +
> > +An ABI (Application Binary Interface) is the set of runtime interfaces
> > +exposed by a library. It is similar to an API (Application Programming
> > +Interface) but is the result of compilation.  It is also effectively
> > +cloned when applications link to dynamic libraries.  That is to say
> > +when an application is compiled to link against dynamic libraries, it
> > +is assumed that the ABI remains constant between the time the application is
> > compiled/linked, and the time that it runs.
> > +Therefore, in the case of dynamic linking, it is critical that an ABI
> > +is preserved, or (when modified), done in such a way that the
> > +application is unable to behave improperly or in an unexpected fashion.
> > +
> > +The DPDK ABI policy
> > +-------------------
> > +
> > +ABI versions are set at the time of major release labeling, and the ABI
> > +may change multiple times, without warning, between the last release
> > +label and the HEAD label of the git tree.
> > +
> > +ABI versions, once released, are available until such time as their
> > +deprecation has been noted in the Release Notes for at least one major
> > +release cycle. For example consider the case where the ABI for DPDK 2.0
> > +has been shipped and then a decision is made to modify it during the
> > +development of DPDK 2.1. The decision will be recorded in the Release
> > +Notes for the DPDK 2.1 release and the modification will be made available in
> > the DPDK 2.2 release.
> > +
> > +ABI versions may be deprecated in whole or in part as needed by a given
> > +update.
> > +
> > +Some ABI changes may be too significant to reasonably maintain multiple
> > +versions. In those cases ABI's may be updated without backward
> > +compatibility being provided. The requirements for doing so are:
> > +
> > +#. At least 3 acknowledgments of the need to do so must be made on the
> > +   dpdk.org mailing list.
> > +
> > +#. A full deprecation cycle, as explained above, must be made to offer
> > +   downstream consumers sufficient warning of the change.
> > +
> > +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
> > +   incorporated must be incremented in parallel with the ABI changes
> > +   themselves.
> > +
> > +Note that the above process for ABI deprecation should not be
> > +undertaken lightly. ABI stability is extremely important for downstream
> > +consumers of the DPDK, especially when distributed in shared object
> > +form. Every effort should be made to preserve the ABI whenever
> > +possible. The ABI should only be changed for significant reasons, such
> > +as performance enhancements. ABI breakage due to changes such as
> > +reorganizing public structure fields for aesthetic or readability purposes should
> > be avoided.
> > +
> > +Examples of Deprecation Notices
> > +-------------------------------
> > +
> > +The following are some examples of ABI deprecation notices which would
> > +be added to the Release Notes:
> > +
> > +* The Macro ``#RTE_FOO`` is deprecated and will be removed with version
> > +2.0,
> > +  to be replaced with the inline function ``rte_foo()``.
> > +
> > +* The function ``rte_mbuf_grok()`` has been updated to include a new
> > +parameter
> > +  in version 2.0. Backwards compatibility will be maintained for this
> > +function
> > +  until the release of version 2.1
> > +
> > +* The members of ``struct rte_foo`` have been reorganized in release
> > +2.0 for
> > +  performance reasons. Existing binary applications will have backwards
> > +  compatibility in release 2.0, while newly built binaries will need to
> > +  reference the new structure variant ``struct rte_foo2``.
> > +Compatibility will
> > +  be removed in release 2.2, and all applications will require updating
> > +and
> > +  rebuilding to the new structure at that time, which will be renamed
> > +to the
> > +  original ``struct rte_foo``.
> > +
> > +* Significant ABI changes are planned for the ``librte_dostuff``
> > +library. The
> > +  upcoming release 2.0 will not contain these changes, but release 2.1
> > +will,
> > +  and no backwards compatibility is planned due to the extensive nature
> > +of
> > +  these changes. Binaries using this library built prior to version 2.1
> > +will
> > +  require updating and recompilation.
> > +
> > +Versioning Macros
> > +-----------------
> > +
> > +When a symbol is exported from a library to provide an API, it also
> > +provides a calling convention (ABI) that is embodied in its name,
> > +return type and arguments. Occasionally that function may need to
> > +change to accommodate new 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
> > +``lib/librte_compat/rte_compat.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 library so that older binaries
> > need not be immediately recompiled.
> > +
> > +The macros exported are:
> > +
> > +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry
> > +binding
> > +  unversioned symbol ``b`` to the internal function ``b_e``.
> > +
> > +
> > +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> > +  unversioned symbol ``b`` to the internal function ``b_e``.
> > +
> > +* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry
> > +instructing
> > +  the linker to bind references to symbol ``b`` to the internal symbol
> > +  ``b_e``.
> > +
> > +
> > +Examples of ABI Macro use
> > +-------------------------
> > +
> > +Updating a public API
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Assume we have a function as follows
> > +
> > +.. code-block:: c
> > +
> > + /*
> > +  * Create an acl context object for apps to
> > +  * manipulate
> > +  */
> > + struct rte_acl_ctx *
> > + rte_acl_create(const struct rte_acl_param *param) {
> > +        ...
> > + }
> > +
> > +
> > +Assume that struct rte_acl_ctx is a private structure, and that a
> > +developer wishes to enhance the acl api so that a debugging flag can be
> > +enabled on a per-context basis.  This requires an addition to the
> > +structure (which, being private, is safe), but it also requires
> > +modifying the code as follows
> > +
> > +.. code-block:: c
> > +
> > + /*
> > +  * Create an acl context object for apps to
> > +  * manipulate
> > +  */
> > + struct rte_acl_ctx *
> > + rte_acl_create(const struct rte_acl_param *param, int debug) {
> > +        ...
> > + }
> > +
> > +
> > +Note also that, being a public function, the header file prototype must
> > +also be changed, as must all the call sites, to reflect the new ABI
> > +footprint.  We will maintain previous ABI versions that are accessible
> > +only to previously compiled binaries
> > +
> > +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
> > +
> > +.. code-block:: none
> > +
> > +   DPDK_2.0 {
> > +        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_2.0 {
> > +        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_2.1 {
> > +        global:
> > +        rte_acl_create;
> One question, does it need a line of "local: *;", like it did in
> librte_ether/rte_ether_version.map?
> 
It shouldn't.  local just specifies that any symbol not already declared global
be unpublished in the ELF file (i.e. not global).  You can declare it again in
the next version node, but since the 2.1 node inherits from the 2.0 node, its
implied.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
  2015-06-25  7:37 ` [dpdk-dev] [PATCH " Gajdzica, MaciejX T
@ 2015-06-25 12:28   ` Neil Horman
  0 siblings, 0 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-25 12:28 UTC (permalink / raw)
  To: Gajdzica, MaciejX T; +Cc: dev

On Thu, Jun 25, 2015 at 07:37:43AM +0000, Gajdzica, MaciejX T wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Tuesday, June 23, 2015 9:34 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> > 
> > Clean up some macro definition typos and comments
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: thomas.monjalon@6wind.com
> > ---
> >  lib/librte_compat/rte_compat.h | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
> > index fb0dc19..75920a1 100644
> > --- a/lib/librte_compat/rte_compat.h
> > +++ b/lib/librte_compat/rte_compat.h
> > @@ -54,7 +54,7 @@
> >   * foo is exported as a global symbol.
> >   *
> >   * 2) rename the existing function int foo(char *string) to
> > - *	int __vsym foo_v20(char *string)
> > + *	int foo_v20(char *string)
> >   *
> >   * 3) Add this macro immediately below the function
> >   *	VERSION_SYMBOL(foo, _v20, 2.0);
> > @@ -63,7 +63,7 @@
> >   *	char foo(int value, int otherval) { ...}
> >   *
> >   * 5) Mark the newest version as the default version
> > - *	BIND_DEFAULT_SYMBOL(foo, 2.1);
> > + *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
> >   *
> >   */
> > 
> > @@ -79,21 +79,21 @@
> >   * 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))
> > +#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> > +RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
> > 
> >  /*
> >   * BASE_SYMBOL
> >   * Creates a symbol version table entry binding unversioned symbol <b>
> >   * to the internal function <b>_<e>
> >   */
> > -#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ",
> > "RTE_STR(b)"@")
> > +#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "
> > +RTE_STR(b)"@")
> > 
> >  /*
> > - * BNID_DEFAULT_SYMBOL
> > + * 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 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))
> > 
> >  #else
> > @@ -103,7 +103,7 @@
> >  #define VERSION_SYMBOL(b, e, v)
> >  #define __vsym
> >  #define BASE_SYMBOL(b, n)
> > -#define BIND_DEFAULT_SYMBOL(b, v)
> > +#define BIND_DEFAULT_SYMBOL(b, e, n)
> > 
> >  /*
> >   * RTE_BUILD_SHARED_LIB=n
> > --
> > 2.1.0
> 
> This patch doesn't solves the issue with static build.
> 
> You have function:
> int foo(int val)
> 
> And you want to create new version of it. So after edit you will have:
> int foo_v20(int val)
> {
> [...]
> }
> VERSION_SYMBOL(foo, _v20, 2.0);
> 
> int foo_v21(int val1, int val2)
> {
> [...]
> }
> BIND_DEFAULT_SYMBOL (foo, _v21, 2.1);
> 
> You have also external application that uses foo function. You try to compile this app with dpdk
> compiled as shared and static. In first case everything will work fine, but in second linker won't
> find definition of foo because it doesn't exist. There are only definitions of foo_v20 and foo_v21.
> 
> Best Regards
> Maciek
> 
As I noted before, you can avoid that by explicitly making the latest version of
the function the static version (that is to say, only rename older versions and
allow the 'latest' to be called rte_acl_create (in this case).  You're right
though, I prefer to rename all functions, and in my example above I didn't
address the static build issue.  I'm adding a macro for that in my repost.

Neil

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
  2015-06-25 11:35       ` Neil Horman
@ 2015-06-25 13:22         ` Thomas Monjalon
  0 siblings, 0 replies; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-25 13:22 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-25 07:35, Neil Horman:
> On Wed, Jun 24, 2015 at 11:09:29PM +0200, Thomas Monjalon wrote:
> > 2015-06-24 14:34, Neil Horman:
> > > +Some ABI changes may be too significant to reasonably maintain multiple
> > > +versions. In those cases ABI's may be updated without backward compatibility
> > > +being provided. The requirements for doing so are:
> > > +
> > > +#. At least 3 acknowledgments of the need to do so must be made on the
> > > +   dpdk.org mailing list.
> > > +
> > > +#. A full deprecation cycle, as explained above, must be made to offer
> > > +   downstream consumers sufficient warning of the change.
> > > +
> > > +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
> > > +   incorporated must be incremented in parallel with the ABI changes
> > > +   themselves.
> > 
> > The proposal was to provide the old and the new ABI in the same source code
> > during the deprecation cycle. The old ABI would be the default and people
> > can build the new one by enabling the NEXT_ABI config option.
> > So the migration to the new ABI is smoother.
> 
> Yes....I'm not sure what you're saying here.  The ABI doesn't 'Change' until the
> old ABI is removed (i.e. old applications are forced to adopt a new ABI), and so
> LIBABIVER has to be updated in parallel with that removal

I'm referring to previous threads suggesting a NEXT_ABI build option to be able
to build the old (default) ABI or the next one.
So the LIBABIVER and .map file would depend of enabling NEXT_ABI or not:
	http://dpdk.org/ml/archives/dev/2015-June/019147.html
	http://dpdk.org/ml/archives/dev/2015-June/019784.html
	http://dpdk.org/ml/archives/dev/2015-June/019810.html

> > [...]
> > > +The macros exported are:
> > > +
> > > +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> > > +  unversioned symbol ``b`` to the internal function ``b_e``.
> > 
> > The definition is the same as BASE_SYMBOL.
> > 
> No, they're different.  VERSION_SYMBOL is defined as:
> VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
> 
> while BASE_SYMBOL is
> #define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")

Yes. I mean the comments are the same, so don't reflect the difference.

> > [...]
> > > +   DPDK_2.0 {
> > > +        global:
> > > +
> > > +        rte_acl_add_rules;
> > > +        rte_acl_build;
> > > +        rte_acl_classify;
> > > +        rte_acl_classify_alg;
> > > +        rte_acl_classify_scalar;
> > > +        rte_acl_create;
> > 
> > So it's declared twice, right?
> > I think it should be explicit.
> > 
> Yes, its listed once for each version node, so 2 delcarations.  I thought that
> was made explicit by the use of the code block.  What else would you like to
> see?

I think you should say it explicitly in the comment below the block.

> > > +        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_2.1 {
> > > +        global:
> > > +        rte_acl_create;
> > > +
> > > +   } DPDK_2.0;

> > [...]
> > > +the macros used for versioning symbols.  That is our next step, mapping this new
> > > +symbol name to the initial symbol name at version node 2.0.  Immediately after
> > > +the function, we add this line of code
> > > +
> > > +.. code-block:: c
> > > +
> > > +   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
> > 
> > Can it be declared before the function?
> > 
> Strictly speaking yes, though its a bit odd from a sylistic point to declare
> versioned aliases for a symbol prior to defining the symbol itself (its like a
> forward declaration)

It allows to declare it near the function header.

> > When do we need to use BASE_SYMBOL?
> > 
> For our purposes you currently don't, because there are no unversioned symbols
> in DPDK (since we use a map file).  I've just included it here for completeness
> in the header file should it ever be needed in the future.

If it can be useful, please integrate a note to explain when it should be used.

> > [...]
> > > +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 symbol
> > > +``rte_acl_create@DPDK_2.1``.  To do this, we modify the public prototype of the call
> > > +in the header file, adding the macro there to inform all including applications,
> > > +that on re-link, the default rte_acl_create symbol should point to this
> > > +function.  Note that we could do this by simply naming the function above
> > > +rte_acl_create, and the linker would chose the most recent version tag to apply
> > > +in the version script, but we can also do this in the header file
> > > +
> > > +.. code-block:: c
> > > +
> > > +   struct rte_acl_ctx *
> > > +   -rte_acl_create(const struct rte_acl_param *param);
> > > +   +rte_acl_create(const struct rte_acl_param *param, int debug);
> > > +   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
> > 
> > Will it work with static library?
> > 
> hmm, this example in particular?  No, I didn't think of that.  To work with a
> static build, you still need to define the unversioned symbol.  Thats easy
> enough to do though, by either defining rte_acl_create as a public api and
> calling the appropriate versioned function, or by creating a macro to point to
> the right version via an alias.  I can fix that easily enough.

Yes please, static libraries are really important in DPDK.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv3 1/3] rte_compat.h : Clean up some typos
  2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
                   ` (3 preceding siblings ...)
  2015-06-25  7:37 ` [dpdk-dev] [PATCH " Gajdzica, MaciejX T
@ 2015-06-25 14:35 ` Neil Horman
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
                     ` (2 more replies)
  2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
  5 siblings, 3 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-25 14:35 UTC (permalink / raw)
  To: dev

Clean up some macro definition typos and comments

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas.monjalon@6wind.com
---
 lib/librte_compat/rte_compat.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index fb0dc19..75920a1 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -54,7 +54,7 @@
  * foo is exported as a global symbol.
  *
  * 2) rename the existing function int foo(char *string) to
- *	int __vsym foo_v20(char *string)
+ *	int foo_v20(char *string)
  *
  * 3) Add this macro immediately below the function
  *	VERSION_SYMBOL(foo, _v20, 2.0);
@@ -63,7 +63,7 @@
  *	char foo(int value, int otherval) { ...}
  *
  * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, 2.1);
+ *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
  *
  */
 
@@ -79,21 +79,21 @@
  * 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))
+#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
 
 /*
  * BASE_SYMBOL
  * Creates a symbol version table entry binding unversioned symbol <b>
  * to the internal function <b>_<e>
  */
-#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@")
+#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")
 
 /*
- * BNID_DEFAULT_SYMBOL
+ * 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 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))
 
 #else
@@ -103,7 +103,7 @@
 #define VERSION_SYMBOL(b, e, v)
 #define __vsym
 #define BASE_SYMBOL(b, n)
-#define BIND_DEFAULT_SYMBOL(b, v)
+#define BIND_DEFAULT_SYMBOL(b, e, n)
 
 /*
  * RTE_BUILD_SHARED_LIB=n
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro
  2015-06-25 14:35 ` [dpdk-dev] [PATCHv3 1/3] " Neil Horman
@ 2015-06-25 14:35   ` Neil Horman
  2015-06-26 10:13     ` Gajdzica, MaciejX T
  2015-06-26 12:52     ` Thomas Monjalon
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation Neil Horman
  2015-06-26 12:45   ` [dpdk-dev] [PATCHv3 1/3] rte_compat.h : Clean up some typos Thomas Monjalon
  2 siblings, 2 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-25 14:35 UTC (permalink / raw)
  To: dev

It was pointed out in my examples that doing shared library symbol versioning by
partitioning symbols to version specific functions (as opposed to leaving the
latest symol version at the base symbol name), neglects to take into account
static builds.  Add a macro to handle that.  If you choose a versioning approach
that uniquely names every version of the symbol, then this macro lets you map
your symbol choice to the base name when building a static library

Also, while I'm at it, since we're documenting this in the guide, take the
abbreviated example out of the header

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas.monjalon@6wind.com
---
 lib/librte_compat/rte_compat.h | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index 75920a1..d7768d5 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -49,22 +49,8 @@
  * Assumptions: DPDK 2.(X) contains a function int foo(char *string)
  *              DPDK 2.(X+1) needs to change foo to be int foo(int index)
  *
- * To accomplish this:
- * 1) Edit lib/<library>/library_version.map to add a DPDK_2.(X+1) node, in which
- * foo is exported as a global symbol.
- *
- * 2) rename the existing function int foo(char *string) to
- *	int foo_v20(char *string)
- *
- * 3) Add this macro immediately below the function
- *	VERSION_SYMBOL(foo, _v20, 2.0);
- *
- * 4) Implement a new version of foo.
- *	char foo(int value, int otherval) { ...}
- *
- * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
- *
+ * Refer to the guidelines document in the docs subdirectory for details on the
+ * use of these macros
  */
 
 /*
@@ -72,6 +58,8 @@
  * 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
  */
 
 /*
@@ -96,6 +84,19 @@
 #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
@@ -104,7 +105,7 @@
 #define __vsym
 #define BASE_SYMBOL(b, n)
 #define BIND_DEFAULT_SYMBOL(b, e, n)
-
+#define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))
 /*
  * RTE_BUILD_SHARED_LIB=n
  */
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation
  2015-06-25 14:35 ` [dpdk-dev] [PATCHv3 1/3] " Neil Horman
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
@ 2015-06-25 14:35   ` Neil Horman
  2015-06-26 13:00     ` Thomas Monjalon
  2015-06-26 12:45   ` [dpdk-dev] [PATCHv3 1/3] rte_compat.h : Clean up some typos Thomas Monjalon
  2 siblings, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-25 14:35 UTC (permalink / raw)
  To: dev

People have been asking for ways to use the ABI macros, heres some docs to
clarify their use.  Included is:

* An overview of what ABI is
* Details of the ABI deprecation process
* Details of the versioning macros
* Examples of their use
* Details of how to use the ABI validator

Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
working on it.  Much of the introductory material was gathered and cleaned up by
him

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: john.mcnamara@intel.com
CC: thomas.monjalon@6wind.com

Change notes:

v2)
     * Fixed RST indentations and spelling errors
     * Rebased to upstream to fix index.rst conflict

v3)
     * Fixed in tact -> intact
     * Added docs to address static linking
     * Removed duplicate documentation from release notes
---
 doc/guides/guidelines/index.rst      |   1 +
 doc/guides/guidelines/versioning.rst | 484 +++++++++++++++++++++++++++++++++++
 doc/guides/rel_notes/abi.rst         |  30 +--
 3 files changed, 487 insertions(+), 28 deletions(-)
 create mode 100644 doc/guides/guidelines/versioning.rst

diff --git a/doc/guides/guidelines/index.rst b/doc/guides/guidelines/index.rst
index 0ee9ab3..bfb9fa3 100644
--- a/doc/guides/guidelines/index.rst
+++ b/doc/guides/guidelines/index.rst
@@ -7,3 +7,4 @@ Guidelines
 
     coding_style
     design
+    versioning
diff --git a/doc/guides/guidelines/versioning.rst b/doc/guides/guidelines/versioning.rst
new file mode 100644
index 0000000..da9eca0
--- /dev/null
+++ b/doc/guides/guidelines/versioning.rst
@@ -0,0 +1,484 @@
+Managing ABI updates
+====================
+
+Description
+-----------
+
+This document details some methods for handling ABI management in the DPDK.
+Note this document is not exhaustive, in that C library versioning is flexible
+allowing multiple methods to achieve various goals, but it will provide the user
+with some introductory methods
+
+General Guidelines
+------------------
+
+#. Whenever possible, ABI should be preserved
+#. The addition of symbols is generally not problematic
+#. The modification of symbols can generally be managed with versioning
+#. The removal of symbols generally is an ABI break and requires bumping of the
+   LIBABIVER macro
+
+What is an ABI
+--------------
+
+An ABI (Application Binary Interface) is the set of runtime interfaces exposed
+by a library. It is similar to an API (Application Programming Interface) but
+is the result of compilation.  It is also effectively cloned when applications
+link to dynamic libraries.  That is to say when an application is compiled to
+link against dynamic libraries, it is assumed that the ABI remains constant
+between the time the application is compiled/linked, and the time that it runs.
+Therefore, in the case of dynamic linking, it is critical that an ABI is
+preserved, or (when modified), done in such a way that the application is unable
+to behave improperly or in an unexpected fashion.
+
+The DPDK ABI policy
+-------------------
+
+ABI versions are set at the time of major release labeling, and the ABI may
+change multiple times, without warning, between the last release label and the
+HEAD label of the git tree.
+
+ABI versions, once released, are available until such time as their
+deprecation has been noted in the Release Notes for at least one major release
+cycle. For example consider the case where the ABI for DPDK 2.0 has been
+shipped and then a decision is made to modify it during the development of
+DPDK 2.1. The decision will be recorded in the Release Notes for the DPDK 2.1
+release and the modification will be made available in the DPDK 2.2 release.
+
+ABI versions may be deprecated in whole or in part as needed by a given
+update.
+
+Some ABI changes may be too significant to reasonably maintain multiple
+versions. In those cases ABI's may be updated without backward compatibility
+being provided. The requirements for doing so are:
+
+#. At least 3 acknowledgments of the need to do so must be made on the
+   dpdk.org mailing list.
+
+#. A full deprecation cycle, as explained above, must be made to offer
+   downstream consumers sufficient warning of the change.
+
+#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
+   incorporated must be incremented in parallel with the ABI changes
+   themselves.
+
+Note that the above process for ABI deprecation should not be undertaken
+lightly. ABI stability is extremely important for downstream consumers of the
+DPDK, especially when distributed in shared object form. Every effort should
+be made to preserve the ABI whenever possible. The ABI should only be changed
+for significant reasons, such as performance enhancements. ABI breakage due to
+changes such as reorganizing public structure fields for aesthetic or
+readability purposes should be avoided.
+
+Examples of Deprecation Notices
+-------------------------------
+
+The following are some examples of ABI deprecation notices which would be
+added to the Release Notes:
+
+* The Macro ``#RTE_FOO`` is deprecated and will be removed with version 2.0,
+  to be replaced with the inline function ``rte_foo()``.
+
+* The function ``rte_mbuf_grok()`` has been updated to include a new parameter
+  in version 2.0. Backwards compatibility will be maintained for this function
+  until the release of version 2.1
+
+* The members of ``struct rte_foo`` have been reorganized in release 2.0 for
+  performance reasons. Existing binary applications will have backwards
+  compatibility in release 2.0, while newly built binaries will need to
+  reference the new structure variant ``struct rte_foo2``. Compatibility will
+  be removed in release 2.2, and all applications will require updating and
+  rebuilding to the new structure at that time, which will be renamed to the
+  original ``struct rte_foo``.
+
+* Significant ABI changes are planned for the ``librte_dostuff`` library. The
+  upcoming release 2.0 will not contain these changes, but release 2.1 will,
+  and no backwards compatibility is planned due to the extensive nature of
+  these changes. Binaries using this library built prior to version 2.1 will
+  require updating and recompilation.
+
+Versioning Macros
+-----------------
+
+When a symbol is exported from a library to provide an API, it also provides a
+calling convention (ABI) that is embodied in its name, return type and
+arguments. Occasionally that function may need to change to accommodate new
+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 ``lib/librte_compat/rte_compat.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
+library so that older binaries need not be immediately recompiled.
+
+The macros exported are:
+
+* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
+  unversioned symbol ``b`` to the internal function ``b_e``.
+
+
+* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
+  unversioned symbol ``b`` to the internal function ``b_e``.
+
+* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry instructing
+  the linker to bind references to symbol ``b`` to the internal symbol
+  ``b_e``.
+
+* ``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.
+
+Examples of ABI Macro use
+-------------------------
+
+Updating a public API
+~~~~~~~~~~~~~~~~~~~~~
+
+Assume we have a function as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param)
+ {
+        ...
+ }
+
+
+Assume that struct rte_acl_ctx is a private structure, and that a developer
+wishes to enhance the acl api so that a debugging flag can be enabled on a
+per-context basis.  This requires an addition to the structure (which, being
+private, is safe), but it also requires modifying the code as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param, int debug)
+ {
+        ...
+ }
+
+
+Note also that, being a public function, the header file prototype must also be
+changed, as must all the call sites, to reflect the new ABI footprint.  We will
+maintain previous ABI versions that are accessible only to previously compiled
+binaries
+
+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
+
+.. code-block:: none 
+
+   DPDK_2.0 {
+        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_2.0 {
+        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_2.1 {
+        global:
+        rte_acl_create;
+
+   } DPDK_2.0;
+
+The addition of the new block tells the linker that a new version node is
+available (DPDK_2.1), which contains the symbol rte_acl_create, and inherits the
+symbols from the DPDK_2.0 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 map 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
+
+.. code-block:: c
+
+  struct rte_acl_ctx *
+ -rte_acl_create(const struct rte_acl_param *param)
+ +rte_acl_create_v20(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 condusive to
+the macros used for versioning symbols.  That is our next step, mapping this new
+symbol name to the initial symbol name at version node 2.0.  Immediately after
+the function, we add this line of code
+
+.. code-block:: c
+
+   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+Remembering to also add the rte_compat.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
+the original rte_acl_create symbol to the original function (but with a new
+name)
+
+Next, we need to create the 2.1 version of the symbol.  We create a new function
+name, with a different suffix, and  implement it appropriately
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   rte_acl_create_v21(const struct rte_acl_param *param, int debug);
+   {
+        struct rte_acl_ctx *ctx = rte_acl_create_v20(param);
+
+        ctx->debug = debug;
+
+        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 symbol
+``rte_acl_create@DPDK_2.1``.  To do this, we modify the public prototype of the call
+in the header file, adding the macro there to inform all including applications,
+that on re-link, the default rte_acl_create symbol should point to this
+function.  Note that we could do this by simply naming the function above
+rte_acl_create, and the linker would chose the most recent version tag to apply
+in the version script, but we can also do this in the header file
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   -rte_acl_create(const struct rte_acl_param *param);
+   +rte_acl_create(const struct rte_acl_param *param, int debug);
+   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+The BIND_DEFAULT_SYMBOL macro explicitly tells applications that include this
+header, to link to the rte_acl_create_v21 function and apply the DPDK_2.1
+version node to it.  This method is more explicit and flexible than just
+re-implementing the exact symbol name, and allows for other features (such as
+linking to the old symbol version by default, when the new ABI is to be opt-in
+for a period.
+
+One last thing we need to do.  Note that 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_v21`` is
+defined, we add this
+
+.. code-block:: c
+
+   struct rte_acl_create_v21(const struct rte_acl_param *param, int debug)
+   {
+        ...
+   }
+   MAP_STATIC_SYMBOL(struct rte_acl_create(const struct rte_acl_param *param, int debug), rte_acl_create_v21);
+
+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_v21``
+
+That's it, on the next shared library rebuild, there will be two versions of
+rte_acl_create, an old DPDK_2.0 version, used by previously built applications,
+and a new DPDK_2.1 version, used by future built applications.
+
+
+Deprecating part of a public API
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Lets assume that you've done the above update, and after a few releases have
+passed you decide you would like to retire the old version of the function.
+After having gone through the ABI deprecation announcement process, removal is
+easy.  Start by removing the symbol from the requisite version map file:
+
+.. code-block:: none
+
+   DPDK_2.0 {
+        global:
+
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+ -      rte_acl_create
+        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_2.1 {
+        global:
+        rte_acl_create;
+   } DPDK_2.0;
+
+
+Next remove the corresponding versioned export
+.. code-block:: c
+
+ -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+
+Note that the internal function definition could also be removed, but its used
+in our example by the newer version _v21, so we leave it in place.  This is a
+coding style choice.
+
+Lastly, we need to bump the LIBABIVER number for this library in the Makefile to
+indicate to applications doing dynamic linking that this is a later, and
+possibly incompatible library version:
+
+.. code-block:: c
+
+   -LIBABIVER := 1
+   +LIBABIVER := 2
+
+Deprecating an entire ABI version
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+While removing a symbol from and ABI may be useful, it is often more practical
+to remove an entire version node at once.  If a version node completely
+specifies an API, then removing part of it, typically makes it incomplete.  In
+those cases it is better to remove the entire node
+ 
+To do this, start by modifying the version map file, such that all symbols from
+the node to be removed are merged into the next node in the map
+
+In the case of our map above, it would transform to look as follows
+
+.. code-block:: none
+
+   DPDK_2.1 {              
+        global:
+              
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+        rte_acl_create
+        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: *;
+ };           
+
+Then any uses of BIND_DEFAULT_SYMBOL that pointed to the old node should be
+updated to point to the new version node in any header files for all affected
+symbols.
+
+.. code-block:: c
+
+ -BIND_DEFAULT_SYMBOL(rte_acl_create, _v20, 2.0);
+ +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+Lastly, any VERSION_SYMBOL macros that point to the old version node should be
+removed, taking care to keep, where need old code in place to support newer
+versions of the symbol.
+
+Running the ABI Validator
+-------------------------
+
+The ``scripts`` directory in the DPDK source tree contains a utility program,
+``validate-abi.sh``, for validating the DPDK ABI based on the Linux `ABI
+Compliance Checker
+<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_.
+
+This has a dependency on the ``abi-compliance-checker`` and ``and abi-dumper``
+utilities which can be installed via a package manager. For example::
+
+   sudo yum install abi-compliance-checker
+   sudo yum install abi-dumper
+
+The syntax of the ``validate-abi.sh`` utility is::
+
+   ./scripts/validate-abi.sh <TAG1> <TAG2> <TARGET>
+
+Where ``TAG1`` and ``TAG2`` are valid git tags on the local repo and target is
+the usual DPDK compilation target.
+
+For example to test the current committed HEAD against a previous release tag
+we could add a temporary tag and run the utility as follows::
+
+   git tag MY_TEMP_TAG
+   ./scripts/validate-abi.sh v2.0.0 MY_TEMP_TAG x86_64-native-linuxapp-gcc
+
+After the validation script completes (it can take a while since it need to
+compile both tags) it will create compatibility reports in the
+``./compat_report`` directory. Listed incompatibilities can be found as
+follows::
+
+  grep -lr Incompatible compat_reports/
diff --git a/doc/guides/rel_notes/abi.rst b/doc/guides/rel_notes/abi.rst
index f00a6ee..4086198 100644
--- a/doc/guides/rel_notes/abi.rst
+++ b/doc/guides/rel_notes/abi.rst
@@ -1,33 +1,7 @@
 ABI policy
 ==========
-ABI versions are set at the time of major release labeling, and ABI may change
-multiple times between the last labeling and the HEAD label of the git tree
-without warning.
-
-ABI versions, once released are available until such time as their
-deprecation has been noted here for at least one major release cycle, after it
-has been tagged.  E.g. the ABI for DPDK 2.0 is shipped, and then the decision to
-remove it is made during the development of DPDK 2.1.  The decision will be
-recorded here, shipped with the DPDK 2.1 release, and actually removed when DPDK
-2.2 ships.
-
-ABI versions may be deprecated in whole, or in part as needed by a given update.
-
-Some ABI changes may be too significant to reasonably maintain multiple
-versions of.  In those events ABI's may be updated without backward
-compatibility provided.  The requirements for doing so are:
-
-#. At least 3 acknowledgments of the need on the dpdk.org
-#. A full deprecation cycle must be made to offer downstream consumers sufficient warning of the change.  E.g. if dpdk 2.0 is under development when the change is proposed, a deprecation notice must be added to this file, and released with dpdk 2.0.  Then the change may be incorporated for dpdk 2.1
-#. The LIBABIVER variable in the makefile(s) where the ABI changes are incorporated must be incremented in parallel with the ABI changes themselves
-
-Note that the above process for ABI deprecation should not be undertaken
-lightly.  ABI stability is extremely important for downstream consumers of the
-DPDK, especially when distributed in shared object form.  Every effort should be
-made to preserve ABI whenever possible.  For instance, reorganizing public
-structure field for aesthetic or readability purposes should be avoided as it will
-cause ABI breakage.  Only significant (e.g. performance) reasons should be seen
-as cause to alter ABI.
+See the guidelines document for details of the ABI policy.  ABI deprecation
+notices are to be posted here
 
 Examples of Deprecation Notices
 -------------------------------
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
@ 2015-06-26 10:13     ` Gajdzica, MaciejX T
  2015-06-26 12:52     ` Thomas Monjalon
  1 sibling, 0 replies; 41+ messages in thread
From: Gajdzica, MaciejX T @ 2015-06-26 10:13 UTC (permalink / raw)
  To: Neil Horman, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Thursday, June 25, 2015 4:36 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL
> macro
> 
> It was pointed out in my examples that doing shared library symbol versioning by
> partitioning symbols to version specific functions (as opposed to leaving the
> latest symol version at the base symbol name), neglects to take into account
> static builds.  Add a macro to handle that.  If you choose a versioning approach
> that uniquely names every version of the symbol, then this macro lets you map
> your symbol choice to the base name when building a static library
> 
> Also, while I'm at it, since we're documenting this in the guide, take the
> abbreviated example out of the header
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: thomas.monjalon@6wind.com

Acked-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>

> ---
>  lib/librte_compat/rte_compat.h | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
> index 75920a1..d7768d5 100644
> --- a/lib/librte_compat/rte_compat.h
> +++ b/lib/librte_compat/rte_compat.h
> @@ -49,22 +49,8 @@
>   * Assumptions: DPDK 2.(X) contains a function int foo(char *string)
>   *              DPDK 2.(X+1) needs to change foo to be int foo(int index)
>   *
> - * To accomplish this:
> - * 1) Edit lib/<library>/library_version.map to add a DPDK_2.(X+1) node, in
> which
> - * foo is exported as a global symbol.
> - *
> - * 2) rename the existing function int foo(char *string) to
> - *	int foo_v20(char *string)
> - *
> - * 3) Add this macro immediately below the function
> - *	VERSION_SYMBOL(foo, _v20, 2.0);
> - *
> - * 4) Implement a new version of foo.
> - *	char foo(int value, int otherval) { ...}
> - *
> - * 5) Mark the newest version as the default version
> - *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
> - *
> + * Refer to the guidelines document in the docs subdirectory for
> + details on the
> + * use of these macros
>   */
> 
>  /*
> @@ -72,6 +58,8 @@
>   * 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
>   */
> 
>  /*
> @@ -96,6 +84,19 @@
>  #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
> @@ -104,7 +105,7 @@
>  #define __vsym
>  #define BASE_SYMBOL(b, n)
>  #define BIND_DEFAULT_SYMBOL(b, e, n)
> -
> +#define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))
>  /*
>   * RTE_BUILD_SHARED_LIB=n
>   */
> --
> 2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 1/3] rte_compat.h : Clean up some typos
  2015-06-25 14:35 ` [dpdk-dev] [PATCHv3 1/3] " Neil Horman
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation Neil Horman
@ 2015-06-26 12:45   ` Thomas Monjalon
  2 siblings, 0 replies; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-26 12:45 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-25 10:35, Neil Horman:
>  #define VERSION_SYMBOL(b, e, v)
>  #define __vsym
>  #define BASE_SYMBOL(b, n)

You forgot to fix the parameter names to (b, e, n) and (b, e).

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
  2015-06-26 10:13     ` Gajdzica, MaciejX T
@ 2015-06-26 12:52     ` Thomas Monjalon
  2015-06-26 14:30       ` Neil Horman
  1 sibling, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-26 12:52 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-25 10:35, Neil Horman:
> +/*
> + * 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
> @@ -104,7 +105,7 @@
>  #define __vsym
>  #define BASE_SYMBOL(b, n)
>  #define BIND_DEFAULT_SYMBOL(b, e, n)
> -
> +#define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))

Is it working with clang and icc?
Why not just define foo as foo_v2?
As this is the equivalent of BIND_DEFAULT_SYMBOL for the static case,
it would be easier to mix them in only one macro.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation
  2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation Neil Horman
@ 2015-06-26 13:00     ` Thomas Monjalon
  2015-06-26 14:54       ` Neil Horman
  0 siblings, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-26 13:00 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-25 10:35, Neil Horman:
> v3)
>      * Fixed in tact -> intact
>      * Added docs to address static linking
>      * Removed duplicate documentation from release notes

It seems you missed some of my previous comments.

[...]
> +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.

Should a versioned symbol <b>@DPDK_<n>

> +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.

Please give a use case of BASE_SYMBOL.

[...]
> +The addition of the new block tells the linker that a new version node is
> +available (DPDK_2.1), which contains the symbol rte_acl_create, and inherits the
> +symbols from the DPDK_2.0 node.

which contains the old version of the symbol rte_acl_create

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro
  2015-06-26 12:52     ` Thomas Monjalon
@ 2015-06-26 14:30       ` Neil Horman
  2015-06-28 20:13         ` Thomas Monjalon
  0 siblings, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-26 14:30 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Fri, Jun 26, 2015 at 02:52:50PM +0200, Thomas Monjalon wrote:
> 2015-06-25 10:35, Neil Horman:
> > +/*
> > + * 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
> > @@ -104,7 +105,7 @@
> >  #define __vsym
> >  #define BASE_SYMBOL(b, n)
> >  #define BIND_DEFAULT_SYMBOL(b, e, n)
> > -
> > +#define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))
> 
> Is it working with clang and icc?
No idea.  It should work with clang (though I don't have it installed at the
moment), as the docs say the .symver directive is supported

as for icc, thats out of my control completely, as I don't have any access to
it.

> Why not just define foo as foo_v2?
I'm not sure what you mean here.  Are you suggesting that we just change the abi
so applications have to call foo_v2 rather than foo when we change the
prototype.  I suppose we could do that, but that seems like it would be an awful
irritant to users.  They would rather have a single symbol to call if it does
the same function.

> As this is the equivalent of BIND_DEFAULT_SYMBOL for the static case,
> it would be easier to mix them in only one macro.
> 
Because of where its used.  If you use BIND_DEFAULT_SYMBOL to do the work of
MAP_STATIC_SYMBOL, every compilation unit will define its own alias and you'll
get symbol conflicts.

Neil

> 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation
  2015-06-26 13:00     ` Thomas Monjalon
@ 2015-06-26 14:54       ` Neil Horman
  2015-06-28 20:24         ` Thomas Monjalon
  0 siblings, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-26 14:54 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Fri, Jun 26, 2015 at 03:00:17PM +0200, Thomas Monjalon wrote:
> 2015-06-25 10:35, Neil Horman:
> > v3)
> >      * Fixed in tact -> intact
> >      * Added docs to address static linking
> >      * Removed duplicate documentation from release notes
> 
> It seems you missed some of my previous comments.
> 
> [...]
> > +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> > +  unversioned symbol ``b`` to the internal function ``b_e``.
> 
> Should a versioned symbol <b>@DPDK_<n>
> 
Sure.

> > +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> > +  unversioned symbol ``b`` to the internal function ``b_e``.
> 
> Please give a use case of BASE_SYMBOL.
> 
No, I'd rather remove it if you really insist.  As noted before the way we set
up the version map files means we currently have no need for this particular
directive.  I only included it for completeness.  I think an example use in
light of that fact would only confuse people.  If you're going to draw a line in
the sand around it, I'll just remove it.

> [...]
> > +The addition of the new block tells the linker that a new version node is
> > +available (DPDK_2.1), which contains the symbol rte_acl_create, and inherits the
> > +symbols from the DPDK_2.0 node.
> 
> which contains the old version of the symbol rte_acl_create
> 
I don't understand, is it not obvious that the DPDK_2.0 node contains the 2.0
version of the symbol and the DPDK_2.1 node contains the 2.1 version of the
symbol?

Neil

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro
  2015-06-26 14:30       ` Neil Horman
@ 2015-06-28 20:13         ` Thomas Monjalon
  2015-06-29 13:44           ` Neil Horman
  0 siblings, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-28 20:13 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-26 10:30, Neil Horman:
> On Fri, Jun 26, 2015 at 02:52:50PM +0200, Thomas Monjalon wrote:
> > 2015-06-25 10:35, Neil Horman:
> > > +/*
> > > + * 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
> > > @@ -104,7 +105,7 @@
> > >  #define __vsym
> > >  #define BASE_SYMBOL(b, n)
> > >  #define BIND_DEFAULT_SYMBOL(b, e, n)
> > > -
> > > +#define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))
> > 
> > Is it working with clang and icc?
> No idea.  It should work with clang (though I don't have it installed at the
> moment), as the docs say the .symver directive is supported
> 
> as for icc, thats out of my control completely, as I don't have any access to
> it.
> 
> > Why not just define foo as foo_v2?
> I'm not sure what you mean here.  Are you suggesting that we just change the abi
> so applications have to call foo_v2 rather than foo when we change the
> prototype.  I suppose we could do that, but that seems like it would be an awful
> irritant to users.  They would rather have a single symbol to call if it does
> the same function.
> 
> > As this is the equivalent of BIND_DEFAULT_SYMBOL for the static case,
> > it would be easier to mix them in only one macro.
> > 
> Because of where its used.  If you use BIND_DEFAULT_SYMBOL to do the work of
> MAP_STATIC_SYMBOL, every compilation unit will define its own alias and you'll
> get symbol conflicts.

Oh, you mean it shouldn't be used in a .h file?
If so, this limitation should be noted in the description above.

OK for that solution, that's the best we have at the moment.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation
  2015-06-26 14:54       ` Neil Horman
@ 2015-06-28 20:24         ` Thomas Monjalon
  2015-06-29 13:53           ` Neil Horman
  0 siblings, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-28 20:24 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-26 10:54, Neil Horman:
> On Fri, Jun 26, 2015 at 03:00:17PM +0200, Thomas Monjalon wrote:
> > 2015-06-25 10:35, Neil Horman:
> > > +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> > > +  unversioned symbol ``b`` to the internal function ``b_e``.
> > 
> > Should a versioned symbol <b>@DPDK_<n>
> > 
> Sure.

When fixed, this series can be applied.

> > > +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> > > +  unversioned symbol ``b`` to the internal function ``b_e``.
> > 
> > Please give a use case of BASE_SYMBOL.
> > 
> No, I'd rather remove it if you really insist.  As noted before the way we set
> up the version map files means we currently have no need for this particular
> directive.  I only included it for completeness.  I think an example use in
> light of that fact would only confuse people.  If you're going to draw a line in
> the sand around it, I'll just remove it.

No line in the sand, but it seems better to remove it to avoid confusing people.
ABI compat is already enough difficult to understand ;)

> > [...]
> > > +The addition of the new block tells the linker that a new version node is
> > > +available (DPDK_2.1), which contains the symbol rte_acl_create, and inherits the
> > > +symbols from the DPDK_2.0 node.
> > 
> > which contains the old version of the symbol rte_acl_create
> > 
> I don't understand, is it not obvious that the DPDK_2.0 node contains the 2.0
> version of the symbol and the DPDK_2.1 node contains the 2.1 version of the
> symbol?

Yes it is.
I thought it was needed to insist on the existence of a new symbol while the
old one still exists. Just trying to be didactic.

Hope this documentation will help to unlock the patch series currently
breaking the ABI.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro
  2015-06-28 20:13         ` Thomas Monjalon
@ 2015-06-29 13:44           ` Neil Horman
  0 siblings, 0 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-29 13:44 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Sun, Jun 28, 2015 at 10:13:31PM +0200, Thomas Monjalon wrote:
> 2015-06-26 10:30, Neil Horman:
> > On Fri, Jun 26, 2015 at 02:52:50PM +0200, Thomas Monjalon wrote:
> > > 2015-06-25 10:35, Neil Horman:
> > > > +/*
> > > > + * 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
> > > > @@ -104,7 +105,7 @@
> > > >  #define __vsym
> > > >  #define BASE_SYMBOL(b, n)
> > > >  #define BIND_DEFAULT_SYMBOL(b, e, n)
> > > > -
> > > > +#define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))
> > > 
> > > Is it working with clang and icc?
> > No idea.  It should work with clang (though I don't have it installed at the
> > moment), as the docs say the .symver directive is supported
> > 
> > as for icc, thats out of my control completely, as I don't have any access to
> > it.
> > 
> > > Why not just define foo as foo_v2?
> > I'm not sure what you mean here.  Are you suggesting that we just change the abi
> > so applications have to call foo_v2 rather than foo when we change the
> > prototype.  I suppose we could do that, but that seems like it would be an awful
> > irritant to users.  They would rather have a single symbol to call if it does
> > the same function.
> > 
> > > As this is the equivalent of BIND_DEFAULT_SYMBOL for the static case,
> > > it would be easier to mix them in only one macro.
> > > 
> > Because of where its used.  If you use BIND_DEFAULT_SYMBOL to do the work of
> > MAP_STATIC_SYMBOL, every compilation unit will define its own alias and you'll
> > get symbol conflicts.
> 
> Oh, you mean it shouldn't be used in a .h file?
> If so, this limitation should be noted in the description above.
> 
Its already noted in the example, I'd rather not make a unilateral statement
about where to use it above because there can be cause to use it internally as
well.

> OK for that solution, that's the best we have at the moment.
> 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation
  2015-06-28 20:24         ` Thomas Monjalon
@ 2015-06-29 13:53           ` Neil Horman
  0 siblings, 0 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-29 13:53 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Sun, Jun 28, 2015 at 10:24:42PM +0200, Thomas Monjalon wrote:
> 2015-06-26 10:54, Neil Horman:
> > On Fri, Jun 26, 2015 at 03:00:17PM +0200, Thomas Monjalon wrote:
> > > 2015-06-25 10:35, Neil Horman:
> > > > +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> > > > +  unversioned symbol ``b`` to the internal function ``b_e``.
> > > 
> > > Should a versioned symbol <b>@DPDK_<n>
> > > 
> > Sure.
> 
> When fixed, this series can be applied.
> 
> > > > +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> > > > +  unversioned symbol ``b`` to the internal function ``b_e``.
> > > 
> > > Please give a use case of BASE_SYMBOL.
> > > 
> > No, I'd rather remove it if you really insist.  As noted before the way we set
> > up the version map files means we currently have no need for this particular
> > directive.  I only included it for completeness.  I think an example use in
> > light of that fact would only confuse people.  If you're going to draw a line in
> > the sand around it, I'll just remove it.
> 
> No line in the sand, but it seems better to remove it to avoid confusing people.
> ABI compat is already enough difficult to understand ;)
> 
> > > [...]
> > > > +The addition of the new block tells the linker that a new version node is
> > > > +available (DPDK_2.1), which contains the symbol rte_acl_create, and inherits the
> > > > +symbols from the DPDK_2.0 node.
> > > 
> > > which contains the old version of the symbol rte_acl_create
> > > 
> > I don't understand, is it not obvious that the DPDK_2.0 node contains the 2.0
> > version of the symbol and the DPDK_2.1 node contains the 2.1 version of the
> > symbol?
> 
> Yes it is.
> I thought it was needed to insist on the existence of a new symbol while the
> old one still exists. Just trying to be didactic.
> 
Ah, no its not a mandate (though doing so is an optional aproach, which trades
off the need to use MAP_STATIC_SYMBOL for the maintain a public symbol with the
same name).  I can include an alternate example of the same exercize that
follows that approach, but given our time contraints would rather to that in a
follow up patch series


Neil

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos
  2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
                   ` (4 preceding siblings ...)
  2015-06-25 14:35 ` [dpdk-dev] [PATCHv3 1/3] " Neil Horman
@ 2015-06-29 13:59 ` Neil Horman
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 2/4] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
                     ` (3 more replies)
  5 siblings, 4 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-29 13:59 UTC (permalink / raw)
  To: dev

Clean up some macro definition typos and comments

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas.monjalon@6wind.com
---
 lib/librte_compat/rte_compat.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index fb0dc19..75920a1 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -54,7 +54,7 @@
  * foo is exported as a global symbol.
  *
  * 2) rename the existing function int foo(char *string) to
- *	int __vsym foo_v20(char *string)
+ *	int foo_v20(char *string)
  *
  * 3) Add this macro immediately below the function
  *	VERSION_SYMBOL(foo, _v20, 2.0);
@@ -63,7 +63,7 @@
  *	char foo(int value, int otherval) { ...}
  *
  * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, 2.1);
+ *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
  *
  */
 
@@ -79,21 +79,21 @@
  * 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))
+#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
 
 /*
  * BASE_SYMBOL
  * Creates a symbol version table entry binding unversioned symbol <b>
  * to the internal function <b>_<e>
  */
-#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@")
+#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")
 
 /*
- * BNID_DEFAULT_SYMBOL
+ * 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 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))
 
 #else
@@ -103,7 +103,7 @@
 #define VERSION_SYMBOL(b, e, v)
 #define __vsym
 #define BASE_SYMBOL(b, n)
-#define BIND_DEFAULT_SYMBOL(b, v)
+#define BIND_DEFAULT_SYMBOL(b, e, n)
 
 /*
  * RTE_BUILD_SHARED_LIB=n
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv4 2/4] rte_compat: Add MAP_STATIC_SYMBOL macro
  2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
@ 2015-06-29 13:59   ` Neil Horman
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 3/4] rte_compat: remove BASE_SYMBOL Neil Horman
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-29 13:59 UTC (permalink / raw)
  To: dev

It was pointed out in my examples that doing shared library symbol versioning by
partitioning symbols to version specific functions (as opposed to leaving the
latest symol version at the base symbol name), neglects to take into account
static builds.  Add a macro to handle that.  If you choose a versioning approach
that uniquely names every version of the symbol, then this macro lets you map
your symbol choice to the base name when building a static library

Also, while I'm at it, since we're documenting this in the guide, take the
abbreviated example out of the header

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas.monjalon@6wind.com
---
 lib/librte_compat/rte_compat.h | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index 75920a1..d7768d5 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -49,22 +49,8 @@
  * Assumptions: DPDK 2.(X) contains a function int foo(char *string)
  *              DPDK 2.(X+1) needs to change foo to be int foo(int index)
  *
- * To accomplish this:
- * 1) Edit lib/<library>/library_version.map to add a DPDK_2.(X+1) node, in which
- * foo is exported as a global symbol.
- *
- * 2) rename the existing function int foo(char *string) to
- *	int foo_v20(char *string)
- *
- * 3) Add this macro immediately below the function
- *	VERSION_SYMBOL(foo, _v20, 2.0);
- *
- * 4) Implement a new version of foo.
- *	char foo(int value, int otherval) { ...}
- *
- * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
- *
+ * Refer to the guidelines document in the docs subdirectory for details on the
+ * use of these macros
  */
 
 /*
@@ -72,6 +58,8 @@
  * 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
  */
 
 /*
@@ -96,6 +84,19 @@
 #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
@@ -104,7 +105,7 @@
 #define __vsym
 #define BASE_SYMBOL(b, n)
 #define BIND_DEFAULT_SYMBOL(b, e, n)
-
+#define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))
 /*
  * RTE_BUILD_SHARED_LIB=n
  */
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv4 3/4] rte_compat: remove BASE_SYMBOL
  2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 2/4] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
@ 2015-06-29 13:59   ` Neil Horman
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 4/4] ABI: Add some documentation Neil Horman
  2015-07-08  9:52   ` [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos Thomas Monjalon
  3 siblings, 0 replies; 41+ messages in thread
From: Neil Horman @ 2015-06-29 13:59 UTC (permalink / raw)
  To: dev

BASE_SYMBOL was only here for completelness.  DPDK currently doesn't need it, so
lets remove it.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
 lib/librte_compat/rte_compat.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index d7768d5..4f60a66 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -70,13 +70,6 @@
 #define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
 
 /*
- * BASE_SYMBOL
- * Creates a symbol version table entry binding unversioned symbol <b>
- * to the internal function <b>_<e>
- */
-#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")
-
-/*
  * BIND_DEFAULT_SYMBOL
  * Creates a symbol version entry instructing the linker to bind references to
  * symbol <b> to the internal symbol <b>_<e>
@@ -103,7 +96,6 @@
  */
 #define VERSION_SYMBOL(b, e, v)
 #define __vsym
-#define BASE_SYMBOL(b, n)
 #define BIND_DEFAULT_SYMBOL(b, e, n)
 #define MAP_STATIC_SYMBOL(f, p) f  __attribute__((alias( RTE_STR(p))))
 /*
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCHv4 4/4] ABI: Add some documentation
  2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 2/4] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 3/4] rte_compat: remove BASE_SYMBOL Neil Horman
@ 2015-06-29 13:59   ` Neil Horman
  2015-06-29 15:07     ` Thomas Monjalon
  2015-07-08  9:52   ` [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos Thomas Monjalon
  3 siblings, 1 reply; 41+ messages in thread
From: Neil Horman @ 2015-06-29 13:59 UTC (permalink / raw)
  To: dev

People have been asking for ways to use the ABI macros, heres some docs to
clarify their use.  Included is:

* An overview of what ABI is
* Details of the ABI deprecation process
* Details of the versioning macros
* Examples of their use
* Details of how to use the ABI validator

Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
working on it.  Much of the introductory material was gathered and cleaned up by
him

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: john.mcnamara@intel.com
CC: thomas.monjalon@6wind.com

Change notes:

v2)
     * Fixed RST indentations and spelling errors
     * Rebased to upstream to fix index.rst conflict

v3)
     * Fixed in tact -> intact
     * Added docs to address static linking
     * Removed duplicate documentation from release notes

v4)
     * Fixed more grammatical errors / etc
     * removed BASE_SYMBOL from documentation
---
 doc/guides/guidelines/index.rst      |   1 +
 doc/guides/guidelines/versioning.rst | 480 +++++++++++++++++++++++++++++++++++
 doc/guides/rel_notes/abi.rst         |  30 +--
 3 files changed, 483 insertions(+), 28 deletions(-)
 create mode 100644 doc/guides/guidelines/versioning.rst

diff --git a/doc/guides/guidelines/index.rst b/doc/guides/guidelines/index.rst
index 0ee9ab3..bfb9fa3 100644
--- a/doc/guides/guidelines/index.rst
+++ b/doc/guides/guidelines/index.rst
@@ -7,3 +7,4 @@ Guidelines
 
     coding_style
     design
+    versioning
diff --git a/doc/guides/guidelines/versioning.rst b/doc/guides/guidelines/versioning.rst
new file mode 100644
index 0000000..63526db
--- /dev/null
+++ b/doc/guides/guidelines/versioning.rst
@@ -0,0 +1,480 @@
+Managing ABI updates
+====================
+
+Description
+-----------
+
+This document details some methods for handling ABI management in the DPDK.
+Note this document is not exhaustive, in that C library versioning is flexible
+allowing multiple methods to achieve various goals, but it will provide the user
+with some introductory methods
+
+General Guidelines
+------------------
+
+#. Whenever possible, ABI should be preserved
+#. The addition of symbols is generally not problematic
+#. The modification of symbols can generally be managed with versioning
+#. The removal of symbols generally is an ABI break and requires bumping of the
+   LIBABIVER macro
+
+What is an ABI
+--------------
+
+An ABI (Application Binary Interface) is the set of runtime interfaces exposed
+by a library. It is similar to an API (Application Programming Interface) but
+is the result of compilation.  It is also effectively cloned when applications
+link to dynamic libraries.  That is to say when an application is compiled to
+link against dynamic libraries, it is assumed that the ABI remains constant
+between the time the application is compiled/linked, and the time that it runs.
+Therefore, in the case of dynamic linking, it is critical that an ABI is
+preserved, or (when modified), done in such a way that the application is unable
+to behave improperly or in an unexpected fashion.
+
+The DPDK ABI policy
+-------------------
+
+ABI versions are set at the time of major release labeling, and the ABI may
+change multiple times, without warning, between the last release label and the
+HEAD label of the git tree.
+
+ABI versions, once released, are available until such time as their
+deprecation has been noted in the Release Notes for at least one major release
+cycle. For example consider the case where the ABI for DPDK 2.0 has been
+shipped and then a decision is made to modify it during the development of
+DPDK 2.1. The decision will be recorded in the Release Notes for the DPDK 2.1
+release and the modification will be made available in the DPDK 2.2 release.
+
+ABI versions may be deprecated in whole or in part as needed by a given
+update.
+
+Some ABI changes may be too significant to reasonably maintain multiple
+versions. In those cases ABI's may be updated without backward compatibility
+being provided. The requirements for doing so are:
+
+#. At least 3 acknowledgments of the need to do so must be made on the
+   dpdk.org mailing list.
+
+#. A full deprecation cycle, as explained above, must be made to offer
+   downstream consumers sufficient warning of the change.
+
+#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
+   incorporated must be incremented in parallel with the ABI changes
+   themselves.
+
+Note that the above process for ABI deprecation should not be undertaken
+lightly. ABI stability is extremely important for downstream consumers of the
+DPDK, especially when distributed in shared object form. Every effort should
+be made to preserve the ABI whenever possible. The ABI should only be changed
+for significant reasons, such as performance enhancements. ABI breakage due to
+changes such as reorganizing public structure fields for aesthetic or
+readability purposes should be avoided.
+
+Examples of Deprecation Notices
+-------------------------------
+
+The following are some examples of ABI deprecation notices which would be
+added to the Release Notes:
+
+* The Macro ``#RTE_FOO`` is deprecated and will be removed with version 2.0,
+  to be replaced with the inline function ``rte_foo()``.
+
+* The function ``rte_mbuf_grok()`` has been updated to include a new parameter
+  in version 2.0. Backwards compatibility will be maintained for this function
+  until the release of version 2.1
+
+* The members of ``struct rte_foo`` have been reorganized in release 2.0 for
+  performance reasons. Existing binary applications will have backwards
+  compatibility in release 2.0, while newly built binaries will need to
+  reference the new structure variant ``struct rte_foo2``. Compatibility will
+  be removed in release 2.2, and all applications will require updating and
+  rebuilding to the new structure at that time, which will be renamed to the
+  original ``struct rte_foo``.
+
+* Significant ABI changes are planned for the ``librte_dostuff`` library. The
+  upcoming release 2.0 will not contain these changes, but release 2.1 will,
+  and no backwards compatibility is planned due to the extensive nature of
+  these changes. Binaries using this library built prior to version 2.1 will
+  require updating and recompilation.
+
+Versioning Macros
+-----------------
+
+When a symbol is exported from a library to provide an API, it also provides a
+calling convention (ABI) that is embodied in its name, return type and
+arguments. Occasionally that function may need to change to accommodate new
+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 ``lib/librte_compat/rte_compat.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
+library so that older binaries need not be immediately recompiled.
+
+The macros exported are:
+
+* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
+  versioned symbol ``b@DPDK_n`` to the internal function ``b_e``.
+
+* ``BIND_DEFAULT_SYMBOL(b, e, n)``: Creates a symbol version entry instructing
+  the linker to bind references to symbol ``b`` to the internal symbol
+  ``b_e``.
+
+* ``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.
+
+Examples of ABI Macro use
+-------------------------
+
+Updating a public API
+~~~~~~~~~~~~~~~~~~~~~
+
+Assume we have a function as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param)
+ {
+        ...
+ }
+
+
+Assume that struct rte_acl_ctx is a private structure, and that a developer
+wishes to enhance the acl api so that a debugging flag can be enabled on a
+per-context basis.  This requires an addition to the structure (which, being
+private, is safe), but it also requires modifying the code as follows
+
+.. code-block:: c
+
+ /*
+  * Create an acl context object for apps to 
+  * manipulate
+  */
+ struct rte_acl_ctx *
+ rte_acl_create(const struct rte_acl_param *param, int debug)
+ {
+        ...
+ }
+
+
+Note also that, being a public function, the header file prototype must also be
+changed, as must all the call sites, to reflect the new ABI footprint.  We will
+maintain previous ABI versions that are accessible only to previously compiled
+binaries
+
+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
+
+.. code-block:: none 
+
+   DPDK_2.0 {
+        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_2.0 {
+        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_2.1 {
+        global:
+        rte_acl_create;
+
+   } DPDK_2.0;
+
+The addition of the new block tells the linker that a new version node is
+available (DPDK_2.1), which contains the symbol rte_acl_create, and inherits the
+symbols from the DPDK_2.0 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 map 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
+
+.. code-block:: c
+
+  struct rte_acl_ctx *
+ -rte_acl_create(const struct rte_acl_param *param)
+ +rte_acl_create_v20(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 condusive to
+the macros used for versioning symbols.  That is our next step, mapping this new
+symbol name to the initial symbol name at version node 2.0.  Immediately after
+the function, we add this line of code
+
+.. code-block:: c
+
+   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+Remembering to also add the rte_compat.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
+the original rte_acl_create symbol to the original function (but with a new
+name)
+
+Next, we need to create the 2.1 version of the symbol.  We create a new function
+name, with a different suffix, and  implement it appropriately
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   rte_acl_create_v21(const struct rte_acl_param *param, int debug);
+   {
+        struct rte_acl_ctx *ctx = rte_acl_create_v20(param);
+
+        ctx->debug = debug;
+
+        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 symbol
+``rte_acl_create@DPDK_2.1``.  To do this, we modify the public prototype of the call
+in the header file, adding the macro there to inform all including applications,
+that on re-link, the default rte_acl_create symbol should point to this
+function.  Note that we could do this by simply naming the function above
+rte_acl_create, and the linker would chose the most recent version tag to apply
+in the version script, but we can also do this in the header file
+
+.. code-block:: c
+
+   struct rte_acl_ctx *
+   -rte_acl_create(const struct rte_acl_param *param);
+   +rte_acl_create(const struct rte_acl_param *param, int debug);
+   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+The BIND_DEFAULT_SYMBOL macro explicitly tells applications that include this
+header, to link to the rte_acl_create_v21 function and apply the DPDK_2.1
+version node to it.  This method is more explicit and flexible than just
+re-implementing the exact symbol name, and allows for other features (such as
+linking to the old symbol version by default, when the new ABI is to be opt-in
+for a period.
+
+One last thing we need to do.  Note that 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_v21`` is
+defined, we add this
+
+.. code-block:: c
+
+   struct rte_acl_create_v21(const struct rte_acl_param *param, int debug)
+   {
+        ...
+   }
+   MAP_STATIC_SYMBOL(struct rte_acl_create(const struct rte_acl_param *param, int debug), rte_acl_create_v21);
+
+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_v21``
+
+That's it, on the next shared library rebuild, there will be two versions of
+rte_acl_create, an old DPDK_2.0 version, used by previously built applications,
+and a new DPDK_2.1 version, used by future built applications.
+
+
+Deprecating part of a public API
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Lets assume that you've done the above update, and after a few releases have
+passed you decide you would like to retire the old version of the function.
+After having gone through the ABI deprecation announcement process, removal is
+easy.  Start by removing the symbol from the requisite version map file:
+
+.. code-block:: none
+
+   DPDK_2.0 {
+        global:
+
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+ -      rte_acl_create
+        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_2.1 {
+        global:
+        rte_acl_create;
+   } DPDK_2.0;
+
+
+Next remove the corresponding versioned export
+.. code-block:: c
+
+ -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
+
+
+Note that the internal function definition could also be removed, but its used
+in our example by the newer version _v21, so we leave it in place.  This is a
+coding style choice.
+
+Lastly, we need to bump the LIBABIVER number for this library in the Makefile to
+indicate to applications doing dynamic linking that this is a later, and
+possibly incompatible library version:
+
+.. code-block:: c
+
+   -LIBABIVER := 1
+   +LIBABIVER := 2
+
+Deprecating an entire ABI version
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+While removing a symbol from and ABI may be useful, it is often more practical
+to remove an entire version node at once.  If a version node completely
+specifies an API, then removing part of it, typically makes it incomplete.  In
+those cases it is better to remove the entire node
+ 
+To do this, start by modifying the version map file, such that all symbols from
+the node to be removed are merged into the next node in the map
+
+In the case of our map above, it would transform to look as follows
+
+.. code-block:: none
+
+   DPDK_2.1 {              
+        global:
+              
+        rte_acl_add_rules;
+        rte_acl_build;
+        rte_acl_classify;
+        rte_acl_classify_alg;
+        rte_acl_classify_scalar;
+        rte_acl_dump;
+        rte_acl_create
+        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: *;
+ };           
+
+Then any uses of BIND_DEFAULT_SYMBOL that pointed to the old node should be
+updated to point to the new version node in any header files for all affected
+symbols.
+
+.. code-block:: c
+
+ -BIND_DEFAULT_SYMBOL(rte_acl_create, _v20, 2.0);
+ +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);
+
+Lastly, any VERSION_SYMBOL macros that point to the old version node should be
+removed, taking care to keep, where need old code in place to support newer
+versions of the symbol.
+
+Running the ABI Validator
+-------------------------
+
+The ``scripts`` directory in the DPDK source tree contains a utility program,
+``validate-abi.sh``, for validating the DPDK ABI based on the Linux `ABI
+Compliance Checker
+<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_.
+
+This has a dependency on the ``abi-compliance-checker`` and ``and abi-dumper``
+utilities which can be installed via a package manager. For example::
+
+   sudo yum install abi-compliance-checker
+   sudo yum install abi-dumper
+
+The syntax of the ``validate-abi.sh`` utility is::
+
+   ./scripts/validate-abi.sh <TAG1> <TAG2> <TARGET>
+
+Where ``TAG1`` and ``TAG2`` are valid git tags on the local repo and target is
+the usual DPDK compilation target.
+
+For example to test the current committed HEAD against a previous release tag
+we could add a temporary tag and run the utility as follows::
+
+   git tag MY_TEMP_TAG
+   ./scripts/validate-abi.sh v2.0.0 MY_TEMP_TAG x86_64-native-linuxapp-gcc
+
+After the validation script completes (it can take a while since it need to
+compile both tags) it will create compatibility reports in the
+``./compat_report`` directory. Listed incompatibilities can be found as
+follows::
+
+  grep -lr Incompatible compat_reports/
diff --git a/doc/guides/rel_notes/abi.rst b/doc/guides/rel_notes/abi.rst
index f00a6ee..4086198 100644
--- a/doc/guides/rel_notes/abi.rst
+++ b/doc/guides/rel_notes/abi.rst
@@ -1,33 +1,7 @@
 ABI policy
 ==========
-ABI versions are set at the time of major release labeling, and ABI may change
-multiple times between the last labeling and the HEAD label of the git tree
-without warning.
-
-ABI versions, once released are available until such time as their
-deprecation has been noted here for at least one major release cycle, after it
-has been tagged.  E.g. the ABI for DPDK 2.0 is shipped, and then the decision to
-remove it is made during the development of DPDK 2.1.  The decision will be
-recorded here, shipped with the DPDK 2.1 release, and actually removed when DPDK
-2.2 ships.
-
-ABI versions may be deprecated in whole, or in part as needed by a given update.
-
-Some ABI changes may be too significant to reasonably maintain multiple
-versions of.  In those events ABI's may be updated without backward
-compatibility provided.  The requirements for doing so are:
-
-#. At least 3 acknowledgments of the need on the dpdk.org
-#. A full deprecation cycle must be made to offer downstream consumers sufficient warning of the change.  E.g. if dpdk 2.0 is under development when the change is proposed, a deprecation notice must be added to this file, and released with dpdk 2.0.  Then the change may be incorporated for dpdk 2.1
-#. The LIBABIVER variable in the makefile(s) where the ABI changes are incorporated must be incremented in parallel with the ABI changes themselves
-
-Note that the above process for ABI deprecation should not be undertaken
-lightly.  ABI stability is extremely important for downstream consumers of the
-DPDK, especially when distributed in shared object form.  Every effort should be
-made to preserve ABI whenever possible.  For instance, reorganizing public
-structure field for aesthetic or readability purposes should be avoided as it will
-cause ABI breakage.  Only significant (e.g. performance) reasons should be seen
-as cause to alter ABI.
+See the guidelines document for details of the ABI policy.  ABI deprecation
+notices are to be posted here
 
 Examples of Deprecation Notices
 -------------------------------
-- 
2.1.0

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv4 4/4] ABI: Add some documentation
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 4/4] ABI: Add some documentation Neil Horman
@ 2015-06-29 15:07     ` Thomas Monjalon
  0 siblings, 0 replies; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-29 15:07 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-06-29 09:59, Neil Horman:
> People have been asking for ways to use the ABI macros, heres some docs to
> clarify their use.  Included is:
> 
> * An overview of what ABI is
> * Details of the ABI deprecation process
> * Details of the versioning macros
> * Examples of their use
> * Details of how to use the ABI validator
> 
> Thanks to John Mcnamara, who duplicated much of this effort at Intel while I was
> working on it.  Much of the introductory material was gathered and cleaned up by
> him
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: john.mcnamara@intel.com
> CC: thomas.monjalon@6wind.com
> 
> Change notes:
> 
> v2)
>      * Fixed RST indentations and spelling errors
>      * Rebased to upstream to fix index.rst conflict
> 
> v3)
>      * Fixed in tact -> intact
>      * Added docs to address static linking
>      * Removed duplicate documentation from release notes
> 
> v4)
>      * Fixed more grammatical errors / etc
>      * removed BASE_SYMBOL from documentation

Series applied with minor fixes, thanks

This doc should help to produce patches with new ABI without breaking
the old one.
For cases where the backward compatibility is impossible to achieve,
a patch amending this doc to suggest a NEXT_ABI option can be discussed.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [dpdk-dev] [PATCH] lib: remove redundant definition of local symbols
  2015-06-25 12:25       ` Neil Horman
@ 2015-06-29 16:35         ` Thomas Monjalon
  2015-06-30 15:50           ` Thomas Monjalon
  0 siblings, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-29 16:35 UTC (permalink / raw)
  To: nhorman; +Cc: dev

The new version nodes inherit from the previous ones which
already include a default catch-all line for not exported symbols.

Reported-by: Helin Zhang <helin.zhang@intel.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_cmdline/rte_cmdline_version.map | 1 -
 lib/librte_ether/rte_ether_version.map     | 1 -
 lib/librte_mbuf/rte_mbuf_version.map       | 1 -
 3 files changed, 3 deletions(-)

diff --git a/lib/librte_cmdline/rte_cmdline_version.map b/lib/librte_cmdline/rte_cmdline_version.map
index 1b0c863..c9fc18a 100644
--- a/lib/librte_cmdline/rte_cmdline_version.map
+++ b/lib/librte_cmdline/rte_cmdline_version.map
@@ -75,5 +75,4 @@ DPDK_2.1 {
 
 	cmdline_poll;
 
-	local: *;
 } DPDK_2.0;
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 012a82e..e3c8fa1 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -111,5 +111,4 @@ DPDK_2.1 {
 
 	rte_eth_dev_set_mc_addr_list;
 
-	local: *;
 } DPDK_2.0;
diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map
index 543dc4c..e10f6bd 100644
--- a/lib/librte_mbuf/rte_mbuf_version.map
+++ b/lib/librte_mbuf/rte_mbuf_version.map
@@ -17,5 +17,4 @@ DPDK_2.1 {
 
 	rte_pktmbuf_pool_create;
 
-	local: *;
 } DPDK_2.0;
-- 
2.4.2

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCH] lib: remove redundant definition of local symbols
  2015-06-29 16:35         ` [dpdk-dev] [PATCH] lib: remove redundant definition of local symbols Thomas Monjalon
@ 2015-06-30 15:50           ` Thomas Monjalon
  0 siblings, 0 replies; 41+ messages in thread
From: Thomas Monjalon @ 2015-06-30 15:50 UTC (permalink / raw)
  To: dev

2015-06-29 18:35, Thomas Monjalon:
> The new version nodes inherit from the previous ones which
> already include a default catch-all line for not exported symbols.
> 
> Reported-by: Helin Zhang <helin.zhang@intel.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos
  2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
                     ` (2 preceding siblings ...)
  2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 4/4] ABI: Add some documentation Neil Horman
@ 2015-07-08  9:52   ` Thomas Monjalon
  2015-07-08 11:04     ` Neil Horman
  3 siblings, 1 reply; 41+ messages in thread
From: Thomas Monjalon @ 2015-07-08  9:52 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

Hi Neil,

2015-06-29 09:59, Neil Horman:
> - *	int __vsym foo_v20(char *string)
> + *	int foo_v20(char *string)
[...]
> -#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@@DPDK_"RTE_STR(n))
> +#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))

In this patch, you removed __vsym from the example but the #define still exists.
Should it be removed? or documented?

Thanks

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos
  2015-07-08  9:52   ` [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos Thomas Monjalon
@ 2015-07-08 11:04     ` Neil Horman
  0 siblings, 0 replies; 41+ messages in thread
From: Neil Horman @ 2015-07-08 11:04 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Wed, Jul 08, 2015 at 11:52:51AM +0200, Thomas Monjalon wrote:
> Hi Neil,
> 
> 2015-06-29 09:59, Neil Horman:
> > - *	int __vsym foo_v20(char *string)
> > + *	int foo_v20(char *string)
> [...]
> > -#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@@DPDK_"RTE_STR(n))
> > +#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))
> 
> In this patch, you removed __vsym from the example but the #define still exists.
> Should it be removed? or documented?
> 
> Thanks
> 

The latter, I'll try send a follow on patch today
Neil

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2015-07-08 11:04 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
2015-06-23 19:33 ` [dpdk-dev] [PATCH 2/2] ABI: Add some documentation Neil Horman
2015-06-24 11:21   ` Mcnamara, John
2015-06-24 11:23 ` [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Mcnamara, John
2015-06-24 18:06   ` Neil Horman
2015-06-24 18:34 ` [dpdk-dev] [PATCHv2 " Neil Horman
2015-06-24 18:34   ` [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation Neil Horman
2015-06-24 21:09     ` Thomas Monjalon
2015-06-25 11:35       ` Neil Horman
2015-06-25 13:22         ` Thomas Monjalon
2015-06-25  7:19     ` Zhang, Helin
2015-06-25  7:42       ` Gonzalez Monroy, Sergio
2015-06-25  8:00         ` Gonzalez Monroy, Sergio
2015-06-25 12:25       ` Neil Horman
2015-06-29 16:35         ` [dpdk-dev] [PATCH] lib: remove redundant definition of local symbols Thomas Monjalon
2015-06-30 15:50           ` Thomas Monjalon
2015-06-24 19:41   ` [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos Thomas Monjalon
2015-06-24 20:15     ` Neil Horman
2015-06-24 20:49       ` Thomas Monjalon
2015-06-25  7:37 ` [dpdk-dev] [PATCH " Gajdzica, MaciejX T
2015-06-25 12:28   ` Neil Horman
2015-06-25 14:35 ` [dpdk-dev] [PATCHv3 1/3] " Neil Horman
2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
2015-06-26 10:13     ` Gajdzica, MaciejX T
2015-06-26 12:52     ` Thomas Monjalon
2015-06-26 14:30       ` Neil Horman
2015-06-28 20:13         ` Thomas Monjalon
2015-06-29 13:44           ` Neil Horman
2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation Neil Horman
2015-06-26 13:00     ` Thomas Monjalon
2015-06-26 14:54       ` Neil Horman
2015-06-28 20:24         ` Thomas Monjalon
2015-06-29 13:53           ` Neil Horman
2015-06-26 12:45   ` [dpdk-dev] [PATCHv3 1/3] rte_compat.h : Clean up some typos Thomas Monjalon
2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 2/4] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 3/4] rte_compat: remove BASE_SYMBOL Neil Horman
2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 4/4] ABI: Add some documentation Neil Horman
2015-06-29 15:07     ` Thomas Monjalon
2015-07-08  9:52   ` [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos Thomas Monjalon
2015-07-08 11:04     ` Neil Horman

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).