[PATCH v6 1/6] eal: trace: add trace point emit for blob

[Ferruh Yigit]

On 1/20/2023 8:40 AM, Ankur Dwivedi wrote:
> Adds a trace point emit function for capturing a blob. The blob
> captures the length passed by the application followed by the array.
> 
> The maximum blob bytes which can be captured is bounded by
> RTE_TRACE_BLOB_LEN_MAX macro. The value for max blob length macro is
> 64 bytes. If the length is less than 64 the remaining trailing bytes
> are set to zero.
> 
> This patch also adds test case for emit blob tracepoint function.
> 
> Signed-off-by: Ankur Dwivedi <adwivedi at marvell.com>
> ---
>  app/test/test_trace.c                      | 11 ++++++++
>  doc/guides/prog_guide/trace_lib.rst        | 12 ++++++++
>  lib/eal/common/eal_common_trace_points.c   |  2 ++
>  lib/eal/include/rte_eal_trace.h            |  6 ++++
>  lib/eal/include/rte_trace_point.h          | 32 ++++++++++++++++++++++
>  lib/eal/include/rte_trace_point_register.h |  9 ++++++
>  lib/eal/version.map                        |  3 ++
>  7 files changed, 75 insertions(+)
> 
> diff --git a/app/test/test_trace.c b/app/test/test_trace.c
> index 6bedf14024..ad4a394a29 100644
> --- a/app/test/test_trace.c
> +++ b/app/test/test_trace.c
> @@ -4,6 +4,7 @@
>  
>  #include <rte_eal_trace.h>
>  #include <rte_lcore.h>
> +#include <rte_random.h>
>  #include <rte_trace.h>
>  
>  #include "test.h"
> @@ -177,7 +178,12 @@ test_fp_trace_points(void)
>  static int
>  test_generic_trace_points(void)
>  {
> +	uint8_t arr[RTE_TRACE_BLOB_LEN_MAX];
>  	int tmp;
> +	int i;
> +
> +	for (i = 0; i < RTE_TRACE_BLOB_LEN_MAX; i++)
> +		arr[i] = i;
>  
>  	rte_eal_trace_generic_void();
>  	rte_eal_trace_generic_u64(0x10000000000000);
> @@ -195,6 +201,11 @@ test_generic_trace_points(void)
>  	rte_eal_trace_generic_ptr(&tmp);
>  	rte_eal_trace_generic_str("my string");
>  	rte_eal_trace_generic_size_t(sizeof(void *));
> +	rte_eal_trace_generic_blob(arr, 0);
> +	rte_eal_trace_generic_blob(arr, 17);
> +	rte_eal_trace_generic_blob(arr, RTE_TRACE_BLOB_LEN_MAX);
> +	rte_eal_trace_generic_blob(arr, rte_rand() %
> +					RTE_TRACE_BLOB_LEN_MAX);
>  	RTE_EAL_TRACE_GENERIC_FUNC;
>  
>  	return TEST_SUCCESS;
> diff --git a/doc/guides/prog_guide/trace_lib.rst b/doc/guides/prog_guide/trace_lib.rst
> index 9a8f38073d..3e0ea5835c 100644
> --- a/doc/guides/prog_guide/trace_lib.rst
> +++ b/doc/guides/prog_guide/trace_lib.rst
> @@ -352,3 +352,15 @@ event ID.
>  The ``packet.header`` and ``packet.context`` will be written in the slow path
>  at the time of trace memory creation. The ``trace.header`` and trace payload
>  will be emitted when the tracepoint function is invoked.
> +
> +Limitations
> +-----------
> +
> +- The ``rte_trace_point_emit_blob()`` function can capture a maximum blob of
> +  length ``RTE_TRACE_BLOB_LEN_MAX`` bytes. The application can call
> +  ``rte_trace_point_emit_blob()`` multiple times with length less than or equal to
> +  ``RTE_TRACE_BLOB_LEN_MAX``, if it needs to capture more than ``RTE_TRACE_BLOB_LEN_MAX``
> +  bytes.
> +- If the length passed to the ``rte_trace_point_emit_blob()`` is less than
> +  ``RTE_TRACE_BLOB_LEN_MAX``, then the trailing ``(RTE_TRACE_BLOB_LEN_MAX - len)``
> +  bytes in the trace are set to zero.
> diff --git a/lib/eal/common/eal_common_trace_points.c b/lib/eal/common/eal_common_trace_points.c
> index 0b0b254615..051f89809c 100644
> --- a/lib/eal/common/eal_common_trace_points.c
> +++ b/lib/eal/common/eal_common_trace_points.c
> @@ -40,6 +40,8 @@ RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_size_t,
>  	lib.eal.generic.size_t)
>  RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_func,
>  	lib.eal.generic.func)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_blob,
> +	lib.eal.generic.blob)
>  
>  RTE_TRACE_POINT_REGISTER(rte_eal_trace_alarm_set,
>  	lib.eal.alarm.set)
> diff --git a/lib/eal/include/rte_eal_trace.h b/lib/eal/include/rte_eal_trace.h
> index 5ef4398230..e0b836eb2f 100644
> --- a/lib/eal/include/rte_eal_trace.h
> +++ b/lib/eal/include/rte_eal_trace.h
> @@ -143,6 +143,12 @@ RTE_TRACE_POINT(
>  	rte_trace_point_emit_string(func);
>  )
>  
> +RTE_TRACE_POINT(
> +	rte_eal_trace_generic_blob,
> +	RTE_TRACE_POINT_ARGS(void *in, uint8_t len),
> +	rte_trace_point_emit_blob(in, len);
> +)
> +
>  #define RTE_EAL_TRACE_GENERIC_FUNC rte_eal_trace_generic_func(__func__)
>  
>  /* Interrupt */
> diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
> index 0f8700974f..aca8344dbf 100644
> --- a/lib/eal/include/rte_trace_point.h
> +++ b/lib/eal/include/rte_trace_point.h
> @@ -144,6 +144,16 @@ _tp _args \
>  #define rte_trace_point_emit_ptr(val)
>  /** Tracepoint function payload for string datatype */
>  #define rte_trace_point_emit_string(val)
> +/**
> + * Tracepoint function to capture a blob.
> + *
> + * @param val
> + *   Pointer to the array to be captured.
> + * @param len
> + *   Length to be captured. The maximum supported length is
> + *   RTE_TRACE_BLOB_LEN_MAX bytes.
> + */
> +#define rte_trace_point_emit_blob(val, len)
>  

This is just for doxygen right, why doxygen comments are not above the
actual macros but there is a separate #if block for it?

>  #endif /* __DOXYGEN__ */
>  
> @@ -152,6 +162,9 @@ _tp _args \
>  /** @internal Macro to define event header size. */
>  #define __RTE_TRACE_EVENT_HEADER_SZ sizeof(uint64_t)
>  
> +/** Macro to define maximum emit length of blob. */
> +#define RTE_TRACE_BLOB_LEN_MAX 64
> +
>  /**
>   * Enable recording events of the given tracepoint in the trace buffer.
>   *
> @@ -374,12 +387,31 @@ do { \
>  	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
>  } while (0)
>  
> +#define rte_trace_point_emit_blob(in, len) \
> +do { \
> +	if (unlikely(in == NULL)) \
> +		return; \
> +	if (len > RTE_TRACE_BLOB_LEN_MAX) \
> +		len = RTE_TRACE_BLOB_LEN_MAX; \
> +	__rte_trace_point_emit(len, uint8_t); \
> +	memcpy(mem, in, len); \
> +	mem = RTE_PTR_ADD(mem, len); \
> +	memset(mem, 0, RTE_TRACE_BLOB_LEN_MAX - len); \
> +	mem = RTE_PTR_ADD(mem, RTE_TRACE_BLOB_LEN_MAX - len); \


Is first memset later memcpy not done because of performance concerns?

> +} while (0)
> +
>  #else
>  
>  #define __rte_trace_point_emit_header_generic(t) RTE_SET_USED(t)
>  #define __rte_trace_point_emit_header_fp(t) RTE_SET_USED(t)
>  #define __rte_trace_point_emit(in, type) RTE_SET_USED(in)
>  #define rte_trace_point_emit_string(in) RTE_SET_USED(in)
> +#define rte_trace_point_emit_blob(in, len) \
> +do { \
> +	RTE_SET_USED(in); \
> +	RTE_SET_USED(len); \
> +} while (0)
> +
>  
>  #endif /* ALLOW_EXPERIMENTAL_API */
>  #endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> diff --git a/lib/eal/include/rte_trace_point_register.h b/lib/eal/include/rte_trace_point_register.h
> index a32f4d731b..7efbac8a72 100644
> --- a/lib/eal/include/rte_trace_point_register.h
> +++ b/lib/eal/include/rte_trace_point_register.h
> @@ -47,6 +47,15 @@ do { \
>  		RTE_STR(in)"[32]", "string_bounded_t"); \
>  } while (0)
>  
> +#define rte_trace_point_emit_blob(in, len) \
> +do { \
> +	RTE_SET_USED(in); \
> +	__rte_trace_point_emit(len, uint8_t); \
> +	__rte_trace_point_emit_field(RTE_TRACE_BLOB_LEN_MAX, \
> +		RTE_STR(in)"["RTE_STR(RTE_TRACE_BLOB_LEN_MAX)"]", \
> +		RTE_STR(uint8_t)); \
> +} while (0)
> +

Why this macro defined here again, it is also defined in
'rte_trace_point.h' already?
Is it because of 'register_fn()' in '__rte_trace_point_register()'?

>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index 7ad12a7dc9..67be24686a 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -440,6 +440,9 @@ EXPERIMENTAL {
>  	rte_thread_detach;
>  	rte_thread_equal;
>  	rte_thread_join;
> +
> +	# added in 23.03
> +	__rte_eal_trace_generic_blob;

This is not a function but a trace object.
I guess it was agreed that trace object not need to be exported, and
trace can be found by name?