> From: Jerin Jacob [mailto:jerinjac...@gmail.com] > Sent: Monday, 7 October 2024 07.45 > > On Sun, Oct 6, 2024 at 7:44 PM Morten Brørup <m...@smartsharesystems.com> > wrote: > > > > Some applications want to omit the trace feature. > > Either to reduce the memory footprint, to reduce the exposed attack > > surface, or for other reasons. > > > > This patch adds an option in rte_config.h to include or omit trace in > the > > build. Trace is included by default. > > > > Omitting trace works by omitting all trace points. > > For API and ABI compatibility, the trace feature itself remains. > > > > Signed-off-by: Morten Brørup <m...@smartsharesystems.com> > > --- > > v6: > > Removed test_trace_perf.c changes; they don't compile for Windows > > target, and are superfluous. > > v5: > > Added public function rte_trace_feature_is_enabled(), to test if > trace > > is build time enabled in both the DPDK and the application. Use in > test > > application instead of private function. (Jerin Jacob) > > v4: > > * Added check for generic trace enabled when registering trace > points, in > > RTE_INIT. (Jerin Jacob) > > * Test application uses function instead of macro to check if generic > > trace is enabled. (Jerin Jacob) > > * Performance test application uses function to check if generic > trace is > > enabled. > > v3: > > * Simpler version with much fewer ifdefs. (Jerin Jacob) > > v2: > > * Added/modified macros required for building applications with trace > > omitted. > > --- > > app/test/test_trace.c | 4 +++ > > config/rte_config.h | 1 + > > lib/eal/common/eal_common_trace.c | 10 +++++++ > > lib/eal/include/rte_trace.h | 33 > ++++++++++++++++++++++ > > lib/eal/include/rte_trace_point.h | 21 ++++++++++++++ > > lib/eal/include/rte_trace_point_register.h | 2 ++ > > lib/eal/version.map | 3 ++ > > 7 files changed, 74 insertions(+) > > > > diff --git a/app/test/test_trace.c b/app/test/test_trace.c > > index 00809f433b..8ea1443044 100644 > > --- a/app/test/test_trace.c > > +++ b/app/test/test_trace.c > > @@ -245,6 +245,10 @@ static struct unit_test_suite trace_tests = { > > static int > > test_trace(void) > > { > > + if (!rte_trace_feature_is_enabled()) { > > + printf("Trace omitted at build-time, skipping > test\n"); > > + return TEST_SKIPPED; > > + } > > return unit_test_suite_runner(&trace_tests); > > } > > > > diff --git a/config/rte_config.h b/config/rte_config.h > > index dd7bb0d35b..fd6f8a2f1a 100644 > > --- a/config/rte_config.h > > +++ b/config/rte_config.h > > @@ -49,6 +49,7 @@ > > #define RTE_MAX_TAILQ 32 > > #define RTE_LOG_DP_LEVEL RTE_LOG_INFO > > #define RTE_MAX_VFIO_CONTAINERS 64 > > +#define RTE_TRACE 1 > > > > /* bsd module defines */ > > #define RTE_CONTIGMEM_MAX_NUM_BUFS 64 > > diff --git a/lib/eal/common/eal_common_trace.c > b/lib/eal/common/eal_common_trace.c > > index 918f49bf4f..06130c756d 100644 > > --- a/lib/eal/common/eal_common_trace.c > > +++ b/lib/eal/common/eal_common_trace.c > > @@ -100,6 +100,16 @@ rte_trace_is_enabled(void) > > return rte_atomic_load_explicit(&trace.status, > rte_memory_order_acquire) != 0; > > } > > > > +bool > > +__rte_trace_feature_is_enabled(void) > > +{ > > +#ifdef RTE_TRACE > > + return true; > > +#else > > + return false; > > +#endif > > +} > > +static __rte_always_inline > > +bool rte_trace_feature_is_enabled(void) > > +{ > > +#ifdef RTE_TRACE > > + return __rte_trace_feature_is_enabled(); > > +#else > > + return false; > > +#endif > > +} > > +__rte_experimental > > +static __rte_always_inline bool > > +__rte_trace_point_generic_is_enabled(void) > > +{ > > +#ifdef RTE_TRACE > > + return true; > > +#else > > + return false; > > +#endif > > __rte_trace_feature_is_enabled(), rte_trace_feature_is_enabled(), > __rte_trace_point_generic_is_enabled() are duplicates. > There is no harm in using a public API inside the implementation. > Please keep only rte_trace_feature_is_enabled() > and use it across implementation and app/test.
They are there to support DPDK being built as a library, and the application being built separately with a different rte_config.h. But I suppose we generally don't support that. I will spin another version, which assumes that both DPDK (library) and application are built with the same rte_config.h. I will rename __rte_trace_point_generic_is_enabled() to rte_trace_feature_is_enabled(), make it non-internal, and use that everywhere. It still needs to be inline, so the compiler can recognize it and omit the implementation when building with RTE_TRACE disabled. > > > +} > > + > > /** > > * @internal > > * > > > rte_vfio_get_device_info; # WINDOWS_NO_EXPORT > > + > > + # added in 24.11 > > + __rte_trace_feature_is_enabled; > > rte_trace_feature_is_enabled; It will be inline, and thus removed from here. > > With the above changes, > > Acked-by: Jerin Jacob <jer...@marvell.com> Thank you for the quick response, Jerin.
