15/10/2024 16:44, Morten Brørup: > > From: Thomas Monjalon [mailto:tho...@monjalon.net] > > Sent: Tuesday, 15 October 2024 16.13 > > > > 15/10/2024 14:53, Morten Brørup: > > > > From: David Marchand [mailto:david.march...@redhat.com] > > > > @@ -255,7 +255,13 @@ __rte_experimental > > > > static inline bool > > > > rte_bitset_test(const uint64_t *bitset, size_t bit_num) > > > > { > > > > +#ifdef ALLOW_EXPERIMENTAL_API > > > > return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num); > > > > +#else > > > > + RTE_SET_USED(bitset); > > > > + RTE_SET_USED(bit_num); > > > > + return false; > > > > +#endif > > > > } > > > > > > This looks wrong! The API is exposed, but does nothing. > > > > Yes, this is what we did already in the past for other experimental > > functions > > called from inline functions. > > There is no choice, the compiler is hitting the warning. > > > > > It is possible to build without ALLOW_EXPERIMENTAL_API; the compiler > > will emit warnings when using experimental APIs. > > > > Yes it is possible to build, > > but it is not said it should work the same. > > > > > If those compiler warnings are not handled as errors, the compiled > > application will be full of bugs. > > > > Yes, that's why there are warnings. > > We may document it better but that's the behavior we have for years. > > There is no easy solution, and making experimental functions work > > without defining ALLOW_EXPERIMENTAL_API is not a really interesting > > goal. > > I think the word "allow" suggests it is not supposed to work if not > > allowed. > > There's a world of difference between "experimental, might have bugs" - which > is what I (and possibly other DPDK consumers) expect - and "experimental, we > know for a fact that it doesn't work" - which is quite a surprise to me.
It does not work if you don't enable it, and there is a warning when compiling. > > It would be more interesting to make sure the users understand > > why we have this flag and how to enable it. > > I propose adding some docs, and mentioning ALLOW_EXPERIMENTAL_API > > in the the __rte_experimental message in rte_compat.h. > > > > If we know that some of these warnings cause bugs in DPDK, we should elevate > these specific instances to error level. Technically I don't know whether it is possible. Look at rte_compat.h > Regarding this specific patch: > Would it be possible to change it to behave like patch 1/3, i.e. completely > omit the experimental APIs if ALLOW_EXPERIMENTAL_API is not defined? I disagree, it would be a lot of churn in the code to disable all experimental API calls.
