From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7A0ABA0093 for ; Fri, 22 May 2020 11:40:48 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 70C4C1D96D; Fri, 22 May 2020 11:40:48 +0200 (CEST) Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by dpdk.org (Postfix) with ESMTP id 422831D966 for ; Fri, 22 May 2020 11:40:47 +0200 (CEST) Received: by mail-wr1-f47.google.com with SMTP id e1so9509096wrt.5 for ; Fri, 22 May 2020 02:40:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ICf2q6WvEkmlwv+SaDJptylcfAVK5Tn1Y2QfvGIW/mI=; b=OzZUWppS7JE7bUlfzIVSC5vj1aUWa0XWBXstPPbjDZa2O40Rm4r8UyyIKehxfBIusE tr+GC2igoQ1ZX0jtg1pv2D/VlMods98fjhtklA7xRxygC9g5whYzYFK2ZNhLQD27lIgP 8o03LZoF+IdeqSVAM8Vkmn8Lc64pWNShxg021zbLrf6JEj9DLCtw9MZ5uJfWVQ3uVrpW J5jWKsr8/Vf8Umeih5ENwUAWudaHWepPE/u9KzIRq4x07fbS3R8vDI19vA8Y3CeO4O/2 BZVK1uyvcvxZV0T/dlB9CYKt8pkBYHw6TKmlREtumCgPEodCRSaEKfNKNw0VljS7L84Z WSVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ICf2q6WvEkmlwv+SaDJptylcfAVK5Tn1Y2QfvGIW/mI=; b=HPmEF+3oxNXFwK2ikYmqfXMC36Ae9RHe4qpKopv8GgcXpEmOlLUcdhne7khUUC/4RU XpuAazpNFHxwI5Ep5cQlRcoLsT7J5benh3BMKPo+5UdsVQ/hNbURsOxEcxrS71MaSmj4 HXNSN1SAV9+mhcnIzoN6Gfe55t5aKqk5XAij1U8KEFEnDNNcVnEfyfc+PefRUhJx4//u oqWni1B2wTcg6vBrslEAFhpNCQmjkoXNSqFifTzZpT0DKsy5REdXhTbVMn/mQiWUnXcj ZLwkUF+FW9h7PApVriDSWmNm0/2gxMjUG4zqT1j7nCqndZj7EQGKXNJWFffAn0Hdy9vr GJLQ== X-Gm-Message-State: AOAM530YiYwmyYV+8SFPHEhVFsZH9uIUj4q5T7f4XPaYfXh6yqH4fJG3 geCZH/nFdn/sC2l4nimKWlrF1F8vvM8= X-Google-Smtp-Source: ABdhPJwn8y4JD4co4jYY5DKeHYPxIlXbP7yd99+F7/LA+pZud1HOPMA4nDYRx6IAfuHY8NBZEmh5Hw== X-Received: by 2002:a5d:5047:: with SMTP id h7mr2663699wrt.7.1590140446930; Fri, 22 May 2020 02:40:46 -0700 (PDT) Received: from localhost ([88.98.246.218]) by smtp.gmail.com with ESMTPSA id u23sm9515905wmu.20.2020.05.22.02.40.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 May 2020 02:40:46 -0700 (PDT) From: luca.boccassi@gmail.com To: Ray Kinsella Cc: dpdk stable Date: Fri, 22 May 2020 10:39:46 +0100 Message-Id: <20200522094022.1025696-7-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200522094022.1025696-1-luca.boccassi@gmail.com> References: <20200519130549.112823-214-luca.boccassi@gmail.com> <20200522094022.1025696-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'doc: fix default symbol binding in ABI guide' has been queued to stable release 19.11.3 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, FYI, your patch has been queued to stable release 19.11.3 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 05/24/20. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Thanks. Luca Boccassi --- >From 2d2ba1fb005f8585606be28276a3ff653cd3c69d Mon Sep 17 00:00:00 2001 From: Ray Kinsella Date: Wed, 6 May 2020 16:41:05 +0100 Subject: [PATCH] doc: fix default symbol binding in ABI guide [ upstream commit 45a4103e680d6b9bfb2b4ee4f4ef528d8de51ec0 ] The document abi_versioning.rst incorrectly instructs the developer to add BIND_DEFAULT_SYMBOL to the public header, not the source file. This commit fixes the issue and adds some clarifications. The commit also clarifies the use of use_function_versioning in the meson/ninja build system, and does some minor re-organization of the document. Fixes: f1ef9794f9bd ("doc: add ABI guidelines") Signed-off-by: Ray Kinsella --- doc/guides/contributing/abi_versioning.rst | 130 ++++++++++++++------- 1 file changed, 87 insertions(+), 43 deletions(-) diff --git a/doc/guides/contributing/abi_versioning.rst b/doc/guides/contributing/abi_versioning.rst index a21f4e7a41..e526fb179a 100644 --- a/doc/guides/contributing/abi_versioning.rst +++ b/doc/guides/contributing/abi_versioning.rst @@ -200,7 +200,7 @@ private, is safe), but it also requires modifying the code as follows 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 +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 @@ -266,12 +266,12 @@ This file needs to be modified as follows } DPDK_20; -The addition of the new block tells the linker that a new version node is -available (DPDK_21), which contains the symbol rte_acl_create, and inherits +The addition of the new block tells the linker that a new version node +``DPDK_21`` is available, which contains the symbol rte_acl_create, and inherits the symbols from the DPDK_20 node. This list is directly translated into a -list of exported symbols when DPDK is compiled as a shared library +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 +Next, we need to specify in the code which function maps to the rte_acl_create symbol at which versions. First, at the site of the initial symbol definition, we need to update the function so that it is uniquely named, and not in conflict with the public symbol name @@ -288,24 +288,29 @@ with the public symbol name ... Note that the base name of the symbol was kept intact, as this is conducive to -the macros used for versioning symbols and we have annotated the function as an -implementation of versioned symbol. That is our next step, mapping this new -symbol name to the initial symbol name at version node 20. Immediately after -the function, we add this line of code +the macros used for versioning symbols and we have annotated the function as +``__vsym``, an implementation of a versioned symbol . That is our next step, +mapping this new symbol name to the initial symbol name at version node 20. +Immediately after the function, we add the VERSION_SYMBOL macro. .. code-block:: c + #include + + ... VERSION_SYMBOL(rte_acl_create, _v20, 20); Remembering to also add the rte_function_versioning.h header to the requisite c -file where these changes are being made. The above macro instructs the linker to +file where these changes are being made. The macro instructs the linker to create a new symbol ``rte_acl_create@DPDK_20``, 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 21 version of the symbol. We create a new function -name, with a different suffix, and implement it appropriately +Please see the section :ref:`Enabling versioning macros +` to enable this macro in the meson/ninja build. +Next, we need to create the new ``v21`` version of the symbol. We create a new +function name, with the ``v21`` suffix, and implement it appropriately. .. code-block:: c @@ -320,35 +325,58 @@ name, with a different suffix, and implement it appropriately } 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_21``. 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 +new parameter in place. Next we need to map this function to the new default +symbol ``rte_acl_create@DPDK_21``. To do this, immediately after the function, +we add the BIND_DEFAULT_SYMBOL macro. + +.. code-block:: c + + #include + + ... + BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 21); + +The macro instructs the linker to create the new default symbol +``rte_acl_create@DPDK_21``, which points to the above newly named function. + +We finally modify the prototype of the call in the public header file, +such that it contains both versions of the symbol and the public API. .. code-block:: c struct rte_acl_ctx * - -rte_acl_create(const struct rte_acl_param *param); - +rte_acl_create_v21(const struct rte_acl_param *param, int debug); - +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 21); - -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_21 -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. + rte_acl_create(const struct rte_acl_param *param); + + struct rte_acl_ctx * __vsym + rte_acl_create_v20(const struct rte_acl_param *param); + + struct rte_acl_ctx * __vsym + rte_acl_create_v21(const struct rte_acl_param *param, int debug); + + +And that's it, on the next shared library rebuild, there will be two versions of +rte_acl_create, an old DPDK_20 version, used by previously built applications, +and a new DPDK_21 version, used by future built applications. + +.. note:: + + **Before you leave**, please take care reviewing the sections on + :ref:`mapping static symbols `, + :ref:`enabling versioning macros `, + and :ref:`ABI deprecation `. + + +.. _mapping_static_symbols: + +Mapping static symbols +______________________ + +Now we've taken what was a public symbol, and duplicated it into two uniquely +and differently named symbols. We've then mapped each of those back to the +public symbol ``rte_acl_create`` with different version tags. This only applies +to dynamic linking, as static linking has no notion of versioning. That leaves +this code in a position of no longer having a symbol simply named +``rte_acl_create`` and a static build will fail on that missing symbol. To correct this, we can simply map a function of our choosing back to the public symbol in the static build with the ``MAP_STATIC_SYMBOL`` macro. Generally the @@ -369,15 +397,31 @@ defined, we add this 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_20 version, used by previously built applications, -and a new DPDK_21 version, used by future built applications. +.. _enabling_versioning_macros: + +Enabling versioning macros +__________________________ + +Finally, we need to indicate to the :doc:`meson/ninja build system +<../prog_guide/build-sdk-meson>` to enable versioning macros when building the +library or driver. In the libraries or driver where we have added symbol +versioning, in the ``meson.build`` file we add the following + +.. code-block:: + + use_function_versioning = true + +at the start of the head of the file. This will indicate to the tool-chain to +enable the function version macros when building. There is no corresponding +directive required for the ``make`` build system. + +.. _abi_deprecation: Deprecating part of a public API ________________________________ -Lets assume that you've done the above update, and in preparation for the next +Lets assume that you've done the above updates, and in preparation for the next major ABI version 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 @@ -421,8 +465,8 @@ Next remove the corresponding versioned export. 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 and declare it -as static. This is a coding style choice. +in our example by the newer version ``v21``, so we leave it in place and declare +it as static. This is a coding style choice. .. _deprecating_entire_abi: -- 2.20.1 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2020-05-22 10:37:39.501733021 +0100 +++ 0007-doc-fix-default-symbol-binding-in-ABI-guide.patch 2020-05-22 10:37:39.060411495 +0100 @@ -1,8 +1,10 @@ -From 45a4103e680d6b9bfb2b4ee4f4ef528d8de51ec0 Mon Sep 17 00:00:00 2001 +From 2d2ba1fb005f8585606be28276a3ff653cd3c69d Mon Sep 17 00:00:00 2001 From: Ray Kinsella Date: Wed, 6 May 2020 16:41:05 +0100 Subject: [PATCH] doc: fix default symbol binding in ABI guide +[ upstream commit 45a4103e680d6b9bfb2b4ee4f4ef528d8de51ec0 ] + The document abi_versioning.rst incorrectly instructs the developer to add BIND_DEFAULT_SYMBOL to the public header, not the source file. This commit fixes the issue and adds some clarifications. @@ -12,7 +14,6 @@ document. Fixes: f1ef9794f9bd ("doc: add ABI guidelines") -Cc: stable@dpdk.org Signed-off-by: Ray Kinsella ---