HI Dodji,
>
> David Marchand <david.march...@redhat.com> writes:
>
> > On Thu, Mar 5, 2020 at 10:19 AM Hemant Agrawal (OSS)
> > <hemant.agra...@oss.nxp.com> wrote:
> >> > On Thu, Mar 5, 2020 at 10:06 AM Hemant Agrawal (OSS)
> >> > <hemant.agra...@oss.nxp.com> wrote:
> >> > >
> >> > > Hi David,
> >> > > > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal
> >> > > > <hemant.agra...@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
