diff mbox series

ethdev: optimize activation of fast-path tracepoints

Message ID 20240913175828.21640-1-adel.belkhiri@gmail.com (mailing list archive)
State Superseded
Delegated to: Ferruh Yigit
Headers
Series ethdev: optimize activation of fast-path tracepoints |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Adel Belkhiri Sept. 13, 2024, 5:58 p.m. UTC 
From: Adel Belkhiri <adel.belkhiri@polymtl.ca>

Split the tracepoints rte_ethdev_trace_rx_burst and
rte_eth_trace_call_rx_callbacks into two separate ones
for empty and non-empty calls to avoid saturating
quickly the trace buffer.

Signed-off-by: Adel Belkhiri <adel.belkhiri@polymtl.ca>
---
 .mailmap                         |  1 +
 lib/ethdev/ethdev_private.c      |  8 ++++++--
 lib/ethdev/ethdev_trace_points.c | 14 ++++++++++----
 lib/ethdev/rte_ethdev.h          |  5 ++++-
 lib/ethdev/rte_ethdev_trace_fp.h | 23 +++++++++++++++++++++--
 lib/ethdev/version.map           |  3 ++-
 6 files changed, 44 insertions(+), 10 deletions(-)

Comments

Morten Brørup Sept. 13, 2024, 7:47 p.m. UTC | #1 
> +++ b/lib/ethdev/version.map
> @@ -207,7 +207,8 @@ EXPERIMENTAL {
>  	rte_flow_dev_dump;
> 
>  	# added in 20.05
> -	__rte_ethdev_trace_rx_burst;
> +	__rte_ethdev_trace_rx_burst_empty;
> +	__rte_ethdev_trace_rx_burst_nonempty;

These two should be moved down below "# added in 24.11" (which is not yet there, so should be added too).

And I don't know why __rte_eth_trace_call_rx_callbacks is not in the version.map file, but I suppose these should be added too:

+ __rte_eth_trace_call_rx_callbacks_empty
+ __rte_eth_trace_call_rx_callbacks_nonempty


Everything else looks good.

With version.map corrected,

Acked-by: Morten Brørup <mb@smartsharesystems.com>
Jerin Jacob Sept. 16, 2024, 8:25 a.m. UTC | #2 
On Sat, Sep 14, 2024 at 1:59 AM Adel Belkhiri <adel.belkhiri@gmail.com> wrote:
>
> From: Adel Belkhiri <adel.belkhiri@polymtl.ca>
>
> Split the tracepoints rte_ethdev_trace_rx_burst and
> rte_eth_trace_call_rx_callbacks into two separate ones
> for empty and non-empty calls to avoid saturating
> quickly the trace buffer.
>
> Signed-off-by: Adel Belkhiri <adel.belkhiri@polymtl.ca>
> ---

> -                                       nb_rx, nb_pkts);
> +       if (unlikely(nb_rx > 0))

You may consider unlikely(nb_rx)


>         rte_flow_dev_dump;
>
>         # added in 20.05
> -       __rte_ethdev_trace_rx_burst;

Removal of a public symbol breaks the ABI. The good news is that 24.11
can break the ABI.
IMO, It is OK to break this ABI.
Also need to update "Removed items" in doc/guides/rel_notes/release_24_11.rst


> +       __rte_ethdev_trace_rx_burst_empty;
> +       __rte_ethdev_trace_rx_burst_nonempty;
>         __rte_ethdev_trace_tx_burst;
>         rte_flow_get_aged_flows;
>
> --
> 2.34.1
>
David Marchand Sept. 16, 2024, 8:31 a.m. UTC | #3 
On Mon, Sep 16, 2024 at 10:26 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >         rte_flow_dev_dump;
> >
> >         # added in 20.05
> > -       __rte_ethdev_trace_rx_burst;
>
> Removal of a public symbol breaks the ABI. The good news is that 24.11
> can break the ABI.
> IMO, It is OK to break this ABI.

This symbol is in the experimental ABI.
And in general, I think we should keep the trace symbols as experimental.
Jerin Jacob Sept. 16, 2024, 8:45 a.m. UTC | #4 
On Mon, Sep 16, 2024 at 2:01 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, Sep 16, 2024 at 10:26 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >         rte_flow_dev_dump;
> > >
> > >         # added in 20.05
> > > -       __rte_ethdev_trace_rx_burst;
> >
> > Removal of a public symbol breaks the ABI. The good news is that 24.11
> > can break the ABI.
> > IMO, It is OK to break this ABI.
>
> This symbol is in the experimental ABI.
> And in general, I think we should keep the trace symbols as experimental.


Ack.

So I think, we need to update only  "Removed items" in
doc/guides/rel_notes/release_24_11.rst

>
>
> --
> David Marchand
>
diff mbox series

Patch

diff --git a/.mailmap b/.mailmap
index 4a508bafad..e86241dced 100644
--- a/.mailmap
+++ b/.mailmap
@@ -16,6 +16,7 @@  Abraham Tovar <abrahamx.tovar@intel.com>
 Adam Bynes <adambynes@outlook.com>
 Adam Dybkowski <adamx.dybkowski@intel.com>
 Adam Ludkiewicz <adam.ludkiewicz@intel.com>
+Adel Belkhiri <adel.belkhiri@polymtl.ca>
 Adham Masarwah <adham@nvidia.com> <adham@mellanox.com>
 Adrian Moreno <amorenoz@redhat.com>
 Adrian Pielech <adrian.pielech@intel.com>
diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c
index 626524558a..9aca7bb635 100644
--- a/lib/ethdev/ethdev_private.c
+++ b/lib/ethdev/ethdev_private.c
@@ -298,8 +298,12 @@  rte_eth_call_rx_callbacks(uint16_t port_id, uint16_t queue_id,
 		cb = cb->next;
 	}
 
-	rte_eth_trace_call_rx_callbacks(port_id, queue_id, (void **)rx_pkts,
-					nb_rx, nb_pkts);
+	if (unlikely(nb_rx > 0))
+		rte_eth_trace_call_rx_callbacks_nonempty(port_id, queue_id, (void **)rx_pkts,
+						nb_rx, nb_pkts);
+	else
+		rte_eth_trace_call_rx_callbacks_empty(port_id, queue_id, (void **)rx_pkts,
+						nb_pkts);
 
 	return nb_rx;
 }
diff --git a/lib/ethdev/ethdev_trace_points.c b/lib/ethdev/ethdev_trace_points.c
index 99e04f5893..6ecbee289b 100644
--- a/lib/ethdev/ethdev_trace_points.c
+++ b/lib/ethdev/ethdev_trace_points.c
@@ -25,14 +25,20 @@  RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_stop,
 RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_close,
 	lib.ethdev.close)
 
-RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rx_burst,
-	lib.ethdev.rx.burst)
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rx_burst_empty,
+	lib.ethdev.rx.burst.empty)
+
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rx_burst_nonempty,
+	lib.ethdev.rx.burst.nonempty)
 
 RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_tx_burst,
 	lib.ethdev.tx.burst)
 
-RTE_TRACE_POINT_REGISTER(rte_eth_trace_call_rx_callbacks,
-	lib.ethdev.call_rx_callbacks)
+RTE_TRACE_POINT_REGISTER(rte_eth_trace_call_rx_callbacks_empty,
+	lib.ethdev.call_rx_callbacks.empty)
+
+RTE_TRACE_POINT_REGISTER(rte_eth_trace_call_rx_callbacks_nonempty,
+	lib.ethdev.call_rx_callbacks.nonempty)
 
 RTE_TRACE_POINT_REGISTER(rte_eth_trace_call_tx_callbacks,
 	lib.ethdev.call_tx_callbacks)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 548fada1c7..e9dbbf677b 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -6132,7 +6132,10 @@  rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
 	}
 #endif
 
-	rte_ethdev_trace_rx_burst(port_id, queue_id, (void **)rx_pkts, nb_rx);
+	if (unlikely(nb_rx > 0))
+		rte_ethdev_trace_rx_burst_nonempty(port_id, queue_id, (void **)rx_pkts, nb_rx);
+	else
+		rte_ethdev_trace_rx_burst_empty(port_id, queue_id, (void **)rx_pkts);
 	return nb_rx;
 }
 
diff --git a/lib/ethdev/rte_ethdev_trace_fp.h b/lib/ethdev/rte_ethdev_trace_fp.h
index 40b6e4756b..d23865996a 100644
--- a/lib/ethdev/rte_ethdev_trace_fp.h
+++ b/lib/ethdev/rte_ethdev_trace_fp.h
@@ -18,7 +18,16 @@  extern "C" {
 #include <rte_trace_point.h>
 
 RTE_TRACE_POINT_FP(
-	rte_ethdev_trace_rx_burst,
+	rte_ethdev_trace_rx_burst_empty,
+	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id,
+		void **pkt_tbl),
+	rte_trace_point_emit_u16(port_id);
+	rte_trace_point_emit_u16(queue_id);
+	rte_trace_point_emit_ptr(pkt_tbl);
+)
+
+RTE_TRACE_POINT_FP(
+	rte_ethdev_trace_rx_burst_nonempty,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id,
 		void **pkt_tbl, uint16_t nb_rx),
 	rte_trace_point_emit_u16(port_id);
@@ -38,7 +47,17 @@  RTE_TRACE_POINT_FP(
 )
 
 RTE_TRACE_POINT_FP(
-	rte_eth_trace_call_rx_callbacks,
+	rte_eth_trace_call_rx_callbacks_empty,
+	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id,
+		void **rx_pkts, uint16_t nb_pkts),
+	rte_trace_point_emit_u16(port_id);
+	rte_trace_point_emit_u16(queue_id);
+	rte_trace_point_emit_ptr(rx_pkts);
+	rte_trace_point_emit_u16(nb_pkts);
+)
+
+RTE_TRACE_POINT_FP(
+	rte_eth_trace_call_rx_callbacks_nonempty,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id,
 		void **rx_pkts, uint16_t nb_rx, uint16_t nb_pkts),
 	rte_trace_point_emit_u16(port_id);
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1669055ca5..92cd4d7a2d 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -207,7 +207,8 @@  EXPERIMENTAL {
 	rte_flow_dev_dump;
 
 	# added in 20.05
-	__rte_ethdev_trace_rx_burst;
+	__rte_ethdev_trace_rx_burst_empty;
+	__rte_ethdev_trace_rx_burst_nonempty;
 	__rte_ethdev_trace_tx_burst;
 	rte_flow_get_aged_flows;