[dpdk-dev] [PATCH v3 2/4] doc: changes to abi policy introducing major abi versions

[Ray Kinsella]

Thanks Kevin for working through all this.
Other comments are inline.

On 30/08/2019 17:20, Kevin Traynor wrote:
> Hi Ray,
> 
> On 15/08/2019 11:23, Ray Kinsella wrote:
>> This policy change introduces major ABI versions, these are
>> declared every year, typically aligned with the LTS release
>> and are supported by subsequent releases in the following year.
>> This change is intended to improve ABI stabilty for those projects
>> consuming DPDK.
>>
>> Signed-off-by: Ray Kinsella <mdr at ashroe.eu>
>> ---
>>  doc/guides/contributing/abi_policy.rst | 308 ++++++++++++++++++++++++---------
>>  doc/guides/contributing/stable.rst     |  38 ++--
>>  2 files changed, 245 insertions(+), 101 deletions(-)
>>
>> diff --git a/doc/guides/contributing/abi_policy.rst b/doc/guides/contributing/abi_policy.rst
>> index 55bacb4..6190bdc 100644
>> --- a/doc/guides/contributing/abi_policy.rst
>> +++ b/doc/guides/contributing/abi_policy.rst
>> @@ -1,33 +1,46 @@
>>  ..  SPDX-License-Identifier: BSD-3-Clause
>> -    Copyright 2018 The DPDK contributors
>> +    Copyright 2019 The DPDK contributors
>>  
>> -.. abi_api_policy:
>> +.. _abi_policy:
>>  
>> -DPDK ABI/API policy
>> -===================
>> +ABI Policy
>> +==========
>>  
>>  Description
>>  -----------
>>  
>> -This document details some methods for handling ABI management in the DPDK.
>> +This document details the management policy that ensures the long-term stability
>> +of the DPDK ABI and API.
>>  
>>  General Guidelines
>>  ------------------
>>  
>> -#. Whenever possible, ABI should be preserved
>> -#. ABI/API may be changed with a deprecation process
>> -#. The modification of symbols can generally be managed with versioning
>> -#. Libraries or APIs marked in ``experimental`` state may change without constraint
>> -#. New APIs will be marked as ``experimental`` for at least one release to allow
>> -   any issues found by users of the new API to be fixed quickly
>> -#. The addition of symbols is generally not problematic
>> -#. The removal of symbols generally is an ABI break and requires bumping of the
>> -   LIBABIVER macro
>> -#. Updates to the minimum hardware requirements, which drop support for hardware which
>> -   was previously supported, should be treated as an ABI change.
>> -
>> -What is an ABI
>> -~~~~~~~~~~~~~~
>> +#. Major ABI versions are declared every **year** and are then supported for one
>> +   year, typically aligned with the :ref:`LTS release <stable_lts_releases>`.
>> +#. The ABI version is managed at a project level in DPDK, with the ABI version
>> +   reflected in all :ref:`library's soname <what_is_soname>`.
>> +#. The ABI should be preserved and not changed lightly. ABI changes must follow
>> +   the outlined :ref:`deprecation process <abi_changes>`.
>> +#. The addition of symbols is generally not problematic. The modification of
>> +   symbols is managed with :ref:`ABI Versioning <abi_versioning>`.
>> +#. The removal of symbols is considered an :ref:`ABI breakage <abi_breakages>`,
>> +   once approved these will form part of the next ABI version.
>> +#. Libraries or APIs marked as :ref:`Experimental <experimental_apis>` are not
>> +   considered part of an ABI version and may change without constraint.
>> +#. Updates to the :ref:`minimum hardware requirements <hw_rqmts>`, which drop
>> +   support for hardware which was previously supported, should be treated as an
>> +   ABI change.
>> +
>> +.. note::
>> +
>> +   In 2019, the DPDK community stated it's intention to move to ABI stable
>> +   releases, over a number of release cycles. Beginning with maintaining ABI
>> +   stability through one year of DPDK releases starting from DPDK 19.11. This
>> +   policy will be reviewed in 2020, with intention of lengthening the stability
>> +   period.
>> +
>> +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
>> @@ -39,30 +52,67 @@ 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.
>>  
>> +What is an ABI version?
>> +~~~~~~~~~~~~~~~~~~~~~~~
>>  
>> -ABI/API Deprecation
>> --------------------
>> +An ABI version is an instance of a library's ABI at a specific release. Certain
>> +releases are considered by the community to be milestone releases, the yearly
>> +LTS for example. Supporting those milestone release's ABI for some number of
>> +subsequent releases is desirable to facilitate application upgrade. Those ABI
>> +version's aligned with milestones release are therefore called 'ABI major
>> +versions' and are supported for some number of releases.
>> +
>> +More details on major ABI version can be found in the :ref:`ABI versioning
>> +<major_abi_versions>` guide.
>>  
>>  The DPDK ABI policy
>> -~~~~~~~~~~~~~~~~~~~
>> +-------------------
>> +
>> +A major ABI version is declared every year, aligned with that year's LTS
>> +release, e.g. v19.11. This ABI version is then supported for one year by all
>> +subsequent releases within that time period, until the next LTS release, e.g.
>> +v20.11.
>> +
>> +At the declaration of a major ABI version, major version numbers encoded in
>> +libraries soname's are bumped to indicate the new version, with the minor
>> +version reset to ``0``. An example would be ``librte_eal.so.20.3`` would become
>> +``librte_eal.so.21.0``.
>> +
>> +The ABI may then change multiple times, without warning, between the last major
>> +ABI version increment and the HEAD label of the git tree, with the condition
>> +that ABI compatibility with the major ABI version is preserved and therefore
>> +soname's do not change.
>> +
>> +Minor versions are incremented to indicate the release of a new ABI compatible
>> +DPDK release, typically the DPDK quarterly releases. An example of this, might
>> +be that ``librte_eal.so.20.1`` would indicate the first ABI compatible DPDK
>> +release, following the declaration of the new major ABI version ``20``.
>> +
>> +ABI versions, are supported by each release until such time as the next major
>> +ABI version is declared. At that time, the deprecation of the previous major ABI
>> +version will be noted in the Release Notes with guidance on individual symbol
>> +depreciation and upgrade notes provided.
>>  
>> -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_changes:
>>  
>> -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 Changes
>> +~~~~~~~~~~~
>>  
>> -ABI versions may be deprecated in whole or in part as needed by a given
>> -update.
>> +The ABI may still change after the declaration of a major ABI version, that is
>> +new APIs may be still added or existing APIs may be modified.
>>  
>> -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:
>> +.. Warning::
>> +
>> +   Note that, the 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.
>> +
> 
> This text is not changed and it reads like *any* performance enhancement
> is a good enough reason for an ABI break. Can't obviously quantify it,
> but maybe "major performance enhancement" is closer to the intended
> tone? Sorry for nit-picking over one word!

I agree, I was in two minds about whether to clarify this section or if
it was fine as-is. I left it there as a general warning to stop and
think before you ask to change the ABI. A performance gain alone doesn't
absolve the contributor from an obligation to preserve ABI compatibility.

Perhaps reword as follows?

.. Warning::

   Note that, this policy details the method by which the ABI may be
changed, with due regard to preserving compatibility and observing
depreciation notices. This process however should not be undertaken
lightly, as a general rule ABI stability is extremely important for
downstream consumers of DPDK. The ABI should only be changed for
significant reasons, such as performance enhancements. ABI breakages due
to changes such as reorganizing public structure fields for aesthetic or
readability purposes should be avoided.

> 
>> +
>> +The requirements for changing the ABI are:
>>  
>>  #. At least 3 acknowledgments of the need to do so must be made on the
>>     dpdk.org mailing list.
>> @@ -71,34 +121,119 @@ being provided. The requirements for doing so are:
>>       no maintainer is available for the component, the tree/sub-tree maintainer
>>       for that component must acknowledge the ABI change instead.
>>  
>> +   - The acknowledgment of a member of the technical board, as a delegate of the
>> +     `technical board <https://core.dpdk.org/techboard/>`_ acknowledging the
>> +     need for the ABI change, is also mandatory.
>> +
>>     - It is also recommended that acknowledgments from different "areas of
>>       interest" be sought for each deprecation, for example: from NIC vendors,
>>       CPU vendors, end-users, etc.
>>  
>> -#. The changes (including an alternative map file) can be included with
>> -   deprecation notice, in wrapped way by the ``RTE_NEXT_ABI`` option,
>> -   to provide more details about oncoming changes.
>> -   ``RTE_NEXT_ABI`` wrapper will be removed when it become the default ABI.
>> -   More preferred way to provide this information is sending the feature
>> -   as a separate patch and reference it in deprecation notice.
>> +#. Backward compatibly with the major ABI version must be maintained through
> 
> s/compatibly/compatibility/

ACK

> 
>> +   :ref:`abi_versioning`, with :ref:`forward-only <forward-only>` compatibility
>> +   offered for any ABI changes that are indicated to be part of the next ABI
>> +   version.
>>  
>> -#. A full deprecation cycle, as explained above, must be made to offer
>> -   downstream consumers sufficient warning of the change.
>> +   - In situations were backward compatibility is not possible, read the
> 
> s/were/where/

ACK

> 
>> +     section on :ref:`abi_breakages`.
>>  
>> -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.
>> +   - No backward or forward compatibility is offered for API changes marked as
>> +     ``experimental``, as described in the section on :ref:`Experimental APIs
>> +     and Libraries <experimental_apis>`.
>>  
>> -.. note::
>> +#. If a newly proposed API functionally replaces an existing one, when the new
>> +   API becomes non-experimental, then the old one is marked with
>> +   ``__rte_deprecated``.
>> +
>> +    - The depreciated API should follow the notification process to be removed,
>> +      see  :ref:`deprecation_notices`.
>> +
>> +    - At the declaration of the next major ABI version, those ABI changes then
>> +      become a formal part of the new ABI and the requirement to preserve ABI
>> +      compatibility with the last major ABI version is then dropped.
>> +
>> +    - The responsibility for removing redundant ABI compatibility code rests
>> +      with the original contributor of the ABI changes, failing that, then with
>> +      the contributor's company and then finally with the maintainer.
>> +
>> +.. _forward-only:
>> +
>> +.. Note::
>> +
>> +   Note that forward-only compatibility is offered for those changes made
>> +   between major ABI versions. As a library's soname can only describe
>> +   compatibility with the last major ABI version, until the next major ABI
>> +   version is declared, these changes therefore cannot be resolved as a runtime
>> +   dependency through the soname. Therefore any application wishing to make use
>> +   of these ABI changes can only ensure that it's runtime dependencies are met
>> +   through Operating System package versioning.
>> +
>> +.. _hw_rqmts:
>> +
>> +.. Note::
>>  
>>     Updates to the minimum hardware requirements, which drop support for hardware
>>     which was previously supported, should be treated as an ABI change, and
>> -   follow the relevant deprecation policy procedures as above: 3 acks and
>> -   announcement at least one release in advance.
>> +   follow the relevant deprecation policy procedures as above: 3 acks, technical
>> +   board approval and announcement at least one release in advance.
>> +
>> +.. _abi_breakages:
>> +
>> +ABI Breakages
>> +~~~~~~~~~~~~~
>> +
>> +For those ABI changes that are too significant to reasonably maintain multiple
>> +symbol versions, there is an amended process. In these cases, ABIs may be
>> +updated without the requirement of backward compatibility being provided. These
>> +changes must follow the `same process :ref:`described above <abi_changes>` as non-breaking
>> +changes, however with the following additional requirements:
>> +
>> +#. ABI breaking changes (including an alternative map file) can be included with
>> +   deprecation notice, in wrapped way by the ``RTE_NEXT_ABI`` option, to provide
>> +   more details about oncoming changes. ``RTE_NEXT_ABI`` wrapper will be removed
>> +   at the declaration of the next major ABI version.
>> +
>> +#. Once approved, and after the depreciation notice has been observed these
>> +   changes will form part of the next declared major ABI version.
>> +
>> +Examples of ABI Changes
>> +~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +The following are examples of allowable ABI changes occurring between
>> +declarations of major ABI versions.
>> +
>> +* DPDK 19.11 release, defines the function ``rte_foo()``, and ``rte_foo()``
>> +  as part of the major ABI version ``20``.
>> +
>> +* DPDK 20.02 release defines a new function ``rte_foo(uint8_t bar)``, and
>> +  this is not a problem as long as the symbol ``rte_foo at DPDK20`` is
>> +  preserved through :ref:`abi_versioning`.
>> +
>> +  - The new function may be marked with the ``__rte_experimental`` tag for a
>> +    number of releases, as described in the section :ref:`experimental_apis`.
>> +
>> +  - Once ``rte_foo(uint8_t bar)`` becomes non-experimental ``rte_foo()`` is then
>> +    declared as ``__rte_depreciated``, with an associated deprecation notice
>> +    provided.
>> +
>> +* DPDK 19.11 is not re-released to include ``rte_foo(uint8_t bar)``, the new
>> +  version of ``rte_foo`` only exists from DPDK 20.02 onwards as described in the
>> +  :ref:`note on forward-only compatibility<forward-only>`.
>> +
>> +* DPDK 20.02 release defines the experimental function ``__rte_experimental
>> +  rte_baz()``. This function may or may not exist in the DPDK 20.05 release.
>> +
>> +* An application ``dPacket`` wishes to use ``rte_foo(uint8_t bar)``, before the
>> +  declaration of the DPDK ``21`` major API version. The application can only
>> +  ensure it's runtime dependencies are met by specifying ``DPDK (>= 20.2)`` as
>> +  an explicit package dependency, as the soname only may only indicate the
>> +  supported major ABI version.
>> +
>> +* At the release of DPDK 20.11, the function ``rte_foo(uint8_t bar)`` becomes
>> +  formally part of then new major ABI version DPDK 21.0 and ``rte_foo()`` may be
>> +  removed.
>> +
>> +.. _deprecation_notices:
>>  
>>  Examples of Deprecation Notices
>>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> @@ -106,46 +241,42 @@ 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 Macro ``#RTE_FOO`` is deprecated and will be removed with ABI version
>> +  21, 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
>> +  in version 20.2. Backwards compatibility will be maintained for this function
>> +  until the release of the new DPDK major ABI version 21, in DPDK version
>> +  20.11.
>>  
>> -* The members of ``struct rte_foo`` have been reorganized in release 2.0 for
>> +* The members of ``struct rte_foo`` have been reorganized in DPDK 20.02 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
>> +  compatibility in release 20.02, while newly built binaries will need to
>> +  reference the new structure variant ``struct rte_foo2``. Compatibility will be
>> +  removed in release 20.11, 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,
>> +  upcoming release 20.02 will not contain these changes, but release 20.11 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
>> +  these changes. Binaries using this library built prior to ABI version 21 will
>>    require updating and recompilation.
>>  
>> -New API replacing previous one
>> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> -
>> -If a new API proposed functionally replaces an existing one, when the new API
>> -becomes non-experimental then the old one is marked with ``__rte_deprecated``.
>> -Deprecated APIs are removed completely just after the next LTS.
>> -
>> -Reminder that old API should follow deprecation process to be removed.
>> +.. _experimental_apis:
>>  
>> +Experimental
>> +------------
>>  
>> -Experimental APIs
>> ------------------
>> +APIs
>> +~~~~
>>  
>> -APIs marked as ``experimental`` are not considered part of the ABI and may
>> -change without warning at any time.  Since changes to APIs are most likely
>> -immediately after their introduction, as users begin to take advantage of
>> -those new APIs and start finding issues with them, new DPDK APIs will be
>> -automatically marked as ``experimental`` to allow for a period of stabilization
>> -before they become part of a tracked ABI.
>> +APIs marked as ``experimental`` are not considered part of an ABI version and
>> +may change without warning at any time. Since changes to APIs are most likely
>> +immediately after their introduction, as users begin to take advantage of those
>> +new APIs and start finding issues with them, new DPDK APIs will be automatically
>> +marked as ``experimental`` to allow for a period of stabilization before they
>> +become part of a tracked ABI version.
>>  
>>  Note that marking an API as experimental is a multi step process.
>>  To mark an API as experimental, the symbols which are desired to be exported
>> @@ -163,7 +294,16 @@ In addition to tagging the code with ``__rte_experimental``,
>>  the doxygen markup must also contain the EXPERIMENTAL string,
>>  and the MAINTAINERS file should note the EXPERIMENTAL libraries.
>>  
>> -For removing the experimental tag associated with an API, deprecation notice
>> -is not required. Though, an API should remain in experimental state for at least
>> -one release. Thereafter, normal process of posting patch for review to mailing
>> -list can be followed.
>> +For removing the experimental tag associated with an API, deprecation notice is
>> +not required. Though, an API should remain in experimental state for at least
>> +one release. Thereafter, the normal process of posting patch for review to
>> +mailing list can be followed.
>> +
>> +Libraries
>> +~~~~~~~~~
>> +
>> +Libraries marked as ``experimental`` are entirely not considered part of an ABI
>> +version, and may change without warning at any time. Experimental libraries
>> +always have a major version of ``0`` to indicate they exist outside of
>> +:ref:`abi_versioning` , with the minor version incremented with each ABI change
>> +to library.
>> diff --git a/doc/guides/contributing/stable.rst b/doc/guides/contributing/stable.rst
>> index 6a5eee9..d95c200 100644
>> --- a/doc/guides/contributing/stable.rst
>> +++ b/doc/guides/contributing/stable.rst
>> @@ -1,7 +1,7 @@
>>  ..  SPDX-License-Identifier: BSD-3-Clause
>>      Copyright 2018 The DPDK contributors
>>  
>> -.. stable_lts_releases:
>> +.. _stable_lts_releases:
>>  
>>  DPDK Stable Releases and Long Term Support
>>  ==========================================
>> @@ -53,6 +53,9 @@ year's November (X.11) release will be maintained as an LTS for 2 years.
>>  After the X.11 release, an LTS branch will be created for it at
>>  http://git.dpdk.org/dpdk-stable where bugfixes will be backported to.
>>  
>> +A LTS release may align with the declaration of a new major ABI version,
>> +please read the :ref:`abi_policy` for more information.
>> +
> 
> Above is worth to mention, but as discussed on call earlier today, the
> changes below should be dropped from this patchset. At present each LTS
> minor release (e.g. 18.11.2) maintains the API/ABI of the original LTS
> release (e.g. 18.11) and that is not changing.

ACK, I will remove

> 
> What type of non-ABI breaking things are backported to LTS branches can
> be discussed during the LTS presentation in DPDK userspace.

Do you anticipate any updates here?

Thanks,

Ray K

> 
> thanks,
> Kevin.
> 
>>  It is anticipated that there will be at least 4 releases per year of the LTS
>>  or approximately 1 every 3 months. However, the cadence can be shorter or
>>  longer depending on the number and criticality of the backported
>> @@ -68,10 +71,13 @@ point the LTS branch will no longer be maintained with no further releases.
>>  What changes should be backported
>>  ---------------------------------
>>  
>> -Backporting should be limited to bug fixes. All patches accepted on the master
>> -branch with a Fixes: tag should be backported to the relevant stable/LTS
>> -branches, unless the submitter indicates otherwise. If there are exceptions,
>> -they will be discussed on the mailing lists.
>> +Backporting is a naturally conservative activity, and therefore should only
>> +include bug fixes and support for new hardware, were adding support does not
>> +necessitate DPDK ABI/API changes.
>> +
>> +All patches accepted on the master branch with a Fixes: tag should be backported
>> +to the relevant stable/LTS branches, unless the submitter indicates otherwise.
>> +If there are exceptions, they will be discussed on the mailing lists.
>>  
>>  Fixes suitable for backport should have a ``Cc: stable at dpdk.org`` tag in the
>>  commit message body as follows::
>> @@ -86,13 +92,18 @@ commit message body as follows::
>>       Signed-off-by: Alex Smith <alex.smith at example.com>
>>  
>>  
>> -Fixes not suitable for backport should not include the ``Cc: stable at dpdk.org`` tag.
>> +Fixes not suitable for backport should not include the ``Cc: stable at dpdk.org``
>> +tag.
>>  
>> -Features should not be backported to stable releases. It may be acceptable, in
>> -limited cases, to back port features for the LTS release where:
>> +New features, with the exception of new hardware support, should not be
>> +backported to stable releases. In the case of new hardware support or any other
>> +exceptional circumstances limited backporting maybe permitted to the LTS release
>> +where:
>>  
>> -* There is a justifiable use case (for example a new PMD).
>> -* The change is non-invasive.
>> +* There is a justifiable use case, for example the change is required to support
>> +  a new platform or device (for example a new PMD).
>> +* The change is ABI/API preserving, it does not present an obvious "new feature"
>> +  to end consumer.
>>  * The work of preparing the backport is done by the proposer.
>>  * There is support within the community.
>>  
>> @@ -119,10 +130,3 @@ A Stable Release will be released by:
>>    list.
>>  
>>  Stable releases are available on the `dpdk.org download page <http://core.dpdk.org/download/>`_.
>> -
>> -
>> -ABI
>> ----
>> -
>> -The Stable Release should not be seen as a way of breaking or circumventing
>> -the DPDK ABI policy.
>>