[v4,1/6] eal: trace: add trace point emit for array

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

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Ankur Dwivedi Dec. 22, 2022, 6:33 a.m. UTC
  Adds a trace point emit function for array. The maximum array
bytes which can be captured is set to 32.

Also adds test case for emit array tracepoint function.

Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
---
 app/test/test_trace.c                      |  3 +++
 lib/eal/common/eal_common_trace_points.c   |  2 ++
 lib/eal/include/rte_eal_trace.h            |  6 ++++++
 lib/eal/include/rte_trace_point.h          | 20 ++++++++++++++++++++
 lib/eal/include/rte_trace_point_register.h |  8 ++++++++
 5 files changed, 39 insertions(+)
  

Comments

Sunil Kumar Kori Dec. 22, 2022, 9:06 a.m. UTC | #1
> -----Original Message-----
> From: Ankur Dwivedi <adwivedi@marvell.com>
> Sent: Thursday, December 22, 2022 12:03 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; david.marchand@redhat.com; mdr@ashroe.eu;
> orika@nvidia.com; ferruh.yigit@amd.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; Ankur Dwivedi
> <adwivedi@marvell.com>
> Subject: [PATCH v4 1/6] eal: trace: add trace point emit for array
> 
> Adds a trace point emit function for array. The maximum array bytes which
> can be captured is set to 32.
> 
> Also adds test case for emit array tracepoint function.
> 
> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> ---
>  app/test/test_trace.c                      |  3 +++
>  lib/eal/common/eal_common_trace_points.c   |  2 ++
>  lib/eal/include/rte_eal_trace.h            |  6 ++++++
>  lib/eal/include/rte_trace_point.h          | 20 ++++++++++++++++++++
>  lib/eal/include/rte_trace_point_register.h |  8 ++++++++
>  5 files changed, 39 insertions(+)
> 

Please make sure that version history should be maintained.

[sniped]

> diff --git a/app/test/test_trace.c b/app/test/test_trace.c index
> 6bedf14024..99cd0762d1 100644
> --- a/app/test/test_trace.c
> +++ b/app/test/test_trace.c
> --
> 2.25.1
  
Morten Brørup Dec. 22, 2022, 10:32 a.m. UTC | #2
> From: Ankur Dwivedi [mailto:adwivedi@marvell.com]
> Sent: Thursday, 22 December 2022 07.33
> 
> Adds a trace point emit function for array. The maximum array
> bytes which can be captured is set to 32.

This seems very useful.

Disclaimer: I am not familiar with the trace library or the CTF file format, so my feedback below may be completely wrong and impossible - please keep in mind when reading.

> 
> Also adds test case for emit array tracepoint function.
> 
> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> ---
>  app/test/test_trace.c                      |  3 +++
>  lib/eal/common/eal_common_trace_points.c   |  2 ++
>  lib/eal/include/rte_eal_trace.h            |  6 ++++++
>  lib/eal/include/rte_trace_point.h          | 20 ++++++++++++++++++++
>  lib/eal/include/rte_trace_point_register.h |  8 ++++++++
>  5 files changed, 39 insertions(+)
> 
> diff --git a/app/test/test_trace.c b/app/test/test_trace.c
> index 6bedf14024..99cd0762d1 100644
> --- a/app/test/test_trace.c
> +++ b/app/test/test_trace.c
> @@ -177,6 +177,7 @@ test_fp_trace_points(void)
>  static int
>  test_generic_trace_points(void)
>  {
> +	uint8_t arr[32] = {0};
>  	int tmp;
> 
>  	rte_eal_trace_generic_void();
> @@ -195,6 +196,8 @@ 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 *));

Please also test smaller, unaligned length, e.g.:
rte_eal_trace_generic_char_array(arr, 17);

Please also test variable len, unknown at build time, e.g.:
rte_eal_trace_generic_char_array(arr, rand() % 31);

> +	rte_eal_trace_generic_char_array(arr, 32);
> +	rte_eal_trace_generic_char_array(arr, 64);
>  	RTE_EAL_TRACE_GENERIC_FUNC;
> 
>  	return TEST_SUCCESS;
> diff --git a/lib/eal/common/eal_common_trace_points.c
> b/lib/eal/common/eal_common_trace_points.c
> index 0b0b254615..93fdaa634e 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_char_array,
> +	lib.eal.generic.char.array)
> 
>  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..34fdd5331f 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_char_array,
> +	RTE_TRACE_POINT_ARGS(void *in, uint8_t len),
> +	rte_trace_point_emit_char_array(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..9d9a9e0aaa 100644
> --- a/lib/eal/include/rte_trace_point.h
> +++ b/lib/eal/include/rte_trace_point.h
> @@ -144,6 +144,8 @@ _tp _args \
>  #define rte_trace_point_emit_ptr(val)
>  /** Tracepoint function payload for string datatype */
>  #define rte_trace_point_emit_string(val)
> +/** Tracepoint function payload for char array */
> +#define rte_trace_point_emit_char_array(val, len)
> 
>  #endif /* __DOXYGEN__ */
> 
> @@ -151,6 +153,8 @@ _tp _args \
>  #define __RTE_TRACE_EMIT_STRING_LEN_MAX 32
>  /** @internal Macro to define event header size. */
>  #define __RTE_TRACE_EVENT_HEADER_SZ sizeof(uint64_t)
> +/** @internal Macro to define maximum emit length of array. */
> +#define __RTE_TRACE_EMIT_ARRAY_LEN_MAX 32
> 
>  /**
>   * Enable recording events of the given tracepoint in the trace
> buffer.
> @@ -374,12 +378,28 @@ do { \
>  	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
>  } while (0)
> 
> +#define rte_trace_point_emit_char_array(in, len) \
> +do { \
> +	if (unlikely(in == NULL)) \
> +		return; \
> +	if (len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX) \
> +		return; \
> +	memcpy(mem, in, len); \
> +	mem = RTE_PTR_ADD(mem, len); \

This does not work for len < 32, because the trace format is defined to emit an array of 32 byte. You must always increment mem by 32:

mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_ARRAY_LEN_MAX); \

Also, instead of silently ignoring len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX, you should:

if (len < __RTE_TRACE_EMIT_ARRAY_LEN_MAX) { \
	memcpy(mem, in, len); \
	memset(RTE_PTR_ADD(mem, len), 0, __RTE_TRACE_EMIT_ARRAY_LEN_MAX - len); \
} else { \
	memcpy(mem, in, __RTE_TRACE_EMIT_ARRAY_LEN_MAX); \
} \

If not emitting the length value (see my comments at the end), you should clear the unused part of the array; notice the added memset().

> +} 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_char_array(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..c76fe4dd48 100644
> --- a/lib/eal/include/rte_trace_point_register.h
> +++ b/lib/eal/include/rte_trace_point_register.h
> @@ -47,6 +47,14 @@ do { \
>  		RTE_STR(in)"[32]", "string_bounded_t"); \
>  } while (0)
> 
> +#define rte_trace_point_emit_char_array(in, len) \
> +do { \
> +	RTE_SET_USED(in); \
> +	if (len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX) \
> +		return; \
> +	__rte_trace_point_emit_field(len, RTE_STR(in)"[32]", "uint8_t");
> \
> +} while (0)
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.25.1
> 

This emits 0..32 byte of binary data into a 32 byte array to the trace. But there is no way to read the length from the trace file, i.e. how many of the 32 bytes contain data.

If length is required to be constant at build time, perhaps the "[32]" in rte_trace_point_register.h can be modified to emit the length as the array size. (If so, the requirement for the length to be constant must be documented.)

Suggestion (not a requirement):

Instead of simply emitting an array, consider emitting a structure where the first field is the length, and the second field is an array of 32 bytes of data:

struct {
    integer {
        size = 8;
    } len;
    integer {
        size = 8;
    } data[32];
}

Or even better, emit a sequence [1]:

struct {
    integer {
        size = 16;
    } length;
    integer {
        size = 8;
    } data[length];
}

[1]: https://diamon.org/ctf/#spec4.2.4

If emitting a sequence, the length does not have to be limited to 32 byte, but could be up to 65535 byte.


And a personal preference about the name: I would prefer _blob instead of _char_array.
  
Ankur Dwivedi Dec. 22, 2022, 3:18 p.m. UTC | #3
Hi Morten,

Thanks for your comments and suggestions. My comments are inline.

>-----Original Message-----
>From: Morten Brørup <mb@smartsharesystems.com>
>Sent: Thursday, December 22, 2022 4:03 PM
>To: Ankur Dwivedi <adwivedi@marvell.com>; dev@dpdk.org
>Cc: thomas@monjalon.net; david.marchand@redhat.com; mdr@ashroe.eu;
>orika@nvidia.com; ferruh.yigit@amd.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;
>Jerin Jacob Kollanukkaran <jerinj@marvell.com>
>Subject: [EXT] RE: [PATCH v4 1/6] eal: trace: add trace point emit for array
>
>External Email
>
>----------------------------------------------------------------------
>> From: Ankur Dwivedi [mailto:adwivedi@marvell.com]
>> Sent: Thursday, 22 December 2022 07.33
>>
>> Adds a trace point emit function for array. The maximum array bytes
>> which can be captured is set to 32.
>
>This seems very useful.
>
>Disclaimer: I am not familiar with the trace library or the CTF file format, so my
>feedback below may be completely wrong and impossible - please keep in mind
>when reading.
>
>>
>> Also adds test case for emit array tracepoint function.
>>
>> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
>> ---
>>  app/test/test_trace.c                      |  3 +++
>>  lib/eal/common/eal_common_trace_points.c   |  2 ++
>>  lib/eal/include/rte_eal_trace.h            |  6 ++++++
>>  lib/eal/include/rte_trace_point.h          | 20 ++++++++++++++++++++
>>  lib/eal/include/rte_trace_point_register.h |  8 ++++++++
>>  5 files changed, 39 insertions(+)
>>
>> diff --git a/app/test/test_trace.c b/app/test/test_trace.c index
>> 6bedf14024..99cd0762d1 100644
>> --- a/app/test/test_trace.c
>> +++ b/app/test/test_trace.c
>> @@ -177,6 +177,7 @@ test_fp_trace_points(void)  static int
>>  test_generic_trace_points(void)
>>  {
>> +	uint8_t arr[32] = {0};
>>  	int tmp;
>>
>>  	rte_eal_trace_generic_void();
>> @@ -195,6 +196,8 @@ 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 *));
>
>Please also test smaller, unaligned length, e.g.:
>rte_eal_trace_generic_char_array(arr, 17);
Ok.
>
>Please also test variable len, unknown at build time, e.g.:
>rte_eal_trace_generic_char_array(arr, rand() % 31);
Ok.
>
>> +	rte_eal_trace_generic_char_array(arr, 32);
>> +	rte_eal_trace_generic_char_array(arr, 64);
>>  	RTE_EAL_TRACE_GENERIC_FUNC;
>>
>>  	return TEST_SUCCESS;
>> diff --git a/lib/eal/common/eal_common_trace_points.c
>> b/lib/eal/common/eal_common_trace_points.c
>> index 0b0b254615..93fdaa634e 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_char_array,
>> +	lib.eal.generic.char.array)
>>
>>  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..34fdd5331f 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_char_array,
>> +	RTE_TRACE_POINT_ARGS(void *in, uint8_t len),
>> +	rte_trace_point_emit_char_array(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..9d9a9e0aaa 100644
>> --- a/lib/eal/include/rte_trace_point.h
>> +++ b/lib/eal/include/rte_trace_point.h
>> @@ -144,6 +144,8 @@ _tp _args \
>>  #define rte_trace_point_emit_ptr(val)
>>  /** Tracepoint function payload for string datatype */  #define
>> rte_trace_point_emit_string(val)
>> +/** Tracepoint function payload for char array */ #define
>> +rte_trace_point_emit_char_array(val, len)
>>
>>  #endif /* __DOXYGEN__ */
>>
>> @@ -151,6 +153,8 @@ _tp _args \
>>  #define __RTE_TRACE_EMIT_STRING_LEN_MAX 32
>>  /** @internal Macro to define event header size. */  #define
>> __RTE_TRACE_EVENT_HEADER_SZ sizeof(uint64_t)
>> +/** @internal Macro to define maximum emit length of array. */
>> +#define __RTE_TRACE_EMIT_ARRAY_LEN_MAX 32
>>
>>  /**
>>   * Enable recording events of the given tracepoint in the trace
>> buffer.
>> @@ -374,12 +378,28 @@ do { \
>>  	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
>} while
>> (0)
>>
>> +#define rte_trace_point_emit_char_array(in, len) \ do { \
>> +	if (unlikely(in == NULL)) \
>> +		return; \
>> +	if (len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX) \
>> +		return; \
>> +	memcpy(mem, in, len); \
>> +	mem = RTE_PTR_ADD(mem, len); \
>
>This does not work for len < 32, because the trace format is defined to emit an
>array of 32 byte. You must always increment mem by 32:
I have tried smaller byte lengths such as mac address (6 bytes). It was working correctly. There were
stale values in rest of the bytes (byte 6 -byte 31).
>
>mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_ARRAY_LEN_MAX); \
>
>Also, instead of silently ignoring len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX,
>you should:
Ok.
>
>if (len < __RTE_TRACE_EMIT_ARRAY_LEN_MAX) { \
>	memcpy(mem, in, len); \
>	memset(RTE_PTR_ADD(mem, len), 0,
>__RTE_TRACE_EMIT_ARRAY_LEN_MAX - len); \ } else { \
>	memcpy(mem, in, __RTE_TRACE_EMIT_ARRAY_LEN_MAX); \ } \
>
>If not emitting the length value (see my comments at the end), you should clear
>the unused part of the array; notice the added memset().
memset will help in clearing the stale values in unused 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_char_array(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..c76fe4dd48 100644
>> --- a/lib/eal/include/rte_trace_point_register.h
>> +++ b/lib/eal/include/rte_trace_point_register.h
>> @@ -47,6 +47,14 @@ do { \
>>  		RTE_STR(in)"[32]", "string_bounded_t"); \
>>  } while (0)
>>
>> +#define rte_trace_point_emit_char_array(in, len) \
>> +do { \
>> +	RTE_SET_USED(in); \
>> +	if (len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX) \
>> +		return; \
>> +	__rte_trace_point_emit_field(len, RTE_STR(in)"[32]", "uint8_t");
>> \
>> +} while (0)
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> --
>> 2.25.1
>>
>
>This emits 0..32 byte of binary data into a 32 byte array to the trace. But there is
>no way to read the length from the trace file, i.e. how many of the 32 bytes
>contain data.
Yes, that's correct.
>
>If length is required to be constant at build time, perhaps the "[32]" in
>rte_trace_point_register.h can be modified to emit the length as the array size.
>(If so, the requirement for the length to be constant must be documented.)
>
>Suggestion (not a requirement):
>
>Instead of simply emitting an array, consider emitting a structure where the first
>field is the length, and the second field is an array of 32 bytes of data:
>
>struct {
>    integer {
>        size = 8;
>    } len;
>    integer {
>        size = 8;
>    } data[32];
>}
>
>Or even better, emit a sequence [1]:
>
>struct {
>    integer {
>        size = 16;
>    } length;
>    integer {
>        size = 8;
>    } data[length];
>}
>
>[1]: https://urldefense.proofpoint.com/v2/url?u=https-3A__diamon.org_ctf_-
>23spec4.2.4&d=DwIFAw&c=nKjWec2b6R0mOyPaz7xtfQ&r=ILjiNF3GF25y6QdHZ
>UxMl6JrStU0MIuCtO5dMzn3Ybk&m=ctL43w6fn6cvuduQqeqQ0Scs3RQQIiv5uba
>Lz9le2xyd8EJBURdSXD8IFLo7k-6H&s=vBCuOfVlqnotYr2SsovxiI7JRawt-uDVCDay-
>_B1VD4&e=
I will look into the above link.
>
>If emitting a sequence, the length does not have to be limited to 32 byte, but
>could be up to 65535 byte.
>
>
>And a personal preference about the name: I would prefer _blob instead of
>_char_array.
Will think about this.
  

Patch

diff --git a/app/test/test_trace.c b/app/test/test_trace.c
index 6bedf14024..99cd0762d1 100644
--- a/app/test/test_trace.c
+++ b/app/test/test_trace.c
@@ -177,6 +177,7 @@  test_fp_trace_points(void)
 static int
 test_generic_trace_points(void)
 {
+	uint8_t arr[32] = {0};
 	int tmp;
 
 	rte_eal_trace_generic_void();
@@ -195,6 +196,8 @@  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_char_array(arr, 32);
+	rte_eal_trace_generic_char_array(arr, 64);
 	RTE_EAL_TRACE_GENERIC_FUNC;
 
 	return TEST_SUCCESS;
diff --git a/lib/eal/common/eal_common_trace_points.c b/lib/eal/common/eal_common_trace_points.c
index 0b0b254615..93fdaa634e 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_char_array,
+	lib.eal.generic.char.array)
 
 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..34fdd5331f 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_char_array,
+	RTE_TRACE_POINT_ARGS(void *in, uint8_t len),
+	rte_trace_point_emit_char_array(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..9d9a9e0aaa 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -144,6 +144,8 @@  _tp _args \
 #define rte_trace_point_emit_ptr(val)
 /** Tracepoint function payload for string datatype */
 #define rte_trace_point_emit_string(val)
+/** Tracepoint function payload for char array */
+#define rte_trace_point_emit_char_array(val, len)
 
 #endif /* __DOXYGEN__ */
 
@@ -151,6 +153,8 @@  _tp _args \
 #define __RTE_TRACE_EMIT_STRING_LEN_MAX 32
 /** @internal Macro to define event header size. */
 #define __RTE_TRACE_EVENT_HEADER_SZ sizeof(uint64_t)
+/** @internal Macro to define maximum emit length of array. */
+#define __RTE_TRACE_EMIT_ARRAY_LEN_MAX 32
 
 /**
  * Enable recording events of the given tracepoint in the trace buffer.
@@ -374,12 +378,28 @@  do { \
 	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 } while (0)
 
+#define rte_trace_point_emit_char_array(in, len) \
+do { \
+	if (unlikely(in == NULL)) \
+		return; \
+	if (len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX) \
+		return; \
+	memcpy(mem, in, len); \
+	mem = RTE_PTR_ADD(mem, 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_char_array(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..c76fe4dd48 100644
--- a/lib/eal/include/rte_trace_point_register.h
+++ b/lib/eal/include/rte_trace_point_register.h
@@ -47,6 +47,14 @@  do { \
 		RTE_STR(in)"[32]", "string_bounded_t"); \
 } while (0)
 
+#define rte_trace_point_emit_char_array(in, len) \
+do { \
+	RTE_SET_USED(in); \
+	if (len > __RTE_TRACE_EMIT_ARRAY_LEN_MAX) \
+		return; \
+	__rte_trace_point_emit_field(len, RTE_STR(in)"[32]", "uint8_t"); \
+} while (0)
+
 #ifdef __cplusplus
 }
 #endif