[v1,13/32] eal/trace: implement provider payload

  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 mem emitters aka
provider payload.

When it used as provider payload, those function copy the trace
field to trace memory based on the tracing mode.

Signed-off-by: Jerin Jacob <jerinj@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 .../common/include/rte_trace_provider.h       | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
  

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 mem emitters aka
> provider payload.
>
> When it used as provider payload, those function copy the trace
> field to trace memory based on the tracing mode.
>
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
>   .../common/include/rte_trace_provider.h       | 87 +++++++++++++++++++
>   1 file changed, 87 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/rte_trace_provider.h b/lib/librte_eal/common/include/rte_trace_provider.h
> index 2257de85b..66e9d2456 100644
> --- a/lib/librte_eal/common/include/rte_trace_provider.h
> +++ b/lib/librte_eal/common/include/rte_trace_provider.h
> @@ -9,6 +9,9 @@
>   #ifndef _RTE_TRACE_PROVIDER_H_
>   #define _RTE_TRACE_PROVIDER_H_
>   
> +#include <rte_branch_prediction.h>
> +#include <rte_cycles.h>
> +#include <rte_log.h>
>   #include <rte_per_lcore.h>
>   #include <rte_string_fns.h>
>   #include <rte_uuid.h>
> @@ -40,4 +43,88 @@ struct __rte_trace_header {
>   
>   RTE_DECLARE_PER_LCORE(void *, trace_mem);
>   
> +static __rte_always_inline void*
> +__rte_trace_mem_get(uint64_t in)
> +{
> +	struct __rte_trace_header *trace = RTE_PER_LCORE(trace_mem);
> +	const uint16_t sz = in & __RTE_TRACE_FIELD_SIZE_MASK;
> +
> +	/* Trace memory is not initialized for this thread */
> +	if (unlikely(trace == NULL)) {
> +		__rte_trace_mem_per_thread_alloc();
> +		trace = RTE_PER_LCORE(trace_mem);
> +		if (unlikely(trace == NULL))
> +			return NULL;
> +	}
> +	/* Check the wrap around case */
> +	uint32_t offset = trace->offset;
> +	if (unlikely((offset + sz) >= trace->len)) {
> +		/* Disable the trace event if it in DISCARD mode */
> +		if (unlikely(in & __RTE_TRACE_FIELD_ENABLE_DISCARD))
> +			return NULL;
> +
> +		offset = 0;
> +	}
> +	/* Align to event header size */
> +	offset = RTE_ALIGN_CEIL(offset, __RTE_TRACE_EVENT_HEADER_SZ);
> +	void *mem = RTE_PTR_ADD(&trace->mem[0], offset);
> +	offset += sz;
> +	trace->offset = offset;
> +
> +	return mem;
> +}
> +
> +static __rte_always_inline void*
> +__rte_trace_emit_ev_header(void *mem, uint64_t in)
> +{
> +	uint64_t val;
> +
> +	/* Event header [63:0] = id [63:48] | timestamp [47:0] */
> +	val = rte_get_tsc_cycles() &
> +		~(0xffffULL << __RTE_TRACE_EVENT_HEADER_ID_SHIFT);
> +	val |= ((in & __RTE_TRACE_FIELD_ID_MASK) <<
> +	      (__RTE_TRACE_EVENT_HEADER_ID_SHIFT - __RTE_TRACE_FIELD_ID_SHIFT));
> +
> +	*(uint64_t *)mem = val;
> +	return RTE_PTR_ADD(mem, __RTE_TRACE_EVENT_HEADER_SZ);
> +}
> +
> +#define __rte_trace_emit_header_generic(t)\
> +	const uint64_t val = __atomic_load_n(t, __ATOMIC_ACQUIRE);\
> +	if (likely(!(val & __RTE_TRACE_FIELD_ENABLE_MASK)))\
> +		return;\
> +	void *mem = __rte_trace_mem_get(val);\
> +	if (unlikely(mem == NULL)) \
> +		return;\
> +	mem = __rte_trace_emit_ev_header(mem, val)
> +
> +#define __rte_trace_emit_header_dp(t)\
> +	if (!rte_trace_is_dp_enabled())\
> +		return;\
> +	__rte_trace_emit_header_generic(t);
> +
> +#define __rte_trace_emit_datatype(in)\
> +	memcpy(mem, &(in), sizeof(in));\
> +	mem = RTE_PTR_ADD(mem, sizeof(in))
> +
> +#define rte_trace_ctf_u64(in) __rte_trace_emit_datatype(in)

Would it be worth to do a type check here? To avoid having someone do 
something like:

uint32_t v = 42;

rte_trace_ctf_u64(v);

which would spew out a 32-bit number, where there should be 64 bits.

Or maybe better: do an assignment, allowing type conversion (promotion 
at least), and type-checking, of sorts. The macro-generated code could 
look something like:

do {

     uint64_t _in = in;

     __rte_trace_emit_datatype(_in);

} while (0)

If you add a type parameter to __rte_trace_emit_datatype(), it can do 
the job.

> +#define rte_trace_ctf_i64(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_u32(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_i32(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_u16(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_i16(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_u8(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_i8(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_int(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_long(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_float(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_ptr(in) __rte_trace_emit_datatype(in)
> +#define rte_trace_ctf_double(in) __rte_trace_emit_datatype(in)
> +
> +#define rte_trace_ctf_string(in)\
Add the usual do { /../ } while (0) here?
> +	if (unlikely(in == NULL))\
> +		return;\
> +	rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX);\
> +	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX)
> +
>   #endif /* _RTE_TRACE_PROVIDER_H_ */
  

On Fri, Mar 20, 2020 at 12:58 AM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:

Thanks for the review.

>
> 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 mem emitters aka
> > provider payload.

> > +
> > +#define rte_trace_ctf_u64(in) __rte_trace_emit_datatype(in)
>
> Would it be worth to do a type check here? To avoid having someone do
> something like:
>
> uint32_t v = 42;
>
> rte_trace_ctf_u64(v);
>
> which would spew out a 32-bit number, where there should be 64 bits.

It is taken care with the register version of this macro by adding
RTE_BUILD_BUG_ON.

http://patches.dpdk.org/patch/66861/

See:
#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")


>
> Or maybe better: do an assignment, allowing type conversion (promotion
> at least), and type-checking, of sorts. The macro-generated code could
> look something like:
>
> do {
>
>      uint64_t _in = in;
>
>      __rte_trace_emit_datatype(_in);
>
> } while (0)
>
> If you add a type parameter to __rte_trace_emit_datatype(), it can do
> the job.
>

> > +
> > +#define rte_trace_ctf_string(in)\
> Add the usual do { /../ } while (0) here?

Ack. Will add it in  the place where do { /../ } while (0) can be added in v2.

> > +     if (unlikely(in == NULL))\
> > +             return;\
> > +     rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX);\
> > +     mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX)
> > +
> >   #endif /* _RTE_TRACE_PROVIDER_H_ */
>
>