[v3] eal: add build-time option to omit trace

Message ID 20240920090826.571755-1-mb@smartsharesystems.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series [v3] eal: add build-time option to omit trace |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS

Commit Message

Morten Brørup Sept. 20, 2024, 9:08 a.m. UTC
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 <mb@smartsharesystems.com>
---
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             | 11 ++++++++++-
 config/rte_config.h               |  1 +
 lib/eal/include/rte_trace_point.h | 21 +++++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)
  

Comments

Jerin Jacob Sept. 23, 2024, 8:39 a.m. UTC | #1
On Fri, Sep 20, 2024 at 2:38 PM Morten Brørup <mb@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 <mb@smartsharesystems.com>
> ---
> 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             | 11 ++++++++++-
>  config/rte_config.h               |  1 +
>  lib/eal/include/rte_trace_point.h | 21 +++++++++++++++++++++
>  3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_trace.c b/app/test/test_trace.c
> index 00809f433b..7918cc865d 100644
> --- a/app/test/test_trace.c
> +++ b/app/test/test_trace.c
> @@ -12,7 +12,16 @@
>
>  int app_dpdk_test_tp_count;
>
> -#ifdef RTE_EXEC_ENV_WINDOWS
> +#if !defined(RTE_TRACE)

Now that, we are is not disabling any symbols,
This conditional compilation can be removed. Use
__rte_trace_point_generic_is_enabled() in another leg.


> +
> +static int
> +test_trace(void)
> +{
> +       printf("trace omitted at build-time, skipping test\n");
> +       return TEST_SKIPPED;
> +}
> +
> +#elif defined(RTE_EXEC_ENV_WINDOWS)
>
>  static int
>  test_trace(void)
> 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/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
> index 41e2a7f99e..b80688ce89 100644
> --- a/lib/eal/include/rte_trace_point.h
> +++ b/lib/eal/include/rte_trace_point.h
> @@ -212,6 +212,25 @@ bool rte_trace_point_is_enabled(rte_trace_point_t *tp);
>  __rte_experimental
>  rte_trace_point_t *rte_trace_point_lookup(const char *name);
>
> +/**
> + * @internal
> + *
> + * Test if the tracepoint compile-time option is enabled.
> + *
> + * @return
> + *   true if tracepoint enabled, false otherwise.
> + */
> +__rte_experimental
> +static __rte_always_inline bool
> +__rte_trace_point_generic_is_enabled(void)

Add to version.map file

> +{
> +#ifdef RTE_TRACE
> +       return true;
> +#else
> +       return false;
> +#endif
> +}
> +
>  /**
>   * @internal
>   *
> @@ -359,6 +378,8 @@ __rte_trace_point_emit_ev_header(void *mem, uint64_t in)
>  #define __rte_trace_point_emit_header_generic(t) \
>  void *mem; \
>  do { \
> +       if (!__rte_trace_point_generic_is_enabled()) \
> +               return; \


To be super safe, Add similar check in
RTE_INIT(trace##_init)

>         const uint64_t val = rte_atomic_load_explicit(t, rte_memory_order_acquire); \
>         if (likely(!(val & __RTE_TRACE_FIELD_ENABLE_MASK))) \
>                 return; \
> --
> 2.43.0
>
  
Morten Brørup Sept. 24, 2024, 1:50 p.m. UTC | #2
> > +++ b/app/test/test_trace.c
> > @@ -12,7 +12,16 @@
> >
> >  int app_dpdk_test_tp_count;
> >
> > -#ifdef RTE_EXEC_ENV_WINDOWS
> > +#if !defined(RTE_TRACE)
> 
> Now that, we are is not disabling any symbols,
> This conditional compilation can be removed. Use
> __rte_trace_point_generic_is_enabled() in another leg.

Done in v4.
Also added to perf test.


> > +/**
> > + * @internal
> > + *
> > + * Test if the tracepoint compile-time option is enabled.
> > + *
> > + * @return
> > + *   true if tracepoint enabled, false otherwise.
> > + */
> > +__rte_experimental
> > +static __rte_always_inline bool
> > +__rte_trace_point_generic_is_enabled(void)
> 
> Add to version.map file

It's static inline, so not an exposed symbol.
__rte_trace_point_fp_is_enabled is also not in the version.map file.


> >  #define __rte_trace_point_emit_header_generic(t) \
> >  void *mem; \
> >  do { \
> > +       if (!__rte_trace_point_generic_is_enabled()) \
> > +               return; \
> 
> 
> To be super safe, Add similar check in
> RTE_INIT(trace##_init)

Done in v4.
  

Patch

diff --git a/app/test/test_trace.c b/app/test/test_trace.c
index 00809f433b..7918cc865d 100644
--- a/app/test/test_trace.c
+++ b/app/test/test_trace.c
@@ -12,7 +12,16 @@ 
 
 int app_dpdk_test_tp_count;
 
-#ifdef RTE_EXEC_ENV_WINDOWS
+#if !defined(RTE_TRACE)
+
+static int
+test_trace(void)
+{
+	printf("trace omitted at build-time, skipping test\n");
+	return TEST_SKIPPED;
+}
+
+#elif defined(RTE_EXEC_ENV_WINDOWS)
 
 static int
 test_trace(void)
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/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
index 41e2a7f99e..b80688ce89 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -212,6 +212,25 @@  bool rte_trace_point_is_enabled(rte_trace_point_t *tp);
 __rte_experimental
 rte_trace_point_t *rte_trace_point_lookup(const char *name);
 
+/**
+ * @internal
+ *
+ * Test if the tracepoint compile-time option is enabled.
+ *
+ * @return
+ *   true if tracepoint enabled, false otherwise.
+ */
+__rte_experimental
+static __rte_always_inline bool
+__rte_trace_point_generic_is_enabled(void)
+{
+#ifdef RTE_TRACE
+	return true;
+#else
+	return false;
+#endif
+}
+
 /**
  * @internal
  *
@@ -359,6 +378,8 @@  __rte_trace_point_emit_ev_header(void *mem, uint64_t in)
 #define __rte_trace_point_emit_header_generic(t) \
 void *mem; \
 do { \
+	if (!__rte_trace_point_generic_is_enabled()) \
+		return; \
 	const uint64_t val = rte_atomic_load_explicit(t, rte_memory_order_acquire); \
 	if (likely(!(val & __RTE_TRACE_FIELD_ENABLE_MASK))) \
 		return; \