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

[David Marchand]

On Tue, Dec 4, 2018 at 4:16 PM Neil Horman <nhorman at tuxdriver.com> wrote:

> 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 at tuxdriver.com>
> wrote:
> > > On Mon, Dec 03, 2018 at 02:01:02PM +0100, David Marchand wrote:
> > > 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.
>

Thanks for the test.
I tried with clang and this works fine as well.
Ferruh, could you do a little test with icc?


> 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
>

I find it much easier to track and, while doing the patch, I have found
another issue in the bbdev library header (another patch coming).
Under the impression that this can go undetected for quite some while...
If no one complains, yes, I am for this adjustment.


I am not sure I see what you mean on check-experimental-syms.sh.
I would only do a s/definition/declaration/ in the error message.
Do you have something else in mind ?


Btw, looking at the documentation, I can find no mention about meson and a
quick grep on check-experimental-syms.sh only catches mk/internal/
rte.compile-pre.mk.
Is there a trick ? or is meson simply not __rte_experimental aware ?


-- 
David Marchand