[v8,1/6] eal: trace: add trace point emit for blob

Message ID 20230206115810.308574-2-adwivedi@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series add trace points in ethdev library |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Ankur Dwivedi Feb. 6, 2023, 11:58 a.m. UTC
  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@marvell.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Sunil Kumar Kori <skori@marvell.com>
Acked-by: Jerin Jacob <jerinj@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          | 31 ++++++++++++++++++++++
 lib/eal/include/rte_trace_point_register.h |  9 +++++++
 6 files changed, 71 insertions(+)
  

Comments

David Marchand Feb. 6, 2023, 2:48 p.m. UTC | #1
On Mon, Feb 6, 2023 at 12:59 PM Ankur Dwivedi <adwivedi@marvell.com> 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@marvell.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Sunil Kumar Kori <skori@marvell.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>

I came accross this patch while looking at CI failures.
Giving my two cents.


> diff --git a/lib/eal/include/rte_eal_trace.h b/lib/eal/include/rte_eal_trace.h
> index 5ef4398230..6f5c022558 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(const void *in, uint8_t len),
> +       rte_trace_point_emit_blob(in, len);
> +)
> +
>  #define RTE_EAL_TRACE_GENERIC_FUNC rte_eal_trace_generic_func(__func__)

This new tracepoint is exported as a public API.
So its underlying symbol __rte_eal_trace_generic_blob (part of the
inlined implementation) is visible to applications:

#define __RTE_TRACE_POINT(_mode, _tp, _args, ...) \
extern rte_trace_point_t __##_tp; \
static __rte_always_inline void \
_tp _args \
{ \
        __rte_trace_point_emit_header_##_mode(&__##_tp); \
        __VA_ARGS__ \
}

__rte_eal_trace_generic_blob must be exported as a versionned symbol
(i.e. listed in EAL version.map).
  
Ankur Dwivedi Feb. 7, 2023, 5:08 a.m. UTC | #2
>On Mon, Feb 6, 2023 at 12:59 PM Ankur Dwivedi <adwivedi@marvell.com>
>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@marvell.com>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>> Acked-by: Sunil Kumar Kori <skori@marvell.com>
>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>
>I came accross this patch while looking at CI failures.
>Giving my two cents.
>
>
>> diff --git a/lib/eal/include/rte_eal_trace.h
>> b/lib/eal/include/rte_eal_trace.h index 5ef4398230..6f5c022558 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(const void *in, uint8_t len),
>> +       rte_trace_point_emit_blob(in, len);
>> +)
>> +
>>  #define RTE_EAL_TRACE_GENERIC_FUNC
>> rte_eal_trace_generic_func(__func__)
>
>This new tracepoint is exported as a public API.
>So its underlying symbol __rte_eal_trace_generic_blob (part of the inlined
>implementation) is visible to applications:
>
>#define __RTE_TRACE_POINT(_mode, _tp, _args, ...) \ extern
>rte_trace_point_t __##_tp; \ static __rte_always_inline void \ _tp _args \ { \
>        __rte_trace_point_emit_header_##_mode(&__##_tp); \
>        __VA_ARGS__ \
>}
>
>__rte_eal_trace_generic_blob must be exported as a versionned symbol (i.e.
>listed in EAL version.map).

Yes, as rte_eal_trace_generic_blob is a generic trace point called by the test app, it needs to be exported.
Will add it in next patch series.
>
>
>--
>David Marchand
  

Patch

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..6f5c022558 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(const 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..4d6b5700dd 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)
 
 #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,30 @@  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); \
+	memset(RTE_PTR_ADD(mem, len), 0, RTE_TRACE_BLOB_LEN_MAX - len); \
+	mem = RTE_PTR_ADD(mem, RTE_TRACE_BLOB_LEN_MAX); \
+} 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..a9682d3f22 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)
+
 #ifdef __cplusplus
 }
 #endif