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

[Sunil Kumar Kori]

> -----Original Message-----
> From: Ankur Dwivedi <adwivedi at marvell.com>
> Sent: Monday, January 23, 2023 2:32 PM
> To: dev at dpdk.org
> Cc: thomas at monjalon.net; david.marchand at redhat.com; mdr at ashroe.eu;
> orika at nvidia.com; ferruh.yigit at amd.com; chas3 at att.com;
> humin29 at huawei.com; linville at tuxdriver.com; ciara.loftus at intel.com;
> qi.z.zhang at intel.com; mw at semihalf.com; mk at semihalf.com;
> shaibran at amazon.com; evgenys at amazon.com; igorch at amazon.com;
> chandu at amd.com; Igor Russkikh <irusskikh at marvell.com>;
> shepard.siegel at atomicrules.com; ed.czeck at atomicrules.com;
> john.miller at atomicrules.com; ajit.khaparde at broadcom.com;
> somnath.kotur at broadcom.com; Jerin Jacob Kollanukkaran
> <jerinj at marvell.com>; Maciej Czekaj [C] <mczekaj at marvell.com>; Shijith
> Thotton <sthotton at marvell.com>; Srisivasubramanian Srinivasan
> <srinivasan at marvell.com>; Harman Kalra <hkalra at marvell.com>;
> rahul.lakkireddy at chelsio.com; johndale at cisco.com; hyonkim at cisco.com;
> liudongdong3 at huawei.com; yisen.zhuang at huawei.com;
> xuanziyang2 at huawei.com; cloud.wangxiaoyun at huawei.com;
> zhouguoyang at huawei.com; simei.su at intel.com; wenjun1.wu at intel.com;
> qiming.yang at intel.com; Yuying.Zhang at intel.com; beilei.xing at intel.com;
> xiao.w.wang at intel.com; jingjing.wu at intel.com; junfeng.guo at intel.com;
> rosen.xu at intel.com; Nithin Kumar Dabilpuram
> <ndabilpuram at marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark at marvell.com>; Sunil Kumar Kori <skori at marvell.com>; Satha
> Koteswara Rao Kottidi <skoteshwar at marvell.com>; Liron Himi
> <lironh at marvell.com>; zr at semihalf.com; Radha Chintakuntla
> <radhac at marvell.com>; Veerasenareddy Burru <vburru at marvell.com>;
> Sathesh B Edara <sedara at marvell.com>; matan at nvidia.com;
> viacheslavo at nvidia.com; longli at microsoft.com; spinler at cesnet.cz;
> chaoyong.he at corigine.com; niklas.soderlund at corigine.com;
> hemant.agrawal at nxp.com; sachin.saxena at oss.nxp.com; g.singh at nxp.com;
> apeksha.gupta at nxp.com; sachin.saxena at nxp.com; aboyer at pensando.io;
> Rasesh Mody <rmody at marvell.com>; Shahed Shaikh
> <shshaikh at marvell.com>; Devendra Singh Rawat
> <dsinghrawat at marvell.com>; andrew.rybchenko at oktetlabs.ru;
> jiawenwu at trustnetic.com; jianwang at trustnetic.com;
> jbehrens at vmware.com; maxime.coquelin at redhat.com;
> chenbo.xia at intel.com; steven.webster at windriver.com;
> matt.peters at windriver.com; bruce.richardson at intel.com;
> mtetsuyah at gmail.com; grive at u256.net; jasvinder.singh at intel.com;
> cristian.dumitrescu at intel.com; jgrajcia at cisco.com;
> mb at smartsharesystems.com; Ankur Dwivedi <adwivedi at marvell.com>
> Subject: [PATCH v7 1/6] eal: trace: add trace point emit for blob
> 
> Adds a trace point emit function for capturing a blob. The blob captures the
> length passed by the application followed by the array.
> 
> The maximum blob bytes which can be captured is bounded by
> RTE_TRACE_BLOB_LEN_MAX macro. The value for max blob length macro is
> 64 bytes. If the length is less than 64 the remaining trailing bytes are set to
> zero.
> 
> This patch also adds test case for emit blob tracepoint function.
> 
> Signed-off-by: Ankur Dwivedi <adwivedi at marvell.com>
> ---
>  app/test/test_trace.c                      | 11 ++++++++
>  doc/guides/prog_guide/trace_lib.rst        | 12 +++++++++
>  lib/eal/common/eal_common_trace_points.c   |  2 ++
>  lib/eal/include/rte_eal_trace.h            |  6 +++++
>  lib/eal/include/rte_trace_point.h          | 31 ++++++++++++++++++++++
>  lib/eal/include/rte_trace_point_register.h |  9 +++++++
>  lib/eal/version.map                        |  3 +++
>  7 files changed, 74 insertions(+)
> 
Acked-by: Sunil Kumar Kori <skori at marvell.com>

> 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);
> +)
> +

As per documentation rte_eal_trace_generic_blob() will emit 64 bytes only i.e. input array cannot be other than uint8_t.
So will it not be better to make it more readable like RTE_TRACE_POINT_ARGS(uint8_t *in, uint8_t len) instead of using void *in.

Rest is fine. Already acked above. 

>  #define RTE_EAL_TRACE_GENERIC_FUNC
> rte_eal_trace_generic_func(__func__)
> 
>  /* Interrupt */

[snipped]

> 2.25.1