On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.march...@redhat.com> wrote: > > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjac...@gmail.com> wrote: > > On Wed, Apr 1, 2020 at 1:49 PM David Marchand <david.march...@redhat.com> > > wrote: > > > - Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag > > > gates access to APIs so that external users are aware of their status. > > > I have been considering setting this flag unconditionally for internal > > > users in the top Makefile/meson for app/ lib/ and drivers/. > > > I could look at this and prepare a patch about this, but this is not > > > enough here. > > > > It makes sense to me. Let me know when you are planning to send that patch, > > I will rebase this series on top of that. > > Feel free to take over, thanks.
OK. > > > > > > If you don't have time then I can send the patch too. > > I assume the patch content will be: > > 1) Removing experimental API from app, lib, drivers, examples with > > make and meson > > 2) Have it enabled at the global level. > > examples are a special case as they can be compiled out of the dpdk sources. > This is why I excluded them of the list in my mail before. There are a lot of examples already that have ALLOW_EXPERIMENTAL_API. What is the issue in enabling ALLOW_EXPERIMENTAL_API in the in tree example application? > > > > Since the trace changes in the "inline" functions of the specific > > library already > > disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and > > it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path > > trace like RTE_TRACE_POINT(). > > Ah indeed. > Note: RTE_ENABLE_TRACE_DP is not plugged with meson. If you see the " eal: define the public API for trace support" patch, I have updated rte_config.h for meson likewise for other configuration such log for DP. Would you like to have a meson option? and update in meson_options.txt. I don't have a strong opinion on either way? Bruce ,any thoughts? diff --git a/config/rte_config.h b/config/rte_config.h index d30786bc0..6b250288c 100644 --- a/config/rte_config.h +++ b/config/rte_config.h @@ -42,6 +42,7 @@ #define RTE_MAX_MEMZONE 2560 #define RTE_MAX_TAILQ 32 #define RTE_LOG_DP_LEVEL RTE_LOG_INFO +#define RTE_ENABLE_TRACE_DP 0 #define RTE_BACKTRACE 1 #define RTE_MAX_VFIO_CONTAINERS 64 > > > > So I can improve RTE_TRACE_POINT_DP() to make absolute NOP if > > ALLOW_EXPERIMENTAL_API not defined. > > > > On the upside, > > The tracing code will be enabled by default(runtime it is disabled by > > default anyway). > > If some need to fastpath API tracing then ALLOW_EXPERIMENTAL_API need > > to enable. > > So this won't break applications. > > So either keep the RTE_ENABLE_TRACE_DP flag or use > ALLOW_EXPERIMENTAL_API... no opinion. > Thomas? > Anyway we need a compile-time option? > The option is just for compatibility? > Then ALLOW_EXPERIMENTAL_API looks to be the right option. Just like log there is a compile-time option( CONFIG_RTE_LOG_DP_LEVEL for log) to disable trace generation in fastpath using RTE_ENABLE_TRACE_DP. This is to avoid "any" performance impact in fastpath code due to tracer. ALLOW_EXPERIMENTAL_API and RTE_ENABLE_TRACE_DP has different scope. I will add ALLOW_EXPERIMENTAL_API and RTE_ENABLE_TRACE_DP. ALLOW_EXPERIMENTAL_API can be removed later when it is stable. NOTE: I have used if (0) c style approach in the code to disable the code with RTE_ENABLE_TRACE_DP. This is to make sure the code always has compilation check respective RTE_ENABLE_TRACE_DP status. So ALLOW_EXPERIMENTAL_API need to avoid code compilation issues with external applications.
