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

["Richardson, Bruce" <bruce.richardson@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand
> Sent: Tuesday, December 4, 2018 12:48 PM
> To: nhorman@tuxdriver.com; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Timothy Redaelli <tredaelli@redhat.com>;
> adrien.mazarguil@6wind.com
> Subject: Re: [dpdk-dev] Marking symbols as experimental in the headers
> only
> 
> On Tue, Dec 4, 2018 at 4:16 PM Neil Horman <nhorman@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@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 ?
> 

Good point, I suspect it isn't. I never thought to investigate how to integrate that tracking into meson builds.

/Bruce