Re: [dpdk-dev] Marking symbols as experimental in the headers only

[Neil Horman <nhorman@tuxdriver.com>]

On Tue, Dec 04, 2018 at 09:21:43AM +0100, David Marchand wrote:
> On Mon, Dec 3, 2018 at 5:48 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Mon, Dec 03, 2018 at 02:01:02PM +0100, David Marchand wrote:
> > > Hello Neil,
> > >
> > > Looking at
> > >
> > http://doc.dpdk.org/guides/contributing/versioning.html#experimental-apis,
> > > is there a real need to mark both the definition and the declaration of a
> > > symbol with the __rte_experimental marker ?
> > >
> > When I initially wrote the feature I marked both the function prototype
> > and its
> > definition with the macro to be consistent, as I like to make both
> > declaration
> > and definition look visually simmilar, just to ease readability, but I'm
> > not
> > bound to doing it that way per-se
> >
> > As to weather or not you can only mark the declaration as experimental,
> > I'm not
> > 100% sure.  I think thats an artifact of the compiler.  That is to say, the
> > macro expands to a compiler attribute that is applied at compile time, and
> > checked at link time, and I'm not sure what clang or gcc will do if there
> > is a
> > discrepancy between the attributes listed in the declaration and the
> > definition.
> >
> > I would say, give it a try, and if you can demonstrate that adding the
> > attribute
> > only to the declaration results in a link-time warning when external code
> > attempts to call an experimental function, I would have no problem
> > handling it
> > that way.
> >
> 
> Did not experiment yet, putting this in my todolist.
> Just, I can see that lib/librte_pipeline/ at least only marks the
> declarations.
> 
I just tried it. If I turn off ALLOW_EXPERIMENTAL_APIS, the ip_pipeline example
breaks with warnings about deprecated functions:

/home/nhorman/git/dpdk/examples/ip_pipeline/action.c:60:4: error: ‘rte_table_hash_crc_key8’ is deprecated: Symbol is not yet part of stable ABI [-Werror=deprecated-declarations]
    params->lb.f_hash = rte_table_hash_crc_key8;
    ^~~~~~
In file included from /home/nhorman/git/dpdk/examples/ip_pipeline/action.c:10:
/home/nhorman/git/dpdk/x86_64-native-linuxapp-gcc/include/rte_table_hash_func.h:56:1: note: declared here
 rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t key_size,
 ^~~~~~~~~~~~~~~~~~~~~~~
/home/nhorman/git/dpdk/examples/ip_pipeline/action.c:64:4: error: ‘rte_table_hash_crc_key16’ is deprecated: Symbol is not yet part of stable ABI [-Werror=deprecated-declarations]
    params->lb.f_hash = rte_table_hash_crc_key16;
    ^~~~~~
...

That is sufficient for me to conclude that __rte_experimental only needs to be
set on the declaration, not the definiton as well.

If you would like to make this adjustment, I'm fine with it, though be aware,
you will likely need to make some adjustments to the check-experimental-syms
script to account for this

Neil

> 
> -- 
> David Marchand