[v6,1/6] eal: trace: add trace point emit for blob
Checks
Commit Message
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>
---
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(+)
Comments
> From: Ankur Dwivedi [mailto:adwivedi@marvell.com]
> Sent: Friday, 20 January 2023 09.41
>
> 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>
> ---
[...]
> +/** 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); \
Alternatively (just a suggestion):
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); \
(If memset() is annotated to inform the compiler that the mem pointer is constant, the compiler should generate exactly the same code.)
> +} 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)
I guess the variable sized trace entry couldn't be implemented.
Anyway, this BLOB implementation is still very useful!
Acked-by: Morten Brørup <mb@smartsharesystems.com>
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@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?
>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit@amd.com>
>Sent: Monday, January 23, 2023 10:58 PM
>To: Ankur Dwivedi <adwivedi@marvell.com>; dev@dpdk.org
>Cc: thomas@monjalon.net; david.marchand@redhat.com; mdr@ashroe.eu;
>orika@nvidia.com; chas3@att.com; humin29@huawei.com;
>linville@tuxdriver.com; ciara.loftus@intel.com; qi.z.zhang@intel.com;
>mw@semihalf.com; mk@semihalf.com; shaibran@amazon.com;
>evgenys@amazon.com; igorch@amazon.com; chandu@amd.com; Igor
>Russkikh <irusskikh@marvell.com>; shepard.siegel@atomicrules.com;
>ed.czeck@atomicrules.com; john.miller@atomicrules.com;
>ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>; Maciej Czekaj [C]
><mczekaj@marvell.com>; Shijith Thotton <sthotton@marvell.com>;
>Srisivasubramanian Srinivasan <srinivasan@marvell.com>; Harman Kalra
><hkalra@marvell.com>; rahul.lakkireddy@chelsio.com; johndale@cisco.com;
>hyonkim@cisco.com; liudongdong3@huawei.com;
>yisen.zhuang@huawei.com; xuanziyang2@huawei.com;
>cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
>simei.su@intel.com; wenjun1.wu@intel.com; qiming.yang@intel.com;
>Yuying.Zhang@intel.com; beilei.xing@intel.com; xiao.w.wang@intel.com;
>jingjing.wu@intel.com; junfeng.guo@intel.com; rosen.xu@intel.com; Nithin
>Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar Kokkilagadda
><kirankumark@marvell.com>; Sunil Kumar Kori <skori@marvell.com>; Satha
>Koteswara Rao Kottidi <skoteshwar@marvell.com>; Liron Himi
><lironh@marvell.com>; zr@semihalf.com; Radha Chintakuntla
><radhac@marvell.com>; Veerasenareddy Burru <vburru@marvell.com>;
>Sathesh B Edara <sedara@marvell.com>; matan@nvidia.com;
>viacheslavo@nvidia.com; longli@microsoft.com; spinler@cesnet.cz;
>chaoyong.he@corigine.com; niklas.soderlund@corigine.com;
>hemant.agrawal@nxp.com; sachin.saxena@oss.nxp.com; g.singh@nxp.com;
>apeksha.gupta@nxp.com; sachin.saxena@nxp.com; aboyer@pensando.io;
>Rasesh Mody <rmody@marvell.com>; Shahed Shaikh
><shshaikh@marvell.com>; Devendra Singh Rawat
><dsinghrawat@marvell.com>; andrew.rybchenko@oktetlabs.ru;
>jiawenwu@trustnetic.com; jianwang@trustnetic.com;
>jbehrens@vmware.com; maxime.coquelin@redhat.com;
>chenbo.xia@intel.com; steven.webster@windriver.com;
>matt.peters@windriver.com; bruce.richardson@intel.com;
>mtetsuyah@gmail.com; grive@u256.net; jasvinder.singh@intel.com;
>cristian.dumitrescu@intel.com; jgrajcia@cisco.com;
>mb@smartsharesystems.com
>Subject: [EXT] Re: [PATCH v6 1/6] eal: trace: add trace point emit for blob
>
>External Email
>
>----------------------------------------------------------------------
>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@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?
The actual macro is within a #ifndef __DOXYGEN__ block. I think that is the reason for including
Doxygen comments here.
>
>> #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?
The memset sets to 0 the unused bytes (RTE_TRACE_BLOB_LEN_MAX - len). So memset is done after memcpy.
>
>> +} 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()'?
Yes the register happens in this function.
>
>> #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?
Yes the export in version.map can be removed. Will remove it in next patch series.
>
>
>
>
On 1/25/2023 3:02 PM, Ankur Dwivedi wrote:
>
>>
>> ----------------------------------------------------------------------
>> 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@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?
>
> The actual macro is within a #ifndef __DOXYGEN__ block. I think that is the reason for including
> Doxygen comments here.
Thanks for confirming.
Why comments are not as part of actual macro, but there is a separate
'#ifdef __DOXYGEN__' block?
>>
>>> #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?
>
> The memset sets to 0 the unused bytes (RTE_TRACE_BLOB_LEN_MAX - len). So memset is done after memcpy.
yep, I can see what is done.
Question is, you can do more simply:
memset(mem, 0, RTE_TRACE_BLOB_LEN_MAX);
memcpy(mem, in, len);
mem = RTE_PTR_ADD(mem, RTE_TRACE_BLOB_LEN_MAX - len);
Why did you prefer the implementation you did, intentionally? If so what
is the intention, performance concerns?
btw, I want to remind that size of the 'len' can be max 64 bytes.
>>
>>> +} 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()'?
>
> Yes the register happens in this function.
You are not really answering questions.
There are three copy of '#define rte_trace_point_emit_blob(in, len)' one
of them is for doxygen comment, please explain why there are two more
copies of it?
>>
>>> #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?
>
> Yes the export in version.map can be removed. Will remove it in next patch series.
ack.
Will there be a separate patch to remove existing symbols? Although I am
not sure if it will be ABI break.
>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit@amd.com>
>Sent: Wednesday, January 25, 2023 9:40 PM
>To: Ankur Dwivedi <adwivedi@marvell.com>; dev@dpdk.org
>Cc: thomas@monjalon.net; david.marchand@redhat.com; mdr@ashroe.eu;
>orika@nvidia.com; chas3@att.com; humin29@huawei.com;
>linville@tuxdriver.com; ciara.loftus@intel.com; qi.z.zhang@intel.com;
>mw@semihalf.com; mk@semihalf.com; shaibran@amazon.com;
>evgenys@amazon.com; igorch@amazon.com; chandu@amd.com; Igor
>Russkikh <irusskikh@marvell.com>; shepard.siegel@atomicrules.com;
>ed.czeck@atomicrules.com; john.miller@atomicrules.com;
>ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>; Maciej Czekaj [C]
><mczekaj@marvell.com>; Shijith Thotton <sthotton@marvell.com>;
>Srisivasubramanian Srinivasan <srinivasan@marvell.com>; Harman Kalra
><hkalra@marvell.com>; rahul.lakkireddy@chelsio.com; johndale@cisco.com;
>hyonkim@cisco.com; liudongdong3@huawei.com;
>yisen.zhuang@huawei.com; xuanziyang2@huawei.com;
>cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
>simei.su@intel.com; wenjun1.wu@intel.com; qiming.yang@intel.com;
>Yuying.Zhang@intel.com; beilei.xing@intel.com; xiao.w.wang@intel.com;
>jingjing.wu@intel.com; junfeng.guo@intel.com; rosen.xu@intel.com; Nithin
>Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar Kokkilagadda
><kirankumark@marvell.com>; Sunil Kumar Kori <skori@marvell.com>; Satha
>Koteswara Rao Kottidi <skoteshwar@marvell.com>; Liron Himi
><lironh@marvell.com>; zr@semihalf.com; Radha Chintakuntla
><radhac@marvell.com>; Veerasenareddy Burru <vburru@marvell.com>;
>Sathesh B Edara <sedara@marvell.com>; matan@nvidia.com;
>viacheslavo@nvidia.com; longli@microsoft.com; spinler@cesnet.cz;
>chaoyong.he@corigine.com; niklas.soderlund@corigine.com;
>hemant.agrawal@nxp.com; sachin.saxena@oss.nxp.com; g.singh@nxp.com;
>apeksha.gupta@nxp.com; sachin.saxena@nxp.com; aboyer@pensando.io;
>Rasesh Mody <rmody@marvell.com>; Shahed Shaikh
><shshaikh@marvell.com>; Devendra Singh Rawat
><dsinghrawat@marvell.com>; andrew.rybchenko@oktetlabs.ru;
>jiawenwu@trustnetic.com; jianwang@trustnetic.com;
>jbehrens@vmware.com; maxime.coquelin@redhat.com;
>chenbo.xia@intel.com; steven.webster@windriver.com;
>matt.peters@windriver.com; bruce.richardson@intel.com;
>mtetsuyah@gmail.com; grive@u256.net; jasvinder.singh@intel.com;
>cristian.dumitrescu@intel.com; jgrajcia@cisco.com;
>mb@smartsharesystems.com
>Subject: Re: [EXT] Re: [PATCH v6 1/6] eal: trace: add trace point emit for blob
>
>On 1/25/2023 3:02 PM, Ankur Dwivedi wrote:
>>
>>>
>>> ---------------------------------------------------------------------
>>> - 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@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?
>>
>> The actual macro is within a #ifndef __DOXYGEN__ block. I think that
>> is the reason for including Doxygen comments here.
>
>Thanks for confirming.
>
>Why comments are not as part of actual macro, but there is a separate '#ifdef
>__DOXYGEN__' block?
The actual rte_trace_point_emit_blob macro containing the definition, is inside a #ifdef ALLOW_EXPERIMENTAL_API block, so the doxygen will not get generated for rte_trace_point_emit_blob unless ALLOW_EXPERIMENTAL_API is defined in doxygen config.
Putting the macro in #ifdef __DOXYGEN__ generates doxygen for the macro, even if ALLOW_EXPERIMENTAL_API is not defined.
>
>>>
>>>> #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?
>>
>> The memset sets to 0 the unused bytes (RTE_TRACE_BLOB_LEN_MAX - len).
>So memset is done after memcpy.
>
>yep, I can see what is done.
>
>Question is, you can do more simply:
>memset(mem, 0, RTE_TRACE_BLOB_LEN_MAX);
>memcpy(mem, in, len);
>mem = RTE_PTR_ADD(mem, RTE_TRACE_BLOB_LEN_MAX - len);
>
>Why did you prefer the implementation you did, intentionally? If so what is
>the intention, performance concerns?
Yes performance is a concern. If memset is done before memcpy, then,
64 <= number of bytes written <= 128, depending on length value from 0 to 64.
But in memset after memcpy, always 64 bytes will be written.
>
>btw, I want to remind that size of the 'len' can be max 64 bytes.
>
>>>
>>>> +} 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()'?
>>
>> Yes the register happens in this function.
>
>You are not really answering questions.
>
>There are three copy of '#define rte_trace_point_emit_blob(in, len)' one of
>them is for doxygen comment, please explain why there are two more copies
>of it?
>
The rte_trace_point_emit_blob is used when ALLOW_EXPERIMENTAL_API is defined. One definition is for that. The other is basically a null definition when ALLOW_EXPERIMENTAL_API is not defined.
>>>
>>>> #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?
>>
>> Yes the export in version.map can be removed. Will remove it in next patch
>series.
>
>ack.
>
>Will there be a separate patch to remove existing symbols? Although I am not
>sure if it will be ABI break.
I will send a separate patch to remove existing tracepoint symbols.
@@ -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;
@@ -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.
@@ -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)
@@ -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 */
@@ -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,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); \
+} 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_ */
@@ -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
@@ -440,6 +440,9 @@ EXPERIMENTAL {
rte_thread_detach;
rte_thread_equal;
rte_thread_join;
+
+ # added in 23.03
+ __rte_eal_trace_generic_blob;
};
INTERNAL {