| Message ID | 20200302145829.27808-1-hemant.agrawal@nxp.com (mailing list archive) |
|---|---|
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id D671FA0568; Mon, 2 Mar 2020 10:26:33 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 76DA82BB8; Mon, 2 Mar 2020 10:26:33 +0100 (CET) Received: from inva020.nxp.com (inva020.nxp.com [92.121.34.13]) by dpdk.org (Postfix) with ESMTP id 1A0F423D for <dev@dpdk.org>; Mon, 2 Mar 2020 10:26:32 +0100 (CET) Received: from inva020.nxp.com (localhost [127.0.0.1]) by inva020.eu-rdc02.nxp.com (Postfix) with ESMTP id EB84A1A0F8C; Mon, 2 Mar 2020 10:26:31 +0100 (CET) Received: from invc005.ap-rdc01.nxp.com (invc005.ap-rdc01.nxp.com [165.114.16.14]) by inva020.eu-rdc02.nxp.com (Postfix) with ESMTP id 41D1E1A02D7; Mon, 2 Mar 2020 10:26:30 +0100 (CET) Received: from bf-netperf1.ap.com (bf-netperf1.ap.freescale.net [10.232.133.63]) by invc005.ap-rdc01.nxp.com (Postfix) with ESMTP id A14A540299; Mon, 2 Mar 2020 17:26:27 +0800 (SGT) From: Hemant Agrawal <hemant.agrawal@nxp.com> To: ferruh.yigit@intel.com Cc: dev@dpdk.org Date: Mon, 2 Mar 2020 20:28:13 +0530 Message-Id: <20200302145829.27808-1-hemant.agrawal@nxp.com> X-Mailer: git-send-email 2.17.1 X-Virus-Scanned: ClamAV using ClamSMTP Subject: [dpdk-dev] [PATCH 00/16] NXP DPAAx fixes and enhancements X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
| Series |
NXP DPAAx fixes and enhancements
|
|
Message
Hemant Agrawal
March 2, 2020, 2:58 p.m. UTC
This patch series add various patches for enhancing and fixing NXP fslmc bus, dpaa bus, and dpaax. - the main change is support to allow thread migration across lcores - improving the multi-process support Apeksha Gupta (1): bus/fslmc: fix dereferencing null pointer Gagandeep Singh (2): bus/fslmc: combine thread specific variables net/dpaa: enable Tx queue taildrop Hemant Agrawal (1): bus/fslmc: support handle portal alloc failure Nipun Gupta (8): bus/fslmc: rework portal allocation to a per thread basis bus/fslmc: limit pthread destructor called for dpaa2 only bus/fslmc: support portal migration drivers: enhance portal allocation failure log bus/fslmc: rename the cinh read functions used for ls1088 net/dpaa: return error on multiple mp config net/dpaa: update process specific device info net/dpaa2: do not prefetch annotaion for physical mode Rohit Raj (3): net/dpaa2: fix 10g port negotiation issue bus/dpaa: enable link state interrupt bus/dpaa: enable set link status Sachin Saxena (1): net/dpaa: add 2.5G support drivers/bus/dpaa/base/fman/fman.c | 10 +- drivers/bus/dpaa/base/fman/netcfg_layer.c | 3 +- drivers/bus/dpaa/base/qbman/process.c | 95 ++- drivers/bus/dpaa/base/qbman/qman.c | 43 ++ drivers/bus/dpaa/dpaa_bus.c | 28 +- drivers/bus/dpaa/include/fman.h | 3 + drivers/bus/dpaa/include/fsl_qman.h | 17 + drivers/bus/dpaa/include/process.h | 23 + drivers/bus/dpaa/rte_bus_dpaa_version.map | 11 + drivers/bus/dpaa/rte_dpaa_bus.h | 6 +- drivers/bus/fslmc/Makefile | 1 + drivers/bus/fslmc/fslmc_bus.c | 2 - drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 284 ++++----- drivers/bus/fslmc/portal/dpaa2_hw_dpio.h | 10 +- drivers/bus/fslmc/portal/dpaa2_hw_pvt.h | 14 +- .../fslmc/qbman/include/fsl_qbman_portal.h | 8 +- drivers/bus/fslmc/qbman/qbman_debug.c | 9 +- drivers/bus/fslmc/qbman/qbman_portal.c | 580 +++++++++++++++++- drivers/bus/fslmc/qbman/qbman_portal.h | 19 +- drivers/bus/fslmc/qbman/qbman_sys.h | 135 +++- drivers/bus/fslmc/rte_fslmc.h | 18 - drivers/common/dpaax/compat.h | 5 +- drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +- drivers/event/dpaa2/dpaa2_eventdev.c | 8 +- drivers/mempool/dpaa/dpaa_mempool.c | 1 + drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 12 +- drivers/net/dpaa/dpaa_ethdev.c | 417 +++++++++---- drivers/net/dpaa/dpaa_ethdev.h | 3 +- drivers/net/dpaa/dpaa_rxtx.c | 71 +++ drivers/net/dpaa/dpaa_rxtx.h | 3 + drivers/net/dpaa2/dpaa2_ethdev.c | 8 +- drivers/net/dpaa2/dpaa2_rxtx.c | 56 +- drivers/raw/dpaa2_cmdif/dpaa2_cmdif.c | 8 +- drivers/raw/dpaa2_qdma/dpaa2_qdma.c | 12 +- 34 files changed, 1566 insertions(+), 365 deletions(-)
Comments
On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal <hemant.agrawal@nxp.com> wrote: > > This patch series add various patches for enhancing and fixing NXP > fslmc bus, dpaa bus, and dpaax. > > - the main change is support to allow thread migration across lcores > - improving the multi-process support This series triggers an ABI warning that must be investigated. https://travis-ci.com/ovsrobot/dpdk/jobs/292904119#L2233
Hi David, > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal > <hemant.agrawal@nxp.com> wrote: > > > > This patch series add various patches for enhancing and fixing NXP > > fslmc bus, dpaa bus, and dpaax. > > > > - the main change is support to allow thread migration across lcores > > - improving the multi-process support > > This series triggers an ABI warning that must be investigated. >https://travis-ci.com/ovsrobot/dpdk/jobs/292904119#L2233 [Hemant] As per the logs: Variables changes summary: 1 Removed, 2 Changed, 0 Added variables 1 Removed variable: 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' {per_lcore_dpaa2_held_bufs@@DPDK_20.0} 2 Changed variables: [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 Error: ABI issue reported for 'abidiff --suppr devtools/libabigail.abignore --no-added-syms --headers-dir1 reference/usr/local/include --headers-dir2 install/usr/local/include reference/dump/librte_bus_fslmc.dump install/dump/librte_bus_fslmc.dump' --------------- These changes are w.r.t modifications in internal structures and variables. They may be ignored. > > -- > David Marchand
On Thu, Mar 5, 2020 at 10:06 AM Hemant Agrawal (OSS) <hemant.agrawal@oss.nxp.com> wrote: > > Hi David, > > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal > > <hemant.agrawal@nxp.com> wrote: > > > > > > This patch series add various patches for enhancing and fixing NXP > > > fslmc bus, dpaa bus, and dpaax. > > > > > > - the main change is support to allow thread migration across lcores > > > - improving the multi-process support > > > > This series triggers an ABI warning that must be investigated. > >https://travis-ci.com/ovsrobot/dpdk/jobs/292904119#L2233 > > [Hemant] > As per the logs: > > Variables changes summary: 1 Removed, 2 Changed, 0 Added variables > 1 Removed variable: > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' {per_lcore_dpaa2_held_bufs@@DPDK_20.0} > 2 Changed variables: > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 > > Error: ABI issue reported for 'abidiff --suppr devtools/libabigail.abignore --no-added-syms --headers-dir1 reference/usr/local/include --headers-dir2 install/usr/local/include reference/dump/librte_bus_fslmc.dump install/dump/librte_bus_fslmc.dump' > > --------------- > > These changes are w.r.t modifications in internal structures and variables. They may be ignored. The ABI check considers symbol exposed in headers available to final users. If those are internal, why are the headers public?
Hi David, > On Thu, Mar 5, 2020 at 10:06 AM Hemant Agrawal (OSS) > <hemant.agrawal@oss.nxp.com> wrote: > > > > Hi David, > > > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal > > > <hemant.agrawal@nxp.com> wrote: > > > > > > > > This patch series add various patches for enhancing and fixing NXP > > > > fslmc bus, dpaa bus, and dpaax. > > > > > > > > - the main change is support to allow thread migration across > > > > lcores > > > > - improving the multi-process support > > > > > > This series triggers an ABI warning that must be investigated. > > >https://travis-ci.com/ovsrobot/dpdk/jobs/292904119#L2233 > > > > [Hemant] > > As per the logs: > > > > Variables changes summary: 1 Removed, 2 Changed, 0 Added variables > > 1 Removed variable: > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' > {per_lcore_dpaa2_held_bufs@@DPDK_20.0} > > 2 Changed variables: > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 > > > > Error: ABI issue reported for 'abidiff --suppr devtools/libabigail.abignore -- > no-added-syms --headers-dir1 reference/usr/local/include --headers-dir2 > install/usr/local/include reference/dump/librte_bus_fslmc.dump > install/dump/librte_bus_fslmc.dump' > > > > --------------- > > > > These changes are w.r.t modifications in internal structures and variables. > They may be ignored. > > The ABI check considers symbol exposed in headers available to final users. > If those are internal, why are the headers public? > [Hemant] These symbols are not part of any public header files, but they are part of *.map files to share them between different driver libs i.e bus_fslmc and net_dpaa2 > > -- > David Marchand
On Thu, Mar 5, 2020 at 10:19 AM Hemant Agrawal (OSS) <hemant.agrawal@oss.nxp.com> wrote: > > On Thu, Mar 5, 2020 at 10:06 AM Hemant Agrawal (OSS) > > <hemant.agrawal@oss.nxp.com> wrote: > > > > > > Hi David, > > > > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal > > > > <hemant.agrawal@nxp.com> wrote: > > > > > > > > > > This patch series add various patches for enhancing and fixing NXP > > > > > fslmc bus, dpaa bus, and dpaax. > > > > > > > > > > - the main change is support to allow thread migration across > > > > > lcores > > > > > - improving the multi-process support > > > > > > > > This series triggers an ABI warning that must be investigated. > > > >https://travis-ci.com/ovsrobot/dpdk/jobs/292904119#L2233 > > > > > > [Hemant] > > > As per the logs: > > > > > > Variables changes summary: 1 Removed, 2 Changed, 0 Added variables > > > 1 Removed variable: > > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' > > {per_lcore_dpaa2_held_bufs@@DPDK_20.0} > > > 2 Changed variables: > > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at > > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 > > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at > > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 > > > > > > Error: ABI issue reported for 'abidiff --suppr devtools/libabigail.abignore -- > > no-added-syms --headers-dir1 reference/usr/local/include --headers-dir2 > > install/usr/local/include reference/dump/librte_bus_fslmc.dump > > install/dump/librte_bus_fslmc.dump' > > > > > > --------------- > > > > > > These changes are w.r.t modifications in internal structures and variables. > > They may be ignored. > > > > The ABI check considers symbol exposed in headers available to final users. > > If those are internal, why are the headers public? > > > > [Hemant] These symbols are not part of any public header files, but they are part of *.map files to share them between different driver libs i.e bus_fslmc and net_dpaa2 I would expect libabigail to skip those symbols, so there is something I have missed in how --headers-dirX work. Anyway, all of those symbols in dpaa are part of the driver ABI. We are still missing a way to mark internal symbols. Neil had posted a framework for this http://patchwork.dpdk.org/project/dpdk/list/?series=5004. In order to get this series passing the checks, I recommend NXP rebasing Neil scripts (I will help reviewing this part), then mark all those symbols as internal in its drivers. Other vendor will convert their drivers later, as there is no need at the moment. Thanks.
Hello, David Marchand <david.marchand@redhat.com> writes: > On Thu, Mar 5, 2020 at 10:19 AM Hemant Agrawal (OSS) > <hemant.agrawal@oss.nxp.com> wrote: >> > On Thu, Mar 5, 2020 at 10:06 AM Hemant Agrawal (OSS) >> > <hemant.agrawal@oss.nxp.com> wrote: >> > > >> > > Hi David, >> > > > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal >> > > > <hemant.agrawal@nxp.com> wrote: >> > > > > >> > > > > This patch series add various patches for enhancing and fixing NXP >> > > > > fslmc bus, dpaa bus, and dpaax. >> > > > > >> > > > > - the main change is support to allow thread migration across >> > > > > lcores >> > > > > - improving the multi-process support >> > > > >> > > > This series triggers an ABI warning that must be investigated. >> > > >https://travis-ci.com/ovsrobot/dpdk/jobs/292904119#L2233 >> > > >> > > [Hemant] >> > > As per the logs: >> > > >> > > Variables changes summary: 1 Removed, 2 Changed, 0 Added variables >> > > 1 Removed variable: >> > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' >> > {per_lcore_dpaa2_held_bufs@@DPDK_20.0} >> > > 2 Changed variables: >> > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at >> > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 >> > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at >> > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 >> > > >> > > Error: ABI issue reported for 'abidiff --suppr devtools/libabigail.abignore -- >> > no-added-syms --headers-dir1 reference/usr/local/include --headers-dir2 >> > install/usr/local/include reference/dump/librte_bus_fslmc.dump >> > install/dump/librte_bus_fslmc.dump' >> > > >> > > --------------- >> > > >> > > These changes are w.r.t modifications in internal structures and variables. >> > They may be ignored. >> > >> > The ABI check considers symbol exposed in headers available to final users. >> > If those are internal, why are the headers public? >> > >> >> [Hemant] These symbols are not part of any public header files, but >> they are part of *.map files to share them between different driver >> libs i.e bus_fslmc and net_dpaa2 > > I would expect libabigail to skip those symbols, so there is something > I have missed in how --headers-dirX work. In libabigail speak, we make a difference between *ELF symbols* and types. --header-dirX is about telling the tool what the public *types* are. As you rightfully implied, types that are defined in files that are not found in the directories specified by --header-dirX are considered to be private types and are thus not shown in the ABI change report. ELF symbols however are a different matter. Header files don't usually define ELF symbols, be they variable of function symbols. Header files can at most declare variables or functions that would be actually defined elsewhere in source code, leading to the definition of ELF variable or function symbols in the final binary. At this point, we aren't talking about types anymore, as the ELF format doesn't know what types (in C or any other language) are. So --header-dirX don't deal with ELF symbols. And from what I understand from the message quoted above, the changes we are talking about have to do with EFL variable symbols which size have changed. So in practise, these are global arrays (exposed by at the binary level as an ELF variable symbol of a given size) with public visibility which size have changed. So my guess would be that if you guys don't want these arrays to be part the binary interface of this library, they should probably be declared static at the C level and accessed through some accessor function or something like that. At least, that's my humble uninformed opinion. In the mean time, the tooling can be tought to ignore changes to these ELF symbols, as you you guys all know already. > Anyway, all of those symbols in dpaa are part of the driver ABI. > We are still missing a way to mark internal symbols. > Neil had posted a framework for this > http://patchwork.dpdk.org/project/dpdk/list/?series=5004. > > In order to get this series passing the checks, I recommend NXP > rebasing Neil scripts (I will help reviewing this part), then mark all > those symbols as internal in its drivers. > Other vendor will convert their drivers later, as there is no need at > the moment. > > Thanks. Cheers,
HI Dodji, > > David Marchand <david.marchand@redhat.com> writes: > > > On Thu, Mar 5, 2020 at 10:19 AM Hemant Agrawal (OSS) > > <hemant.agrawal@oss.nxp.com> wrote: > >> > On Thu, Mar 5, 2020 at 10:06 AM Hemant Agrawal (OSS) > >> > <hemant.agrawal@oss.nxp.com> wrote: > >> > > > >> > > Hi David, > >> > > > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal > >> > > > <hemant.agrawal@nxp.com> wrote: > >> > > > > > >> > > > > This patch series add various patches for enhancing and > >> > > > > fixing NXP fslmc bus, dpaa bus, and dpaax. > >> > > > > > >> > > > > - the main change is support to allow thread migration across > >> > > > > lcores > >> > > > > - improving the multi-process support > >> > > > > >> > > > This series triggers an ABI warning that must be investigated. > >> > > >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F% > >> > > >2Ftravis- > ci.com%2Fovsrobot%2Fdpdk%2Fjobs%2F292904119%23L2233& > >> > > > >;data=02%7C01%7Chemant.agrawal%40nxp.com%7C91a3230cfd334c28bbd > b0 > >> > > > >8d7c4dee590%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6371 > 943 > >> > > > >33920176015&sdata=%2BViKwS2sNucwLFD9VtvwxOK1huq0g%2B6TfT6 > Fqp > >> > > >Nyz5w%3D&reserved=0 > >> > > > >> > > [Hemant] > >> > > As per the logs: > >> > > > >> > > Variables changes summary: 1 Removed, 2 Changed, 0 Added > >> > > variables > >> > > 1 Removed variable: > >> > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' > >> > {per_lcore_dpaa2_held_bufs@@DPDK_20.0} > >> > > 2 Changed variables: > >> > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at > >> > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 > >> > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at > >> > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 > >> > > > >> > > Error: ABI issue reported for 'abidiff --suppr > >> > > devtools/libabigail.abignore -- > >> > no-added-syms --headers-dir1 reference/usr/local/include > >> > --headers-dir2 install/usr/local/include > >> > reference/dump/librte_bus_fslmc.dump > >> > install/dump/librte_bus_fslmc.dump' > >> > > > >> > > --------------- > >> > > > >> > > These changes are w.r.t modifications in internal structures and > variables. > >> > They may be ignored. > >> > > >> > The ABI check considers symbol exposed in headers available to final > users. > >> > If those are internal, why are the headers public? > >> > > >> > >> [Hemant] These symbols are not part of any public header files, but > >> they are part of *.map files to share them between different driver > >> libs i.e bus_fslmc and net_dpaa2 > > > > I would expect libabigail to skip those symbols, so there is something > > I have missed in how --headers-dirX work. > > In libabigail speak, we make a difference between *ELF symbols* and types. > > --header-dirX is about telling the tool what the public *types* are. As you > rightfully implied, types that are defined in files that are not found in the > directories specified by --header-dirX are considered to be private types and > are thus not shown in the ABI change report. > > ELF symbols however are a different matter. Header files don't usually define > ELF symbols, be they variable of function symbols. Header files can at most > declare variables or functions that would be actually defined elsewhere in > source code, leading to the definition of ELF variable or function symbols in the > final binary. At this point, we aren't talking about types anymore, as the ELF > format doesn't know what types (in C or any other language) are. So --header- > dirX don't deal with ELF symbols. > > And from what I understand from the message quoted above, the changes we > are talking about have to do with EFL variable symbols which size have > changed. So in practise, these are global arrays (exposed by at the binary level > as an ELF variable symbol of a given size) with public visibility which size have > changed. > > So my guess would be that if you guys don't want these arrays to be part the > binary interface of this library, they should probably be declared static at the C > level and accessed through some accessor function or something like that. At > least, that's my humble uninformed opinion. [Hemant] Actually some of these are in datapath, there is a performance impact of accessing them via function calls. > > In the mean time, the tooling can be tought to ignore changes to these ELF > symbols, as you you guys all know already. > [Hemant] will you please help me about adding entry to libagigail.abignore I tried doing following, but it is not helping --- a/devtools/libabigail.abignore +++ b/devtools/libabigail.abignore @@ -2,10 +2,15 @@ symbol_version = EXPERIMENTAL [suppress_variable] symbol_version = EXPERIMENTAL + name = per_lcore__dpaa2_io + name = dpaa2_io_portal ; Explicit ignore for driver-only ABI [suppress_type] name = rte_cryptodev_ops + name = dpaa2_io_portal_t > > > Anyway, all of those symbols in dpaa are part of the driver ABI. > > We are still missing a way to mark internal symbols. > > Neil had posted a framework for this > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatchwo > rk.dpdk.org%2Fproject%2Fdpdk%2Flist%2F%3Fseries%3D5004&data=02 > %7C01%7Chemant.agrawal%40nxp.com%7C91a3230cfd334c28bbdb08d7c4d > ee590%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6371943339 > 20186005&sdata=1Is%2BqQwP%2Bn0QVJ2HYK2%2Bx7TJooEvry1sNUUN > fWMygkM%3D&reserved=0. > > > > In order to get this series passing the checks, I recommend NXP > > rebasing Neil scripts (I will help reviewing this part), then mark all > > those symbols as internal in its drivers. > > Other vendor will convert their drivers later, as there is no need at > > the moment. > > [Hemant] I have commented on Neil's series. It needs more changes in existing code. An approach like __rte_experimental will work better. > > Thanks. > Regards, Hemant
07/04/2020 12:25, Hemant Agrawal: > > In the mean time, the tooling can be tought to ignore changes to these ELF > > symbols, as you you guys all know already. > > > [Hemant] will you please help me about adding entry to libagigail.abignore > I tried doing following, but it is not helping > > --- a/devtools/libabigail.abignore > +++ b/devtools/libabigail.abignore > @@ -2,10 +2,15 @@ > symbol_version = EXPERIMENTAL > [suppress_variable] > symbol_version = EXPERIMENTAL > + name = per_lcore__dpaa2_io > + name = dpaa2_io_portal > > ; Explicit ignore for driver-only ABI > [suppress_type] > name = rte_cryptodev_ops > + name = dpaa2_io_portal_t > > > > > Anyway, all of those symbols in dpaa are part of the driver ABI. > > > We are still missing a way to mark internal symbols. > > > Neil had posted a framework for this > > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatchwo > > rk.dpdk.org%2Fproject%2Fdpdk%2Flist%2F%3Fseries%3D5004&data=02 > > %7C01%7Chemant.agrawal%40nxp.com%7C91a3230cfd334c28bbdb08d7c4d > > ee590%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6371943339 > > 20186005&sdata=1Is%2BqQwP%2Bn0QVJ2HYK2%2Bx7TJooEvry1sNUUN > > fWMygkM%3D&reserved=0. > > > > > > In order to get this series passing the checks, I recommend NXP > > > rebasing Neil scripts (I will help reviewing this part), then mark all > > > those symbols as internal in its drivers. > > > Other vendor will convert their drivers later, as there is no need at > > > the moment. > > > > [Hemant] I have commented on Neil's series. > It needs more changes in existing code. > An approach like __rte_experimental will work better. I guess you mean __rte_internal? Please Hemant don't wait for someone else filling the gap. If __rte_internal is the right approach, please complete and use it.
Hello Hemant, Hemant Agrawal <hemant.agrawal@nxp.com> writes: [...] >> >> > > [Hemant] >> >> > > As per the logs: >> >> > > >> >> > > Variables changes summary: 1 Removed, 2 Changed, 0 Added >> >> > > variables >> >> > > 1 Removed variable: >> >> > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' >> >> > {per_lcore_dpaa2_held_bufs@@DPDK_20.0} >> >> > > 2 Changed variables: >> >> > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at >> >> > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 >> >> > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at >> >> > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 >> >> > > >> >> > > Error: ABI issue reported for 'abidiff --suppr >> >> > > devtools/libabigail.abignore -- >> >> > no-added-syms --headers-dir1 reference/usr/local/include >> >> > --headers-dir2 install/usr/local/include >> >> > reference/dump/librte_bus_fslmc.dump >> >> > install/dump/librte_bus_fslmc.dump' [...] >> In the mean time, the tooling can be tought to ignore changes to these ELF >> symbols, as you you guys all know already. >> > [Hemant] will you please help me about adding entry to libagigail.abignore > I tried doing following, but it is not helping > --- a/devtools/libabigail.abignore > +++ b/devtools/libabigail.abignore > @@ -2,10 +2,15 @@ > symbol_version = EXPERIMENTAL > [suppress_variable] > symbol_version = EXPERIMENTAL > + name = per_lcore__dpaa2_io > + name = dpaa2_io_portal > > ; Explicit ignore for driver-only ABI > [suppress_type] > name = rte_cryptodev_ops > + name = dpaa2_io_portal_t So, I understand you want the tooling to ignore changes to the global arrays dpaa2_io_portal and per_lcore__dpaa2_io, right? If that is correct, then here are the entries you should add to the libabigail.abignore file (please make sure the comments I have added there is accurate): [suppress_variable] # This global variable is exported by the binary but is not part of # the logical ABI. In a perfect world, that variable should not be # global, and we should access it via an accessor function. We do # that right now because of performance concerns. name = dpaa2_io_portal [suppress_variable] # This global variable is exported by the binary but is not part of # the logical ABI. In a perfect world, that variable should not be # global, and we should access it via an accessor function. We do # that right now because of performance concerns. name = per_lcore__dpaa2_io I hope this helps. Cheers,
Hello Thomas, Hemant, Thomas Monjalon <thomas@monjalon.net> writes: > 07/04/2020 12:25, Hemant Agrawal: [...] >> [Hemant] I have commented on Neil's series. >> It needs more changes in existing code. >> An approach like __rte_experimental will work better. > > I guess you mean __rte_internal? > > Please Hemant don't wait for someone else filling the gap. > If __rte_internal is the right approach, please complete and use it. Just so that I understand, is __rte_internal an ELF version that the symbols per_lcore_dpaa2_held_bufs, dpaa2_io_portal and per_lcore__dpaa2_io should have in the binary? If that is the case, then it seems to me that the __rte_internal approach that you are suggesting would be a much better approach that the one I replied to Hemant about below. I didn't mean to tell Hemant what approach he should take :-) I was just trying to help him get the syntax of a libabigail suppression specification right. Sorry for the confusion I might have induced. Dodji Seketeli <dseketel@redhat.com> writes: > Hello Hemant, > > Hemant Agrawal <hemant.agrawal@nxp.com> writes: > > [...] > >>> >> > > [Hemant] >>> >> > > As per the logs: >>> >> > > >>> >> > > Variables changes summary: 1 Removed, 2 Changed, 0 Added >>> >> > > variables >>> >> > > 1 Removed variable: >>> >> > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs' >>> >> > {per_lcore_dpaa2_held_bufs@@DPDK_20.0} >>> >> > > 2 Changed variables: >>> >> > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at >>> >> > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048 >>> >> > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at >>> >> > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16 >>> >> > > >>> >> > > Error: ABI issue reported for 'abidiff --suppr >>> >> > > devtools/libabigail.abignore -- >>> >> > no-added-syms --headers-dir1 reference/usr/local/include >>> >> > --headers-dir2 install/usr/local/include >>> >> > reference/dump/librte_bus_fslmc.dump >>> >> > install/dump/librte_bus_fslmc.dump' > > [...] > >>> In the mean time, the tooling can be tought to ignore changes to these ELF >>> symbols, as you you guys all know already. >>> >> [Hemant] will you please help me about adding entry to libagigail.abignore >> I tried doing following, but it is not helping >> --- a/devtools/libabigail.abignore >> +++ b/devtools/libabigail.abignore >> @@ -2,10 +2,15 @@ >> symbol_version = EXPERIMENTAL >> [suppress_variable] >> symbol_version = EXPERIMENTAL >> + name = per_lcore__dpaa2_io >> + name = dpaa2_io_portal >> >> ; Explicit ignore for driver-only ABI >> [suppress_type] >> name = rte_cryptodev_ops >> + name = dpaa2_io_portal_t > > So, I understand you want the tooling to ignore changes to the global > arrays dpaa2_io_portal and per_lcore__dpaa2_io, right? > > If that is correct, then here are the entries you should add to the > libabigail.abignore file (please make sure the comments I have added > there is accurate): > > [suppress_variable] > # This global variable is exported by the binary but is not part of > # the logical ABI. In a perfect world, that variable should not be > # global, and we should access it via an accessor function. We do > # that right now because of performance concerns. > name = dpaa2_io_portal > > [suppress_variable] > # This global variable is exported by the binary but is not part of > # the logical ABI. In a perfect world, that variable should not be > # global, and we should access it via an accessor function. We do > # that right now because of performance concerns. > name = per_lcore__dpaa2_io > > I hope this helps. > > Cheers,
08/04/2020 09:52, Dodji Seketeli: > Hello Thomas, Hemant, > > Thomas Monjalon <thomas@monjalon.net> writes: > > 07/04/2020 12:25, Hemant Agrawal: > > [...] > > >> [Hemant] I have commented on Neil's series. > >> It needs more changes in existing code. > >> An approach like __rte_experimental will work better. > > > > I guess you mean __rte_internal? > > > > Please Hemant don't wait for someone else filling the gap. > > If __rte_internal is the right approach, please complete and use it. > > Just so that I understand, is __rte_internal an ELF version that the > symbols per_lcore_dpaa2_held_bufs, dpaa2_io_portal and > per_lcore__dpaa2_io should have in the binary? Correct > If that is the case, then it seems to me that the __rte_internal > approach that you are suggesting would be a much better approach that > the one I replied to Hemant about below. Yes I think we all agree, just waiting for the patch to be ready. > I didn't mean to tell Hemant what approach he should take :-) I was just > trying to help him get the syntax of a libabigail suppression > specification right. > > Sorry for the confusion I might have induced. No problem, thanks for the help.