[v1,12/32] eal/trace: implement registration payload

Message ID 20200318190241.3150971-13-jerinj@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series DPDK Trace support |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Jerin Jacob Kollanukkaran March 18, 2020, 7:02 p.m. UTC
  From: Jerin Jacob <jerinj@marvell.com>

The trace function payloads such as rte_trace_ctf_* have
dual functions. The first to emit the payload for the registration
function and the second one to act as trace memory emitters.

When it used as registration payload, it will do the following to
fulfill the registration job.
- Find out the size of the event
- Generate metadata field string using __rte_trace_emit_ctf_field().

Signed-off-by: Jerin Jacob <jerinj@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 lib/librte_eal/common/eal_common_trace.c      | 19 +++++++
 lib/librte_eal/common/include/rte_trace.h     | 20 ++++++++
 .../common/include/rte_trace_register.h       | 50 +++++++++++++++++++
 lib/librte_eal/rte_eal_version.map            |  2 +
 4 files changed, 91 insertions(+)
  

Comments

Mattias Rönnblom March 19, 2020, 7:15 p.m. UTC | #1
On 2020-03-18 20:02, jerinj@marvell.com wrote:
> From: Jerin Jacob <jerinj@marvell.com>
>
> The trace function payloads such as rte_trace_ctf_* have
> dual functions. The first to emit the payload for the registration
> function and the second one to act as trace memory emitters.
>
> When it used as registration payload, it will do the following to
> fulfill the registration job.
> - Find out the size of the event
> - Generate metadata field string using __rte_trace_emit_ctf_field().
>
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
>   lib/librte_eal/common/eal_common_trace.c      | 19 +++++++
>   lib/librte_eal/common/include/rte_trace.h     | 20 ++++++++
>   .../common/include/rte_trace_register.h       | 50 +++++++++++++++++++
>   lib/librte_eal/rte_eal_version.map            |  2 +
>   4 files changed, 91 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_trace.c b/lib/librte_eal/common/eal_common_trace.c
> index 6cb724080..94d3da0d3 100644
> --- a/lib/librte_eal/common/eal_common_trace.c
> +++ b/lib/librte_eal/common/eal_common_trace.c
> @@ -493,6 +493,25 @@ trace_mem_per_thread_free(void)
>   	rte_spinlock_unlock(&trace->lock);
>   }
>   
> +void
> +__rte_trace_emit_ctf_field(size_t sz, const char *in, const char *datatype)
> +{
> +	char *field = RTE_PER_LCORE(ctf_field);
> +	int count = RTE_PER_LCORE(ctf_count);
> +	int rc;
> +
> +	RTE_PER_LCORE(trace_point_sz) += sz;
> +	rc = snprintf(RTE_PTR_ADD(field, count),
> +		      RTE_MAX(0, TRACE_CTF_FIELD_SIZE - 1 - count),
> +		      "%s %s;", datatype, in);
> +	if (rc <= 0) {
> +		RTE_PER_LCORE(trace_point_sz) = 0;
> +		trace_crit("CTF field is too long");
> +		return;
> +	}
> +	RTE_PER_LCORE(ctf_count) += rc;
> +}
> +
>   int
>   __rte_trace_point_register(rte_trace_t handle, const char *name, uint32_t level,
>   			 void (*fn)(void))
> diff --git a/lib/librte_eal/common/include/rte_trace.h b/lib/librte_eal/common/include/rte_trace.h
> index 358b1b7ca..c24fe8d66 100644
> --- a/lib/librte_eal/common/include/rte_trace.h
> +++ b/lib/librte_eal/common/include/rte_trace.h
> @@ -520,6 +520,8 @@ _tp _args \
>   
>   /** @internal Macro to define maximum emit length of string datatype. */
>   #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 @warning
> @@ -553,6 +555,24 @@ void __rte_trace_mem_per_thread_alloc(void);
>   __rte_experimental
>   int __rte_trace_point_register(rte_trace_t trace, const char *name,
>   			     uint32_t level, void (*fn)(void));
> +/**
> + * @internal @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Helper function to emit ctf field.
> + *
> + * @param sz
> + *   The tracepoint size.
> + * @param field
> + *   The name of the trace event.
> + * @param type
> + *   The datatype of the trace event as string.
> + * @return
> + *   - 0: Success.
> + *   - <0: Failure.
> + */
> +__rte_experimental
> +void __rte_trace_emit_ctf_field(size_t sz, const char *field, const char *type);
>   
>   #ifdef RTE_TRACE_POINT_REGISTER_SELECT
>   #include <rte_trace_register.h>
> diff --git a/lib/librte_eal/common/include/rte_trace_register.h b/lib/librte_eal/common/include/rte_trace_register.h
> index e9940b414..93d6396df 100644
> --- a/lib/librte_eal/common/include/rte_trace_register.h
> +++ b/lib/librte_eal/common/include/rte_trace_register.h
> @@ -10,6 +10,7 @@
>   #define _RTE_TRACE_REGISTER_H_
>   
>   #include <rte_per_lcore.h>
> +#include <rte_log.h>
>   
>   RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz);
>   
> @@ -17,4 +18,53 @@ RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz);
>   	__rte_trace_point_register(&__##trace, RTE_STR(name),\
>   			RTE_LOG_ ## level, (void (*)(void)) trace)
>   
> +#define __rte_trace_emit_header_generic(t)\
> +	RTE_PER_LCORE(trace_point_sz) = __RTE_TRACE_EVENT_HEADER_SZ
> +
> +#define __rte_trace_emit_header_dp(t) __rte_trace_emit_header_generic(t)
> +
> +#define rte_trace_ctf_u64(in)\
> +	RTE_BUILD_BUG_ON(sizeof(uint64_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(uint64_t), RTE_STR(in), "uint64_t")

Delegate to a generic macro, to which you pass the type and the "in" 
parameter.

> +#define rte_trace_ctf_i64(in)\
> +	RTE_BUILD_BUG_ON(sizeof(int64_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(int64_t), RTE_STR(in), "int64_t")
> +#define rte_trace_ctf_u32(in)\
> +	RTE_BUILD_BUG_ON(sizeof(uint32_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(uint32_t), RTE_STR(in), "uint32_t")
> +#define rte_trace_ctf_i32(in)\
> +	RTE_BUILD_BUG_ON(sizeof(int32_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(int32_t), RTE_STR(in), "int32_t")
> +#define rte_trace_ctf_u16(in)\
> +	RTE_BUILD_BUG_ON(sizeof(uint16_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(uint16_t), RTE_STR(in), "uint16_t")
> +#define rte_trace_ctf_i16(in)\
> +	RTE_BUILD_BUG_ON(sizeof(int16_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(int16_t), RTE_STR(in), "int16_t")
> +#define rte_trace_ctf_u8(in)\
> +	RTE_BUILD_BUG_ON(sizeof(uint8_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(uint8_t), RTE_STR(in), "uint8_t")
> +#define rte_trace_ctf_i8(in)\
> +	RTE_BUILD_BUG_ON(sizeof(int8_t) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(int8_t), RTE_STR(in), "int8_t")
> +#define rte_trace_ctf_int(in)\
> +	RTE_BUILD_BUG_ON(sizeof(int) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(int), RTE_STR(in), "int32_t")
> +#define rte_trace_ctf_long(in)\
> +	RTE_BUILD_BUG_ON(sizeof(long) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(long), RTE_STR(in), "long")
> +#define rte_trace_ctf_float(in)\
> +	RTE_BUILD_BUG_ON(sizeof(float) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(float), RTE_STR(in), "float")
> +#define rte_trace_ctf_double(in)\
> +	RTE_BUILD_BUG_ON(sizeof(double) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(double), RTE_STR(in), "double")
> +#define rte_trace_ctf_ptr(in)\
> +	RTE_BUILD_BUG_ON(sizeof(void *) != sizeof(typeof(in)));\
> +	__rte_trace_emit_ctf_field(sizeof(void *), RTE_STR(in), "uintptr_t")
> +#define rte_trace_ctf_string(in)\
> +	RTE_SET_USED(in);\
> +	__rte_trace_emit_ctf_field(__RTE_TRACE_EMIT_STRING_LEN_MAX,\
> +				   RTE_STR(in)"[32]", "string_bounded_t")
> +
>   #endif /* _RTE_TRACE_REGISTER_H_ */
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index f56d1867e..0c787302f 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -339,7 +339,9 @@ EXPERIMENTAL {
>   	# added in 20.05
>   	rte_thread_getname;
>   	__rte_trace_mem_per_thread_alloc;
> +	__rte_trace_emit_ctf_field;
>   	__rte_trace_point_register;
> +	per_lcore_trace_point_sz;
>   	per_lcore_trace_mem;
>   	rte_trace_global_is_enabled;
>   	rte_trace_global_is_disabled;
  
Jerin Jacob March 23, 2020, 9:24 a.m. UTC | #2
On Fri, Mar 20, 2020 at 12:45 AM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-03-18 20:02, jerinj@marvell.com wrote:
> > From: Jerin Jacob <jerinj@marvell.com>
> >

> > +
> > +#define rte_trace_ctf_u64(in)\
> > +     RTE_BUILD_BUG_ON(sizeof(uint64_t) != sizeof(typeof(in)));\
> > +     __rte_trace_emit_ctf_field(sizeof(uint64_t), RTE_STR(in), "uint64_t")
>
> Delegate to a generic macro, to which you pass the type and the "in"
> parameter.

Not delegated to have a generic patch to catch the following error in
compile time.

uint32_t val = 12;

rte_trace_ctf_u64(val)
  
Mattias Rönnblom March 23, 2020, 10:18 a.m. UTC | #3
On 2020-03-23 10:24, Jerin Jacob wrote:
> On Fri, Mar 20, 2020 at 12:45 AM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-03-18 20:02, jerinj@marvell.com wrote:
>>> From: Jerin Jacob <jerinj@marvell.com>
>>>
>>> +
>>> +#define rte_trace_ctf_u64(in)\
>>> +     RTE_BUILD_BUG_ON(sizeof(uint64_t) != sizeof(typeof(in)));\
>>> +     __rte_trace_emit_ctf_field(sizeof(uint64_t), RTE_STR(in), "uint64_t")
>> Delegate to a generic macro, to which you pass the type and the "in"
>> parameter.
> Not delegated to have a generic patch to catch the following error in
> compile time.
>
> uint32_t val = 12;
>
> rte_trace_ctf_u64(val)

If you pass the type as I suggested, is there something preventing 
generating exactly the same code as the non-delegating macros?


#define __rte_trace_cft(in, in_type) \

     RTE_BUILD_BUG_ON(sizeof(in_type), != sizeof(typeof(in)));

     __rte_trace_emit_cft_field(sizeof(in_type), RTE_STR(in), 
RTE_STR(in_type))

#define rte_trace_cft_u64(in) \

     __rte_trace_ctf(in, uint64_t)
  
Jerin Jacob March 23, 2020, 10:50 a.m. UTC | #4
On Mon, Mar 23, 2020 at 3:48 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-03-23 10:24, Jerin Jacob wrote:
> > On Fri, Mar 20, 2020 at 12:45 AM Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com> wrote:
> >> On 2020-03-18 20:02, jerinj@marvell.com wrote:
> >>> From: Jerin Jacob <jerinj@marvell.com>
> >>>
> >>> +
> >>> +#define rte_trace_ctf_u64(in)\
> >>> +     RTE_BUILD_BUG_ON(sizeof(uint64_t) != sizeof(typeof(in)));\
> >>> +     __rte_trace_emit_ctf_field(sizeof(uint64_t), RTE_STR(in), "uint64_t")
> >> Delegate to a generic macro, to which you pass the type and the "in"
> >> parameter.
> > Not delegated to have a generic patch to catch the following error in
> > compile time.
> >
> > uint32_t val = 12;
> >
> > rte_trace_ctf_u64(val)
>
> If you pass the type as I suggested, is there something preventing
> generating exactly the same code as the non-delegating macros?
>
>
> #define __rte_trace_cft(in, in_type) \
>
>      RTE_BUILD_BUG_ON(sizeof(in_type), != sizeof(typeof(in)));
>
>      __rte_trace_emit_cft_field(sizeof(in_type), RTE_STR(in),
> RTE_STR(in_type))
>
> #define rte_trace_cft_u64(in) \
>
>      __rte_trace_ctf(in, uint64_t)

This will work. I will change to this scheme in v2.



>
>
  

Patch

diff --git a/lib/librte_eal/common/eal_common_trace.c b/lib/librte_eal/common/eal_common_trace.c
index 6cb724080..94d3da0d3 100644
--- a/lib/librte_eal/common/eal_common_trace.c
+++ b/lib/librte_eal/common/eal_common_trace.c
@@ -493,6 +493,25 @@  trace_mem_per_thread_free(void)
 	rte_spinlock_unlock(&trace->lock);
 }
 
+void
+__rte_trace_emit_ctf_field(size_t sz, const char *in, const char *datatype)
+{
+	char *field = RTE_PER_LCORE(ctf_field);
+	int count = RTE_PER_LCORE(ctf_count);
+	int rc;
+
+	RTE_PER_LCORE(trace_point_sz) += sz;
+	rc = snprintf(RTE_PTR_ADD(field, count),
+		      RTE_MAX(0, TRACE_CTF_FIELD_SIZE - 1 - count),
+		      "%s %s;", datatype, in);
+	if (rc <= 0) {
+		RTE_PER_LCORE(trace_point_sz) = 0;
+		trace_crit("CTF field is too long");
+		return;
+	}
+	RTE_PER_LCORE(ctf_count) += rc;
+}
+
 int
 __rte_trace_point_register(rte_trace_t handle, const char *name, uint32_t level,
 			 void (*fn)(void))
diff --git a/lib/librte_eal/common/include/rte_trace.h b/lib/librte_eal/common/include/rte_trace.h
index 358b1b7ca..c24fe8d66 100644
--- a/lib/librte_eal/common/include/rte_trace.h
+++ b/lib/librte_eal/common/include/rte_trace.h
@@ -520,6 +520,8 @@  _tp _args \
 
 /** @internal Macro to define maximum emit length of string datatype. */
 #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 @warning
@@ -553,6 +555,24 @@  void __rte_trace_mem_per_thread_alloc(void);
 __rte_experimental
 int __rte_trace_point_register(rte_trace_t trace, const char *name,
 			     uint32_t level, void (*fn)(void));
+/**
+ * @internal @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Helper function to emit ctf field.
+ *
+ * @param sz
+ *   The tracepoint size.
+ * @param field
+ *   The name of the trace event.
+ * @param type
+ *   The datatype of the trace event as string.
+ * @return
+ *   - 0: Success.
+ *   - <0: Failure.
+ */
+__rte_experimental
+void __rte_trace_emit_ctf_field(size_t sz, const char *field, const char *type);
 
 #ifdef RTE_TRACE_POINT_REGISTER_SELECT
 #include <rte_trace_register.h>
diff --git a/lib/librte_eal/common/include/rte_trace_register.h b/lib/librte_eal/common/include/rte_trace_register.h
index e9940b414..93d6396df 100644
--- a/lib/librte_eal/common/include/rte_trace_register.h
+++ b/lib/librte_eal/common/include/rte_trace_register.h
@@ -10,6 +10,7 @@ 
 #define _RTE_TRACE_REGISTER_H_
 
 #include <rte_per_lcore.h>
+#include <rte_log.h>
 
 RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz);
 
@@ -17,4 +18,53 @@  RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz);
 	__rte_trace_point_register(&__##trace, RTE_STR(name),\
 			RTE_LOG_ ## level, (void (*)(void)) trace)
 
+#define __rte_trace_emit_header_generic(t)\
+	RTE_PER_LCORE(trace_point_sz) = __RTE_TRACE_EVENT_HEADER_SZ
+
+#define __rte_trace_emit_header_dp(t) __rte_trace_emit_header_generic(t)
+
+#define rte_trace_ctf_u64(in)\
+	RTE_BUILD_BUG_ON(sizeof(uint64_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(uint64_t), RTE_STR(in), "uint64_t")
+#define rte_trace_ctf_i64(in)\
+	RTE_BUILD_BUG_ON(sizeof(int64_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(int64_t), RTE_STR(in), "int64_t")
+#define rte_trace_ctf_u32(in)\
+	RTE_BUILD_BUG_ON(sizeof(uint32_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(uint32_t), RTE_STR(in), "uint32_t")
+#define rte_trace_ctf_i32(in)\
+	RTE_BUILD_BUG_ON(sizeof(int32_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(int32_t), RTE_STR(in), "int32_t")
+#define rte_trace_ctf_u16(in)\
+	RTE_BUILD_BUG_ON(sizeof(uint16_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(uint16_t), RTE_STR(in), "uint16_t")
+#define rte_trace_ctf_i16(in)\
+	RTE_BUILD_BUG_ON(sizeof(int16_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(int16_t), RTE_STR(in), "int16_t")
+#define rte_trace_ctf_u8(in)\
+	RTE_BUILD_BUG_ON(sizeof(uint8_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(uint8_t), RTE_STR(in), "uint8_t")
+#define rte_trace_ctf_i8(in)\
+	RTE_BUILD_BUG_ON(sizeof(int8_t) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(int8_t), RTE_STR(in), "int8_t")
+#define rte_trace_ctf_int(in)\
+	RTE_BUILD_BUG_ON(sizeof(int) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(int), RTE_STR(in), "int32_t")
+#define rte_trace_ctf_long(in)\
+	RTE_BUILD_BUG_ON(sizeof(long) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(long), RTE_STR(in), "long")
+#define rte_trace_ctf_float(in)\
+	RTE_BUILD_BUG_ON(sizeof(float) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(float), RTE_STR(in), "float")
+#define rte_trace_ctf_double(in)\
+	RTE_BUILD_BUG_ON(sizeof(double) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(double), RTE_STR(in), "double")
+#define rte_trace_ctf_ptr(in)\
+	RTE_BUILD_BUG_ON(sizeof(void *) != sizeof(typeof(in)));\
+	__rte_trace_emit_ctf_field(sizeof(void *), RTE_STR(in), "uintptr_t")
+#define rte_trace_ctf_string(in)\
+	RTE_SET_USED(in);\
+	__rte_trace_emit_ctf_field(__RTE_TRACE_EMIT_STRING_LEN_MAX,\
+				   RTE_STR(in)"[32]", "string_bounded_t")
+
 #endif /* _RTE_TRACE_REGISTER_H_ */
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f56d1867e..0c787302f 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -339,7 +339,9 @@  EXPERIMENTAL {
 	# added in 20.05
 	rte_thread_getname;
 	__rte_trace_mem_per_thread_alloc;
+	__rte_trace_emit_ctf_field;
 	__rte_trace_point_register;
+	per_lcore_trace_point_sz;
 	per_lcore_trace_mem;
 	rte_trace_global_is_enabled;
 	rte_trace_global_is_disabled;