Re: [dpdk-dev] [PATCH v1 0/9] dpdk: introduce __rte_internal tag

[Neil Horman <nhorman@tuxdriver.com>]

On Thu, Jun 13, 2019 at 09:53:46AM +0200, David Marchand wrote:
> On Wed, Jun 12, 2019 at 10:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > Hey-
> >         Based on our recent conversations regarding the use of symbols only
> > meant for internal dpdk consumption (between dpdk libraries), this is an
> > idea
> > that I've come up with that I'd like to get some feedback on
> >
> > Summary:
> > 1) We have symbols in the DPDK that are meant to be used between DPDK
> > libraries,
> > but not by applications linking to them
> > 2) We would like to document those symbols in the code, so as to note them
> > clearly as for being meant for internal use only
> > 3) Linker symbol visibility is a very coarse grained tool, and so there is
> > no
> > good way in a single library to mark items as being meant for use only by
> > other
> > DPDK libraries, at least not without some extensive runtime checking
> >
> >
> > Proposal:
> > I'm proposing that we introduce the __rte_internal tag.  From a coding
> > standpoint it works a great deal like the __rte_experimental tag in that it
> > expempts the tagged symbol from ABI constraints (as the only users should
> > be
> > represented in the DPDK build environment).  Additionally, the
> > __rte_internal
> > macro resolves differently based on the definition of the BUILDING_RTE_SDK
> > flag
> > (working under the assumption that said flag should only ever be set if we
> > are
> > actually building DPDK libraries which will make use of internal calls).
> > If the
> > BUILDING_RTE_SDK flag is set __rte_internal resolves to
> > __attribute__((section
> > "text.internal)), placing it in a special text section which is then used
> > to
> > validate that the the symbol appears in the INTERNAL section of the
> > corresponding library version map).  If BUILDING_RTE_SDK is not set, then
> > __rte_internal resolves to __attribute__((error("..."))), which causes any
> > caller of the tagged function to throw an error at compile time,
> > indicating that
> > the symbol is not available for external use.
> >
> > This isn't a perfect solution, as applications can still hack around it of
> > course, but I think it hits some of the high points, restricting symbol
> > access
> > for any library that prototypes its public and private symbols in the same
> > header file, excluding the internal symbols from ABI constraints, and
> > clearly
> > documenting those symbols which we wish to limit to internal usage.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > CC: Bruce Richardson <bruce.richardson@intel.com>
> > CC: Thomas Monjalon <thomas@monjalon.net>
> >
> >
> >
> On the principle, are we not breaking the ABI for the libraries that we
> touch with this?
> 
Yes, strictly speaking on principle, we are, because we are removing symbols
from the exported symbol list, but the purpose of this patch set was to identify
those symbols which, by (convention/documentation/etc), we had intended to
mandate should never be used by applications outside of the DPDK.  Since this
patch is meant to identify those symbols, and somewhat more formally isolate and
prevent users from accessing them, I was making the argument that it was ok to
do so.  If you like we can go through the process of deprecating them, prior to
moving them to an internal space, but given that I was under the impression
anyone reporting a bug about the symbol disappearing would be responded to with
a note indicating that they never should have been accessed to begin with, I had
assumed the movement would be ok.

> Compilation is broken in patch 1 and 2 because the script rename is part of
> patch 3.
> 
Good point, I'll reshuffle those to be more correct.
Neil

> 
> -- 
> David Marchand