[v5,1/6] eal: trace: add trace point emit for blob
Checks
Commit Message
Adds a trace point emit function for emitting a blob. The maximum blob
bytes which can be captured is maximum value contained in uint16_t,
which is 65535.
Also adds test case for emit array tracepoint function.
Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
---
app/test/test_trace.c | 5 +++++
lib/eal/common/eal_common_trace_points.c | 2 ++
lib/eal/include/rte_eal_trace.h | 6 ++++++
lib/eal/include/rte_trace_point.h | 17 +++++++++++++++++
lib/eal/include/rte_trace_point_register.h | 7 +++++++
lib/eal/version.map | 3 +++
6 files changed, 40 insertions(+)
Comments
> From: Ankur Dwivedi [mailto:adwivedi@marvell.com]
> Sent: Thursday, 12 January 2023 12.22
>
> Adds a trace point emit function for emitting a blob. The maximum blob
> bytes which can be captured is maximum value contained in uint16_t,
> which is 65535.
>
> Also adds test case for emit array tracepoint function.
>
> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> ---
Excellent, thank you.
[...]
> +#define rte_trace_point_emit_blob(in, len) \
> +do { \
> + if (unlikely(in == NULL)) \
> + return; \
> + __rte_trace_point_emit(len, uint16_t); \
> + memcpy(mem, in, len); \
> + mem = RTE_PTR_ADD(mem, len); \
> +} while (0)
I am somewhat unsure about my concerns here, so please forgive me if they are invalid...
Is rte_trace_point_emit_blob() always called with "len" being a variable, then this is OK.
If "len" can be a non-constant formula (e.g. len++), you need a temporary variable:
#define rte_trace_point_emit_blob(in, len) \
do { \
uint16_t _len = len; \
if (unlikely(in == NULL)) \
return; \
__rte_trace_point_emit(_len, uint16_t); \
memcpy(mem, in, _len); \
mem = RTE_PTR_ADD(_mem, _len); \
} while (0)
But I don't think this can ever happen.
However, if "len" can be a constant (e.g. 6), does __rte_trace_point_emit(len, uint16_t) accept it? (The __rte_trace_point_emit() macro is shown below.) A constant has no pointer to it (i.e. &6 does not exist).
Looking deeper at it, I'm afraid this question can be generalized to all the existing macros/functions calling __rte_trace_point_emit().
And now that I have hijacked your patch with a generalized question, I wonder if the existing __rte_trace_point_emit() has a bug? It uses sizeof(in), but I think it should use sizeof(type).
It looks like this:
#define __rte_trace_point_emit(in, type) \
do { \
memcpy(mem, &(in), sizeof(in)); \
mem = RTE_PTR_ADD(mem, sizeof(in)); \
} while (0)
Alternatively, __rte_trace_point_emit() should RTE_BUILD_BUG_ON(typeof(in) != type).
If my concerns above are invalid, then:
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Hi Morten,
My comments are inline.
>-----Original Message-----
>From: Morten Brørup <mb@smartsharesystems.com>
>Sent: Thursday, January 12, 2023 6:09 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
>Subject: [EXT] RE: [PATCH v5 1/6] eal: trace: add trace point emit for blob
>
>External Email
>
>----------------------------------------------------------------------
>> From: Ankur Dwivedi [mailto:adwivedi@marvell.com]
>> Sent: Thursday, 12 January 2023 12.22
>>
>> Adds a trace point emit function for emitting a blob. The maximum blob
>> bytes which can be captured is maximum value contained in uint16_t,
>> which is 65535.
>>
>> Also adds test case for emit array tracepoint function.
>>
>> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
>> ---
>
>Excellent, thank you.
>
>[...]
>
>> +#define rte_trace_point_emit_blob(in, len) \ do { \
>> + if (unlikely(in == NULL)) \
>> + return; \
>> + __rte_trace_point_emit(len, uint16_t); \
>> + memcpy(mem, in, len); \
>> + mem = RTE_PTR_ADD(mem, len); \
>> +} while (0)
>
>I am somewhat unsure about my concerns here, so please forgive me if they are
>invalid...
>
>Is rte_trace_point_emit_blob() always called with "len" being a variable, then
>this is OK.
Yes rte_trace_point_emit_blob is always called with len being a variable.
>
>If "len" can be a non-constant formula (e.g. len++), you need a temporary
>variable:
>
>#define rte_trace_point_emit_blob(in, len) \ do { \
> uint16_t _len = len; \
> if (unlikely(in == NULL)) \
> return; \
> __rte_trace_point_emit(_len, uint16_t); \
> memcpy(mem, in, _len); \
> mem = RTE_PTR_ADD(_mem, _len); \
>} while (0)
>
>But I don't think this can ever happen.
Yes, I think the same.
>
>However, if "len" can be a constant (e.g. 6), does __rte_trace_point_emit(len,
>uint16_t) accept it? (The __rte_trace_point_emit() macro is shown below.) A
>constant has no pointer to it (i.e. &6 does not exist).
>
>Looking deeper at it, I'm afraid this question can be generalized to all the
>existing macros/functions calling __rte_trace_point_emit().
>
>And now that I have hijacked your patch with a generalized question, I wonder
>if the existing __rte_trace_point_emit() has a bug? It uses sizeof(in), but I think
>it should use sizeof(type).
>
>It looks like this:
>
>#define __rte_trace_point_emit(in, type) \ do { \
> memcpy(mem, &(in), sizeof(in)); \
> mem = RTE_PTR_ADD(mem, sizeof(in)); \
>} while (0)
>
>Alternatively, __rte_trace_point_emit() should RTE_BUILD_BUG_ON(typeof(in)
>!= type).
Yes there would be compilation error if typeof(in) is not same as type.
>
>
>If my concerns above are invalid, then:
>
>Acked-by: Morten Brørup <mb@smartsharesystems.com>
> -----Original Message-----
> From: Ankur Dwivedi <adwivedi@marvell.com>
> Sent: Thursday, January 12, 2023 4:52 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;
> mb@smartsharesystems.com; Ankur Dwivedi <adwivedi@marvell.com>
> Subject: [PATCH v5 1/6] eal: trace: add trace point emit for blob
>
> Adds a trace point emit function for emitting a blob. The maximum blob
> bytes which can be captured is maximum value contained in uint16_t, which
> is 65535.
>
> Also adds test case for emit array tracepoint function.
>
> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> ---
> app/test/test_trace.c | 5 +++++
> lib/eal/common/eal_common_trace_points.c | 2 ++
> lib/eal/include/rte_eal_trace.h | 6 ++++++
> lib/eal/include/rte_trace_point.h | 17 +++++++++++++++++
> lib/eal/include/rte_trace_point_register.h | 7 +++++++
> lib/eal/version.map | 3 +++
> 6 files changed, 40 insertions(+)
>
> diff --git a/app/test/test_trace.c b/app/test/test_trace.c index
> 6bedf14024..cf781dc25b 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,6 +178,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 +197,9 @@ 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, 17);
Do we need this test case now as (rte_rand % 32) will make sure to validate array of valid size only i.e. 0-31 ?
> + rte_eal_trace_generic_blob(arr, 32);
> + rte_eal_trace_generic_blob(arr, rte_rand() % 32);
When executed trace_autotest then I didn't find 3rd generic.blob trace point.
Can you please explain the behavior ?
> 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..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..02e3a564a1 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, uint16_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..21b6bf7bf6 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_blob(val, len)
>
> #endif /* __DOXYGEN__ */
>
> @@ -374,12 +376,27 @@ 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; \
> + __rte_trace_point_emit(len, uint16_t); \
Just to confirm that is there any typo meaning "in should be used in place of len here" ?
> + memcpy(mem, in, len); \
> + mem = RTE_PTR_ADD(mem, len); \
Also rte_trace_point_emit copies and increment the memory pointer itself. Then what is the purpose of above 2 instructions ?
> +} 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..a54f9769da 100644
> --- a/lib/eal/include/rte_trace_point_register.h
> +++ b/lib/eal/include/rte_trace_point_register.h
> @@ -47,6 +47,13 @@ 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, uint16_t); \
> + __rte_trace_point_emit_field(len, RTE_STR(in)"["#len"]",
> +RTE_STR(uint8_t)); \ } while (0)
> +
> #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;
> };
>
> INTERNAL {
> --
> 2.25.1
One more observation that the size of lib.eal.generic.blob trace point is 31962 bytes.
Is this trace point this much big or some bug ?
> -----Original Message-----
> From: Sunil Kumar Kori
> Sent: Thursday, January 12, 2023 10:00 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>; 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; Ankur Dwivedi <adwivedi@marvell.com>
> Subject: RE: [PATCH v5 1/6] eal: trace: add trace point emit for blob
>
> > -----Original Message-----
> > From: Ankur Dwivedi <adwivedi@marvell.com>
> > Sent: Thursday, January 12, 2023 4:52 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; mb@smartsharesystems.com; Ankur Dwivedi
> > <adwivedi@marvell.com>
> > Subject: [PATCH v5 1/6] eal: trace: add trace point emit for blob
> >
> > Adds a trace point emit function for emitting a blob. The maximum blob
> > bytes which can be captured is maximum value contained in uint16_t,
> > which is 65535.
> >
> > Also adds test case for emit array tracepoint function.
> >
> > Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> > ---
> > app/test/test_trace.c | 5 +++++
> > lib/eal/common/eal_common_trace_points.c | 2 ++
> > lib/eal/include/rte_eal_trace.h | 6 ++++++
> > lib/eal/include/rte_trace_point.h | 17 +++++++++++++++++
> > lib/eal/include/rte_trace_point_register.h | 7 +++++++
> > lib/eal/version.map | 3 +++
> > 6 files changed, 40 insertions(+)
> >
> > diff --git a/app/test/test_trace.c b/app/test/test_trace.c index
> > 6bedf14024..cf781dc25b 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,6 +178,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 +197,9 @@ 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, 17);
> Do we need this test case now as (rte_rand % 32) will make sure to validate
> array of valid size only i.e. 0-31 ?
>
> > + rte_eal_trace_generic_blob(arr, 32);
> > + rte_eal_trace_generic_blob(arr, rte_rand() % 32);
> When executed trace_autotest then I didn't find 3rd generic.blob trace point.
> Can you please explain the behavior ?
>
> > 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..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..02e3a564a1 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, uint16_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..21b6bf7bf6 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_blob(val, len)
> >
> > #endif /* __DOXYGEN__ */
> >
> > @@ -374,12 +376,27 @@ 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; \
> > + __rte_trace_point_emit(len, uint16_t); \
> Just to confirm that is there any typo meaning "in should be used in place of
> len here" ?
>
> > + memcpy(mem, in, len); \
> > + mem = RTE_PTR_ADD(mem, len); \
> Also rte_trace_point_emit copies and increment the memory pointer itself.
> Then what is the purpose of above 2 instructions ?
>
> > +} 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..a54f9769da 100644
> > --- a/lib/eal/include/rte_trace_point_register.h
> > +++ b/lib/eal/include/rte_trace_point_register.h
> > @@ -47,6 +47,13 @@ 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, uint16_t); \
> > + __rte_trace_point_emit_field(len, RTE_STR(in)"["#len"]",
> > +RTE_STR(uint8_t)); \ } while (0)
> > +
> > #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;
> > };
> >
> > INTERNAL {
> > --
> > 2.25.1
@@ -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,6 +178,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 +197,9 @@ 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, 17);
+ rte_eal_trace_generic_blob(arr, 32);
+ rte_eal_trace_generic_blob(arr, rte_rand() % 32);
RTE_EAL_TRACE_GENERIC_FUNC;
return TEST_SUCCESS;
@@ -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, uint16_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,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_blob(val, len)
#endif /* __DOXYGEN__ */
@@ -374,12 +376,27 @@ 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; \
+ __rte_trace_point_emit(len, uint16_t); \
+ 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_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,13 @@ 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, uint16_t); \
+ __rte_trace_point_emit_field(len, RTE_STR(in)"["#len"]", 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 {