diff mbox series

[v4,4/5] trace: support expression for blob length

Message ID 20250304160633.385185-5-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series Trace point framework enhancement for dmadev |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

David Marchand March 4, 2025, 4:06 p.m. UTC 
Support any expression as a blob length by using an intermediate
variable in the trace point emitter itself.
This also avoids any side effect on the passed variable.

On the "register" side, prefix the length variable in the trace metadata
with the name of the emitted argument.

With this, we can update ethdev traces and avoid intermediate variables.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v2:
- fixed length field in CTF metadata,

Changes since v1:
- removed code relying on arguments in "registering"
  rte_trace_point_emit_blob implementation (see patch 3),
- moved build check in non registering __rte_trace_point_emit,

---
 lib/eal/include/rte_trace_point.h          | 12 ++++++----
 lib/eal/include/rte_trace_point_register.h |  6 ++++-
 lib/ethdev/ethdev_trace.h                  | 27 ++++++----------------
 3 files changed, 19 insertions(+), 26 deletions(-)

Comments

Sunil Kumar Kori March 5, 2025, 5:20 a.m. UTC | #1 
> Support any expression as a blob length by using an intermediate variable in
> the trace point emitter itself.
> This also avoids any side effect on the passed variable.
> 
> On the "register" side, prefix the length variable in the trace metadata with
> the name of the emitted argument.
> 
> With this, we can update ethdev traces and avoid intermediate variables.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v2:
> - fixed length field in CTF metadata,
> 
> Changes since v1:
> - removed code relying on arguments in "registering"
>   rte_trace_point_emit_blob implementation (see patch 3),
> - moved build check in non registering __rte_trace_point_emit,
> 
> ---
>  lib/eal/include/rte_trace_point.h          | 12 ++++++----
>  lib/eal/include/rte_trace_point_register.h |  6 ++++-
>  lib/ethdev/ethdev_trace.h                  | 27 ++++++----------------
>  3 files changed, 19 insertions(+), 26 deletions(-)
>
> 2.48.1
Acked-by: Sunil Kumar Kori <skori@marvell.com>
diff mbox series

Patch

diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
index 0780460759..84de233d59 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -378,6 +378,7 @@  do { \
 
 #define __rte_trace_point_emit(name, in, type) \
 do { \
+	RTE_BUILD_BUG_ON(sizeof(type) != sizeof(typeof(*in))); \
 	memcpy(mem, in, sizeof(*in)); \
 	mem = RTE_PTR_ADD(mem, sizeof(*in)); \
 } while (0)
@@ -392,13 +393,14 @@  do { \
 
 #define rte_trace_point_emit_blob(in, len) \
 do { \
+	uint8_t size = len; \
 	if (unlikely(in == NULL)) \
 		return; \
-	if (len > RTE_TRACE_BLOB_LEN_MAX) \
-		len = RTE_TRACE_BLOB_LEN_MAX; \
-	__rte_trace_point_emit(RTE_STR(len), &len, uint8_t); \
-	memcpy(mem, in, len); \
-	memset(RTE_PTR_ADD(mem, len), 0, RTE_TRACE_BLOB_LEN_MAX - len); \
+	if (size > RTE_TRACE_BLOB_LEN_MAX) \
+		size = RTE_TRACE_BLOB_LEN_MAX; \
+	__rte_trace_point_emit("size", &size, uint8_t); \
+	memcpy(mem, in, size); \
+	memset(RTE_PTR_ADD(mem, size), 0, RTE_TRACE_BLOB_LEN_MAX - size); \
 	mem = RTE_PTR_ADD(mem, RTE_TRACE_BLOB_LEN_MAX); \
 } while (0)
 
diff --git a/lib/eal/include/rte_trace_point_register.h b/lib/eal/include/rte_trace_point_register.h
index ff861c1fba..ac391eb28f 100644
--- a/lib/eal/include/rte_trace_point_register.h
+++ b/lib/eal/include/rte_trace_point_register.h
@@ -50,8 +50,12 @@  do { \
 
 #define rte_trace_point_emit_blob(in, len) \
 do { \
+	uint8_t size = len; \
 	RTE_SET_USED(in); \
-	__rte_trace_point_emit(RTE_STR(len), &len, uint8_t); \
+	RTE_SET_USED(size); \
+	__rte_trace_point_emit_field(sizeof(uint8_t), \
+		RTE_STR(in) "_size", \
+		RTE_STR(uint8_t)); \
 	__rte_trace_point_emit_field(RTE_TRACE_BLOB_LEN_MAX, \
 		RTE_STR(in)"[" RTE_STR(RTE_TRACE_BLOB_LEN_MAX)"]", \
 		RTE_STR(uint8_t)); \
diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h
index 5951ae2d99..c65b78590a 100644
--- a/lib/ethdev/ethdev_trace.h
+++ b/lib/ethdev/ethdev_trace.h
@@ -860,10 +860,8 @@  RTE_TRACE_POINT(
 	rte_ethdev_trace_mac_addr_add,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id,
 		const struct rte_ether_addr *addr, uint32_t pool, int ret),
-	uint8_t len = RTE_ETHER_ADDR_LEN;
-
 	rte_trace_point_emit_u16(port_id);
-	rte_trace_point_emit_blob(addr->addr_bytes, len);
+	rte_trace_point_emit_blob(addr->addr_bytes, RTE_ETHER_ADDR_LEN);
 	rte_trace_point_emit_u32(pool);
 	rte_trace_point_emit_int(ret);
 )
@@ -872,20 +870,16 @@  RTE_TRACE_POINT(
 	rte_ethdev_trace_mac_addr_remove,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id,
 		const struct rte_ether_addr *addr),
-	uint8_t len = RTE_ETHER_ADDR_LEN;
-
 	rte_trace_point_emit_u16(port_id);
-	rte_trace_point_emit_blob(addr->addr_bytes, len);
+	rte_trace_point_emit_blob(addr->addr_bytes, RTE_ETHER_ADDR_LEN);
 )
 
 RTE_TRACE_POINT(
 	rte_ethdev_trace_default_mac_addr_set,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id,
 		const struct rte_ether_addr *addr),
-	uint8_t len = RTE_ETHER_ADDR_LEN;
-
 	rte_trace_point_emit_u16(port_id);
-	rte_trace_point_emit_blob(addr->addr_bytes, len);
+	rte_trace_point_emit_blob(addr->addr_bytes, RTE_ETHER_ADDR_LEN);
 )
 
 RTE_TRACE_POINT(
@@ -1102,11 +1096,9 @@  RTE_TRACE_POINT(
 	RTE_TRACE_POINT_ARGS(uint16_t port_id,
 		const struct rte_ether_addr *mc_addr_set, uint32_t nb_mc_addr,
 		int ret),
-	uint8_t len = nb_mc_addr * RTE_ETHER_ADDR_LEN;
-
 	rte_trace_point_emit_u16(port_id);
 	rte_trace_point_emit_u32(nb_mc_addr);
-	rte_trace_point_emit_blob(mc_addr_set, len);
+	rte_trace_point_emit_blob(mc_addr_set, nb_mc_addr * RTE_ETHER_ADDR_LEN);
 	rte_trace_point_emit_int(ret);
 )
 
@@ -1214,13 +1206,10 @@  RTE_TRACE_POINT(
 	rte_ethdev_trace_get_dcb_info,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id,
 		const struct rte_eth_dcb_info *dcb_info, int ret),
-	uint8_t num_user_priorities = RTE_ETH_DCB_NUM_USER_PRIORITIES;
-	uint8_t num_tcs = RTE_ETH_DCB_NUM_TCS;
-
 	rte_trace_point_emit_u16(port_id);
 	rte_trace_point_emit_u8(dcb_info->nb_tcs);
-	rte_trace_point_emit_blob(dcb_info->prio_tc, num_user_priorities);
-	rte_trace_point_emit_blob(dcb_info->tc_bws, num_tcs);
+	rte_trace_point_emit_blob(dcb_info->prio_tc, RTE_ETH_DCB_NUM_USER_PRIORITIES);
+	rte_trace_point_emit_blob(dcb_info->tc_bws, RTE_ETH_DCB_NUM_TCS);
 	rte_trace_point_emit_int(ret);
 )
 
@@ -2126,10 +2115,8 @@  RTE_TRACE_POINT_FP(
 	rte_eth_trace_macaddr_get,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id,
 		const struct rte_ether_addr *mac_addr),
-	uint8_t len = RTE_ETHER_ADDR_LEN;
-
 	rte_trace_point_emit_u16(port_id);
-	rte_trace_point_emit_blob(mac_addr->addr_bytes, len);
+	rte_trace_point_emit_blob(mac_addr->addr_bytes, RTE_ETHER_ADDR_LEN);
 )
 
 /* Called in loop in examples/ip_pipeline */