Hello,

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://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,

-- 
                Dodji

Reply via email to