[RFC,2/5] common/mlx5: introduce tracepoints for mlx5 drivers

Message ID 20230420100803.494-3-viacheslavo@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: introduce Tx datapath tracing |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Slava Ovsiienko April 20, 2023, 10:08 a.m. UTC
  There is an intention to engage DPDK tracing capabilities
for mlx5 PMDs monitoring and profiling in various modes.
The patch introduces tracepoints for the Tx datapath in
the ethernet device driver.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/common/mlx5/meson.build  |  1 +
 drivers/common/mlx5/mlx5_trace.c | 25 +++++++++++
 drivers/common/mlx5/mlx5_trace.h | 72 ++++++++++++++++++++++++++++++++
 drivers/common/mlx5/version.map  |  8 ++++
 4 files changed, 106 insertions(+)
 create mode 100644 drivers/common/mlx5/mlx5_trace.c
 create mode 100644 drivers/common/mlx5/mlx5_trace.h
  

Comments

Jerin Jacob April 20, 2023, 10:11 a.m. UTC | #1
On Thu, Apr 20, 2023 at 3:38 PM Viacheslav Ovsiienko
<viacheslavo@nvidia.com> wrote:
>
> There is an intention to engage DPDK tracing capabilities
> for mlx5 PMDs monitoring and profiling in various modes.
> The patch introduces tracepoints for the Tx datapath in
> the ethernet device driver.
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
>  drivers/common/mlx5/meson.build  |  1 +
>  drivers/common/mlx5/mlx5_trace.c | 25 +++++++++++
>  drivers/common/mlx5/mlx5_trace.h | 72 ++++++++++++++++++++++++++++++++
>  drivers/common/mlx5/version.map  |  8 ++++
>  4 files changed, 106 insertions(+)
>  create mode 100644 drivers/common/mlx5/mlx5_trace.c
>  create mode 100644 drivers/common/mlx5/mlx5_trace.h
>
> diff --git a/drivers/common/mlx5/meson.build b/drivers/common/mlx5/meson.build
> index 9dc809f192..e074ffb140 100644
> --- a/drivers/common/mlx5/meson.build
> +++ b/drivers/common/mlx5/meson.build
> @@ -19,6 +19,7 @@ sources += files(
>          'mlx5_common_mp.c',
>          'mlx5_common_mr.c',
>          'mlx5_malloc.c',
> +        'mlx5_trace.c',
>          'mlx5_common_pci.c',
>          'mlx5_common_devx.c',
>          'mlx5_common_utils.c',
> diff --git a/drivers/common/mlx5/mlx5_trace.c b/drivers/common/mlx5/mlx5_trace.c
> new file mode 100644
> index 0000000000..b9f14413ad
> --- /dev/null
> +++ b/drivers/common/mlx5/mlx5_trace.c
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2022 NVIDIA Corporation & Affiliates
> + */
> +
> +#include <rte_trace_point_register.h>
> +#include <mlx5_trace.h>
> +
> +RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_entry,
> +       pmd.net.mlx5.tx.entry)
> +
> +RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_exit,
> +       pmd.net.mlx5.tx.exit)
> +
> +RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_wqe,
> +       pmd.net.mlx5.tx.wqe)
> +
> +RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_wait,
> +       pmd.net.mlx5.tx.wait)
> +
> +RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_push,
> +       pmd.net.mlx5.tx.push)
> +
> +RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_complete,
> +       pmd.net.mlx5.tx.complete)
> +
> diff --git a/drivers/common/mlx5/mlx5_trace.h b/drivers/common/mlx5/mlx5_trace.h
> new file mode 100644
> index 0000000000..57512e654f
> --- /dev/null
> +++ b/drivers/common/mlx5/mlx5_trace.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2022 NVIDIA Corporation & Affiliates
> + */
> +
> +#ifndef RTE_PMD_MLX5_TRACE_H_
> +#define RTE_PMD_MLX5_TRACE_H_
> +
> +/**
> + * @file
> + *
> + * API for mlx5 PMD trace support
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <mlx5_prm.h>
> +#include <rte_mbuf.h>
> +#include <rte_trace_point.h>
> +
> +RTE_TRACE_POINT_FP(
> +       rte_pmd_mlx5_trace_tx_entry,
> +       RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id),
> +       rte_trace_point_emit_u16(port_id);
> +       rte_trace_point_emit_u16(queue_id);
> +)
> +
> +RTE_TRACE_POINT_FP(
> +       rte_pmd_mlx5_trace_tx_exit,
> +       RTE_TRACE_POINT_ARGS(uint16_t nb_sent, uint16_t nb_req),
> +       rte_trace_point_emit_u16(nb_sent);
> +       rte_trace_point_emit_u16(nb_req);
> +)
> +
> +RTE_TRACE_POINT_FP(
> +       rte_pmd_mlx5_trace_tx_wqe,
> +       RTE_TRACE_POINT_ARGS(uint32_t opcode),
> +       rte_trace_point_emit_u32(opcode);
> +)
> +
> +RTE_TRACE_POINT_FP(
> +       rte_pmd_mlx5_trace_tx_wait,
> +       RTE_TRACE_POINT_ARGS(uint64_t ts),
> +       rte_trace_point_emit_u64(ts);
> +)
> +
> +
> +RTE_TRACE_POINT_FP(
> +       rte_pmd_mlx5_trace_tx_push,
> +       RTE_TRACE_POINT_ARGS(const struct rte_mbuf *mbuf, uint16_t wqe_id),
> +       rte_trace_point_emit_ptr(mbuf);
> +       rte_trace_point_emit_u32(mbuf->pkt_len);
> +       rte_trace_point_emit_u16(mbuf->nb_segs);
> +       rte_trace_point_emit_u16(wqe_id);
> +)
> +
> +RTE_TRACE_POINT_FP(
> +       rte_pmd_mlx5_trace_tx_complete,
> +       RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id,
> +                            uint16_t wqe_id, uint64_t ts),
> +       rte_trace_point_emit_u16(port_id);
> +       rte_trace_point_emit_u16(queue_id);
> +       rte_trace_point_emit_u64(ts);
> +       rte_trace_point_emit_u16(wqe_id);
> +)
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* RTE_PMD_MLX5_TRACE_H_ */
> diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map
> index e05e1aa8c5..d0ec8571e6 100644
> --- a/drivers/common/mlx5/version.map
> +++ b/drivers/common/mlx5/version.map
> @@ -158,5 +158,13 @@ INTERNAL {
>
>         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
>         mlx5_os_interrupt_handler_destroy; # WINDOWS_NO_EXPORT
> +
> +       __rte_pmd_mlx5_trace_tx_entry;
> +       __rte_pmd_mlx5_trace_tx_exit;
> +       __rte_pmd_mlx5_trace_tx_wqe;
> +       __rte_pmd_mlx5_trace_tx_wait;
> +       __rte_pmd_mlx5_trace_tx_push;
> +       __rte_pmd_mlx5_trace_tx_complete;

No need to expose these symbols. It is getting removed from rest of
DPDK. Application can do rte_trace_lookup() to get this address.


> +
>         local: *;
>  };
> --
> 2.18.1
>
  
Slava Ovsiienko June 13, 2023, 3:50 p.m. UTC | #2
Hi,

<..snip..>
> >
> >         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
> >         mlx5_os_interrupt_handler_destroy; # WINDOWS_NO_EXPORT
> > +
> > +       __rte_pmd_mlx5_trace_tx_entry;
> > +       __rte_pmd_mlx5_trace_tx_exit;
> > +       __rte_pmd_mlx5_trace_tx_wqe;
> > +       __rte_pmd_mlx5_trace_tx_wait;
> > +       __rte_pmd_mlx5_trace_tx_push;
> > +       __rte_pmd_mlx5_trace_tx_complete;
> 
> No need to expose these symbols. It is getting removed from rest of DPDK.
> Application can do rte_trace_lookup() to get this address.
> 
> 
It is not for application, it is for PMD itself, w/o exposing the symbols build failed.

With best regards,
Slava
  
Jerin Jacob June 13, 2023, 3:53 p.m. UTC | #3
On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko <viacheslavo@nvidia.com> wrote:
>
> Hi,
>
> <..snip..>
> > >
> > >         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
> > >         mlx5_os_interrupt_handler_destroy; # WINDOWS_NO_EXPORT
> > > +
> > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > +       __rte_pmd_mlx5_trace_tx_push;
> > > +       __rte_pmd_mlx5_trace_tx_complete;
> >
> > No need to expose these symbols. It is getting removed from rest of DPDK.
> > Application can do rte_trace_lookup() to get this address.
> >
> >
> It is not for application, it is for PMD itself, w/o exposing the symbols build failed.

PMD is implementing this trace endpoints, not consuming this trace
point. Right? If so, Why to expose these symbols?

>
> With best regards,
> Slava
>
  
Slava Ovsiienko June 13, 2023, 3:59 p.m. UTC | #4
Hi,

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, June 13, 2023 6:53 PM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org
> Subject: Re: [RFC 2/5] common/mlx5: introduce tracepoints for mlx5 drivers
> 
> On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko <viacheslavo@nvidia.com>
> wrote:
> >
> > Hi,
> >
> > <..snip..>
> > > >
> > > >         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
> > > >         mlx5_os_interrupt_handler_destroy; # WINDOWS_NO_EXPORT
> > > > +
> > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > >
> > > No need to expose these symbols. It is getting removed from rest of DPDK.
> > > Application can do rte_trace_lookup() to get this address.
> > >
> > >
> > It is not for application, it is for PMD itself, w/o exposing the symbols build
> failed.
> 
> PMD is implementing this trace endpoints, not consuming this trace point.
> Right? If so, Why to expose these symbols?

As far as understand:
The tracepoint routines are defined in dedicated common/mlx5_trace.c file.
The tx_burst in mlx5 is implemented as template in header file, and this
template is used in multiple .c files under net/mlx5 filder.
So, common/mlx5 should expose its symbols to net/mlx5 to allow successful
linkage.

With best regards,
Slava
  
Jerin Jacob June 13, 2023, 4:01 p.m. UTC | #5
On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko <viacheslavo@nvidia.com> wrote:
>
> Hi,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Tuesday, June 13, 2023 6:53 PM
> > To: Slava Ovsiienko <viacheslavo@nvidia.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [RFC 2/5] common/mlx5: introduce tracepoints for mlx5 drivers
> >
> > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko <viacheslavo@nvidia.com>
> > wrote:
> > >
> > > Hi,
> > >
> > > <..snip..>
> > > > >
> > > > >         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
> > > > >         mlx5_os_interrupt_handler_destroy; # WINDOWS_NO_EXPORT
> > > > > +
> > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > >
> > > > No need to expose these symbols. It is getting removed from rest of DPDK.
> > > > Application can do rte_trace_lookup() to get this address.
> > > >
> > > >
> > > It is not for application, it is for PMD itself, w/o exposing the symbols build
> > failed.
> >
> > PMD is implementing this trace endpoints, not consuming this trace point.
> > Right? If so, Why to expose these symbols?
>
> As far as understand:
> The tracepoint routines are defined in dedicated common/mlx5_trace.c file.
> The tx_burst in mlx5 is implemented as template in header file, and this
> template is used in multiple .c files under net/mlx5 filder.
> So, common/mlx5 should expose its symbols to net/mlx5 to allow successful
> linkage.

OK. I missed the fact the these are in common code and net driver is
depened on that.
So changes makes sense.

>
> With best regards,
> Slava
  
Thomas Monjalon June 27, 2023, 12:39 a.m. UTC | #6
13/06/2023 18:01, Jerin Jacob:
> On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko <viacheslavo@nvidia.com> wrote:
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko <viacheslavo@nvidia.com>
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > <..snip..>
> > > > > >
> > > > > >         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
> > > > > >         mlx5_os_interrupt_handler_destroy; # WINDOWS_NO_EXPORT
> > > > > > +
> > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > >
> > > > > No need to expose these symbols. It is getting removed from rest of DPDK.
> > > > > Application can do rte_trace_lookup() to get this address.
> > > > >
> > > > >
> > > > It is not for application, it is for PMD itself, w/o exposing the symbols build
> > > failed.
> > >
> > > PMD is implementing this trace endpoints, not consuming this trace point.
> > > Right? If so, Why to expose these symbols?
> >
> > As far as understand:
> > The tracepoint routines are defined in dedicated common/mlx5_trace.c file.
> > The tx_burst in mlx5 is implemented as template in header file, and this
> > template is used in multiple .c files under net/mlx5 filder.
> > So, common/mlx5 should expose its symbols to net/mlx5 to allow successful
> > linkage.
> 
> OK. I missed the fact the these are in common code and net driver is
> depened on that.
> So changes makes sense.

It does not make sense to me.
These are tracepoints for the ethdev driver.
Why declaring them in the common library?
  
Slava Ovsiienko June 27, 2023, 6:15 a.m. UTC | #7
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, June 27, 2023 3:40 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinjacobk@gmail.com>; Raslan Darawsheh
> <rasland@nvidia.com>
> Subject: Re: [RFC 2/5] common/mlx5: introduce tracepoints for mlx5 drivers
> 
> 13/06/2023 18:01, Jerin Jacob:
> > On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko <viacheslavo@nvidia.com>
> wrote:
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko
> > > > <viacheslavo@nvidia.com>
> > > > wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > <..snip..>
> > > > > > >
> > > > > > >         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
> > > > > > >         mlx5_os_interrupt_handler_destroy; #
> > > > > > > WINDOWS_NO_EXPORT
> > > > > > > +
> > > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > > >
> > > > > > No need to expose these symbols. It is getting removed from rest of
> DPDK.
> > > > > > Application can do rte_trace_lookup() to get this address.
> > > > > >
> > > > > >
> > > > > It is not for application, it is for PMD itself, w/o exposing
> > > > > the symbols build
> > > > failed.
> > > >
> > > > PMD is implementing this trace endpoints, not consuming this trace
> point.
> > > > Right? If so, Why to expose these symbols?
> > >
> > > As far as understand:
> > > The tracepoint routines are defined in dedicated common/mlx5_trace.c
> file.
> > > The tx_burst in mlx5 is implemented as template in header file, and
> > > this template is used in multiple .c files under net/mlx5 filder.
> > > So, common/mlx5 should expose its symbols to net/mlx5 to allow
> > > successful linkage.
> >
> > OK. I missed the fact the these are in common code and net driver is
> > depened on that.
> > So changes makes sense.
> 
> It does not make sense to me.
> These are tracepoints for the ethdev driver.
> Why declaring them in the common library?

Just to gather all mlx5 traces in the single file, to see all available tracing caps in single view.

With best regards,
Slava
  
Thomas Monjalon June 27, 2023, 7:28 a.m. UTC | #8
27/06/2023 08:15, Slava Ovsiienko:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 13/06/2023 18:01, Jerin Jacob:
> > > On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko <viacheslavo@nvidia.com>
> > wrote:
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko
> > > > > <viacheslavo@nvidia.com>
> > > > > wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > <..snip..>
> > > > > > > >
> > > > > > > >         mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
> > > > > > > >         mlx5_os_interrupt_handler_destroy; #
> > > > > > > > WINDOWS_NO_EXPORT
> > > > > > > > +
> > > > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > > > >
> > > > > > > No need to expose these symbols. It is getting removed from rest of
> > DPDK.
> > > > > > > Application can do rte_trace_lookup() to get this address.
> > > > > > >
> > > > > > >
> > > > > > It is not for application, it is for PMD itself, w/o exposing
> > > > > > the symbols build
> > > > > failed.
> > > > >
> > > > > PMD is implementing this trace endpoints, not consuming this trace
> > point.
> > > > > Right? If so, Why to expose these symbols?
> > > >
> > > > As far as understand:
> > > > The tracepoint routines are defined in dedicated common/mlx5_trace.c
> > file.
> > > > The tx_burst in mlx5 is implemented as template in header file, and
> > > > this template is used in multiple .c files under net/mlx5 filder.
> > > > So, common/mlx5 should expose its symbols to net/mlx5 to allow
> > > > successful linkage.
> > >
> > > OK. I missed the fact the these are in common code and net driver is
> > > depened on that.
> > > So changes makes sense.
> > 
> > It does not make sense to me.
> > These are tracepoints for the ethdev driver.
> > Why declaring them in the common library?
> 
> Just to gather all mlx5 traces in the single file, to see all available tracing caps in single view.

Better to not export them.
  
Slava Ovsiienko June 27, 2023, 8:19 a.m. UTC | #9
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, June 27, 2023 10:29 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinjacobk@gmail.com>; Raslan Darawsheh
> <rasland@nvidia.com>
> Subject: Re: [RFC 2/5] common/mlx5: introduce tracepoints for mlx5 drivers
> 
> 27/06/2023 08:15, Slava Ovsiienko:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 13/06/2023 18:01, Jerin Jacob:
> > > > On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko
> > > > <viacheslavo@nvidia.com>
> > > wrote:
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko
> > > > > > <viacheslavo@nvidia.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > <..snip..>
> > > > > > > > >
> > > > > > > > >         mlx5_os_interrupt_handler_create; #
> WINDOWS_NO_EXPORT
> > > > > > > > >         mlx5_os_interrupt_handler_destroy; #
> > > > > > > > > WINDOWS_NO_EXPORT
> > > > > > > > > +
> > > > > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > > > > >
> > > > > > > > No need to expose these symbols. It is getting removed
> > > > > > > > from rest of
> > > DPDK.
> > > > > > > > Application can do rte_trace_lookup() to get this address.
> > > > > > > >
> > > > > > > >
> > > > > > > It is not for application, it is for PMD itself, w/o
> > > > > > > exposing the symbols build
> > > > > > failed.
> > > > > >
> > > > > > PMD is implementing this trace endpoints, not consuming this
> > > > > > trace
> > > point.
> > > > > > Right? If so, Why to expose these symbols?
> > > > >
> > > > > As far as understand:
> > > > > The tracepoint routines are defined in dedicated
> > > > > common/mlx5_trace.c
> > > file.
> > > > > The tx_burst in mlx5 is implemented as template in header file,
> > > > > and this template is used in multiple .c files under net/mlx5 filder.
> > > > > So, common/mlx5 should expose its symbols to net/mlx5 to allow
> > > > > successful linkage.
> > > >
> > > > OK. I missed the fact the these are in common code and net driver
> > > > is depened on that.
> > > > So changes makes sense.
> > >
> > > It does not make sense to me.
> > > These are tracepoints for the ethdev driver.
> > > Why declaring them in the common library?
> >
> > Just to gather all mlx5 traces in the single file, to see all available tracing
> caps in single view.
> 
> Better to not export them.
> 
It is not about export, we have analyzing script over the trace data, and it would be better to see all the tracing
routines gathered in one place. I would prefer not to spread trace point definitions over the multiple source files.
 
With best regards,
Slava
  
Thomas Monjalon June 27, 2023, 9:33 a.m. UTC | #10
27/06/2023 10:19, Slava Ovsiienko:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 27/06/2023 08:15, Slava Ovsiienko:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 13/06/2023 18:01, Jerin Jacob:
> > > > > On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko
> > > > > <viacheslavo@nvidia.com>
> > > > wrote:
> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko
> > > > > > > <viacheslavo@nvidia.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > <..snip..>
> > > > > > > > > >
> > > > > > > > > >         mlx5_os_interrupt_handler_create; #
> > WINDOWS_NO_EXPORT
> > > > > > > > > >         mlx5_os_interrupt_handler_destroy; #
> > > > > > > > > > WINDOWS_NO_EXPORT
> > > > > > > > > > +
> > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > > > > > >
> > > > > > > > > No need to expose these symbols. It is getting removed
> > > > > > > > > from rest of
> > > > DPDK.
> > > > > > > > > Application can do rte_trace_lookup() to get this address.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > It is not for application, it is for PMD itself, w/o
> > > > > > > > exposing the symbols build
> > > > > > > failed.
> > > > > > >
> > > > > > > PMD is implementing this trace endpoints, not consuming this
> > > > > > > trace
> > > > point.
> > > > > > > Right? If so, Why to expose these symbols?
> > > > > >
> > > > > > As far as understand:
> > > > > > The tracepoint routines are defined in dedicated
> > > > > > common/mlx5_trace.c
> > > > file.
> > > > > > The tx_burst in mlx5 is implemented as template in header file,
> > > > > > and this template is used in multiple .c files under net/mlx5 filder.
> > > > > > So, common/mlx5 should expose its symbols to net/mlx5 to allow
> > > > > > successful linkage.
> > > > >
> > > > > OK. I missed the fact the these are in common code and net driver
> > > > > is depened on that.
> > > > > So changes makes sense.
> > > >
> > > > It does not make sense to me.
> > > > These are tracepoints for the ethdev driver.
> > > > Why declaring them in the common library?
> > >
> > > Just to gather all mlx5 traces in the single file, to see all available tracing
> > caps in single view.
> > 
> > Better to not export them.
> > 
> It is not about export, we have analyzing script over the trace data, and it would be better to see all the tracing
> routines gathered in one place. I would prefer not to spread trace point definitions over the multiple source files.

You don't need to export trace symbols for using them.
It has been discussed already with Jerin that we prefer not exporting
trace symbols if it can be avoided. Here it can be avoided.
  
Slava Ovsiienko June 27, 2023, 9:43 a.m. UTC | #11
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, June 27, 2023 12:34 PM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinjacobk@gmail.com>; Raslan Darawsheh
> <rasland@nvidia.com>; david.marchand@redhat.com; Maayan Kashani
> <mkashani@nvidia.com>
> Subject: Re: [RFC 2/5] common/mlx5: introduce tracepoints for mlx5 drivers
> 
> 27/06/2023 10:19, Slava Ovsiienko:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 27/06/2023 08:15, Slava Ovsiienko:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 13/06/2023 18:01, Jerin Jacob:
> > > > > > On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko
> > > > > > <viacheslavo@nvidia.com>
> > > > > wrote:
> > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko
> > > > > > > > <viacheslavo@nvidia.com>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > <..snip..>
> > > > > > > > > > >
> > > > > > > > > > >         mlx5_os_interrupt_handler_create; #
> > > WINDOWS_NO_EXPORT
> > > > > > > > > > >         mlx5_os_interrupt_handler_destroy; #
> > > > > > > > > > > WINDOWS_NO_EXPORT
> > > > > > > > > > > +
> > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > > > > > > >
> > > > > > > > > > No need to expose these symbols. It is getting removed
> > > > > > > > > > from rest of
> > > > > DPDK.
> > > > > > > > > > Application can do rte_trace_lookup() to get this address.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > It is not for application, it is for PMD itself, w/o
> > > > > > > > > exposing the symbols build
> > > > > > > > failed.
> > > > > > > >
> > > > > > > > PMD is implementing this trace endpoints, not consuming
> > > > > > > > this trace
> > > > > point.
> > > > > > > > Right? If so, Why to expose these symbols?
> > > > > > >
> > > > > > > As far as understand:
> > > > > > > The tracepoint routines are defined in dedicated
> > > > > > > common/mlx5_trace.c
> > > > > file.
> > > > > > > The tx_burst in mlx5 is implemented as template in header
> > > > > > > file, and this template is used in multiple .c files under net/mlx5
> filder.
> > > > > > > So, common/mlx5 should expose its symbols to net/mlx5 to
> > > > > > > allow successful linkage.
> > > > > >
> > > > > > OK. I missed the fact the these are in common code and net
> > > > > > driver is depened on that.
> > > > > > So changes makes sense.
> > > > >
> > > > > It does not make sense to me.
> > > > > These are tracepoints for the ethdev driver.
> > > > > Why declaring them in the common library?
> > > >
> > > > Just to gather all mlx5 traces in the single file, to see all
> > > > available tracing
> > > caps in single view.
> > >
> > > Better to not export them.
> > >
> > It is not about export, we have analyzing script over the trace data,
> > and it would be better to see all the tracing routines gathered in one place.
> I would prefer not to spread trace point definitions over the multiple source
> files.
> 
> You don't need to export trace symbols for using them.
> It has been discussed already with Jerin that we prefer not exporting trace
> symbols if it can be avoided. Here it can be avoided.

Without exporting symbols defined in common/mlx5 we have link error for net/mlx5.
So, do you insist on having dedicated mlx5_trace.c per /net/mlx5, common/mlx5, etc?

With best regards,
Slava
  
Thomas Monjalon June 27, 2023, 11:36 a.m. UTC | #12
27/06/2023 11:43, Slava Ovsiienko:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 27/06/2023 10:19, Slava Ovsiienko:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 27/06/2023 08:15, Slava Ovsiienko:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 13/06/2023 18:01, Jerin Jacob:
> > > > > > > On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko
> > > > > > > <viacheslavo@nvidia.com>
> > > > > > wrote:
> > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko
> > > > > > > > > <viacheslavo@nvidia.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi,
> > > > > > > > > >
> > > > > > > > > > <..snip..>
> > > > > > > > > > > >
> > > > > > > > > > > >         mlx5_os_interrupt_handler_create; #
> > > > WINDOWS_NO_EXPORT
> > > > > > > > > > > >         mlx5_os_interrupt_handler_destroy; #
> > > > > > > > > > > > WINDOWS_NO_EXPORT
> > > > > > > > > > > > +
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > > > > > > > >
> > > > > > > > > > > No need to expose these symbols. It is getting removed
> > > > > > > > > > > from rest of
> > > > > > DPDK.
> > > > > > > > > > > Application can do rte_trace_lookup() to get this address.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > It is not for application, it is for PMD itself, w/o
> > > > > > > > > > exposing the symbols build
> > > > > > > > > failed.
> > > > > > > > >
> > > > > > > > > PMD is implementing this trace endpoints, not consuming
> > > > > > > > > this trace
> > > > > > point.
> > > > > > > > > Right? If so, Why to expose these symbols?
> > > > > > > >
> > > > > > > > As far as understand:
> > > > > > > > The tracepoint routines are defined in dedicated
> > > > > > > > common/mlx5_trace.c
> > > > > > file.
> > > > > > > > The tx_burst in mlx5 is implemented as template in header
> > > > > > > > file, and this template is used in multiple .c files under net/mlx5
> > filder.
> > > > > > > > So, common/mlx5 should expose its symbols to net/mlx5 to
> > > > > > > > allow successful linkage.
> > > > > > >
> > > > > > > OK. I missed the fact the these are in common code and net
> > > > > > > driver is depened on that.
> > > > > > > So changes makes sense.
> > > > > >
> > > > > > It does not make sense to me.
> > > > > > These are tracepoints for the ethdev driver.
> > > > > > Why declaring them in the common library?
> > > > >
> > > > > Just to gather all mlx5 traces in the single file, to see all
> > > > > available tracing
> > > > caps in single view.
> > > >
> > > > Better to not export them.
> > > >
> > > It is not about export, we have analyzing script over the trace data,
> > > and it would be better to see all the tracing routines gathered in one place.
> > I would prefer not to spread trace point definitions over the multiple source
> > files.
> > 
> > You don't need to export trace symbols for using them.
> > It has been discussed already with Jerin that we prefer not exporting trace
> > symbols if it can be avoided. Here it can be avoided.
> 
> Without exporting symbols defined in common/mlx5 we have link error for net/mlx5.
> So, do you insist on having dedicated mlx5_trace.c per /net/mlx5, common/mlx5, etc?

Yes please it looks better to have tracepoints close to their respective functions.
  

Patch

diff --git a/drivers/common/mlx5/meson.build b/drivers/common/mlx5/meson.build
index 9dc809f192..e074ffb140 100644
--- a/drivers/common/mlx5/meson.build
+++ b/drivers/common/mlx5/meson.build
@@ -19,6 +19,7 @@  sources += files(
         'mlx5_common_mp.c',
         'mlx5_common_mr.c',
         'mlx5_malloc.c',
+        'mlx5_trace.c',
         'mlx5_common_pci.c',
         'mlx5_common_devx.c',
         'mlx5_common_utils.c',
diff --git a/drivers/common/mlx5/mlx5_trace.c b/drivers/common/mlx5/mlx5_trace.c
new file mode 100644
index 0000000000..b9f14413ad
--- /dev/null
+++ b/drivers/common/mlx5/mlx5_trace.c
@@ -0,0 +1,25 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2022 NVIDIA Corporation & Affiliates
+ */
+
+#include <rte_trace_point_register.h>
+#include <mlx5_trace.h>
+
+RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_entry,
+	pmd.net.mlx5.tx.entry)
+
+RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_exit,
+	pmd.net.mlx5.tx.exit)
+
+RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_wqe,
+	pmd.net.mlx5.tx.wqe)
+
+RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_wait,
+	pmd.net.mlx5.tx.wait)
+
+RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_push,
+	pmd.net.mlx5.tx.push)
+
+RTE_TRACE_POINT_REGISTER(rte_pmd_mlx5_trace_tx_complete,
+	pmd.net.mlx5.tx.complete)
+
diff --git a/drivers/common/mlx5/mlx5_trace.h b/drivers/common/mlx5/mlx5_trace.h
new file mode 100644
index 0000000000..57512e654f
--- /dev/null
+++ b/drivers/common/mlx5/mlx5_trace.h
@@ -0,0 +1,72 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2022 NVIDIA Corporation & Affiliates
+ */
+
+#ifndef RTE_PMD_MLX5_TRACE_H_
+#define RTE_PMD_MLX5_TRACE_H_
+
+/**
+ * @file
+ *
+ * API for mlx5 PMD trace support
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <mlx5_prm.h>
+#include <rte_mbuf.h>
+#include <rte_trace_point.h>
+
+RTE_TRACE_POINT_FP(
+	rte_pmd_mlx5_trace_tx_entry,
+	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id),
+	rte_trace_point_emit_u16(port_id);
+	rte_trace_point_emit_u16(queue_id);
+)
+
+RTE_TRACE_POINT_FP(
+	rte_pmd_mlx5_trace_tx_exit,
+	RTE_TRACE_POINT_ARGS(uint16_t nb_sent, uint16_t nb_req),
+	rte_trace_point_emit_u16(nb_sent);
+	rte_trace_point_emit_u16(nb_req);
+)
+
+RTE_TRACE_POINT_FP(
+	rte_pmd_mlx5_trace_tx_wqe,
+	RTE_TRACE_POINT_ARGS(uint32_t opcode),
+	rte_trace_point_emit_u32(opcode);
+)
+
+RTE_TRACE_POINT_FP(
+	rte_pmd_mlx5_trace_tx_wait,
+	RTE_TRACE_POINT_ARGS(uint64_t ts),
+	rte_trace_point_emit_u64(ts);
+)
+
+
+RTE_TRACE_POINT_FP(
+	rte_pmd_mlx5_trace_tx_push,
+	RTE_TRACE_POINT_ARGS(const struct rte_mbuf *mbuf, uint16_t wqe_id),
+	rte_trace_point_emit_ptr(mbuf);
+	rte_trace_point_emit_u32(mbuf->pkt_len);
+	rte_trace_point_emit_u16(mbuf->nb_segs);
+	rte_trace_point_emit_u16(wqe_id);
+)
+
+RTE_TRACE_POINT_FP(
+	rte_pmd_mlx5_trace_tx_complete,
+	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t queue_id,
+			     uint16_t wqe_id, uint64_t ts),
+	rte_trace_point_emit_u16(port_id);
+	rte_trace_point_emit_u16(queue_id);
+	rte_trace_point_emit_u64(ts);
+	rte_trace_point_emit_u16(wqe_id);
+)
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_PMD_MLX5_TRACE_H_ */
diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map
index e05e1aa8c5..d0ec8571e6 100644
--- a/drivers/common/mlx5/version.map
+++ b/drivers/common/mlx5/version.map
@@ -158,5 +158,13 @@  INTERNAL {
 
 	mlx5_os_interrupt_handler_create; # WINDOWS_NO_EXPORT
 	mlx5_os_interrupt_handler_destroy; # WINDOWS_NO_EXPORT
+
+	__rte_pmd_mlx5_trace_tx_entry;
+	__rte_pmd_mlx5_trace_tx_exit;
+	__rte_pmd_mlx5_trace_tx_wqe;
+	__rte_pmd_mlx5_trace_tx_wait;
+	__rte_pmd_mlx5_trace_tx_push;
+	__rte_pmd_mlx5_trace_tx_complete;
+
 	local: *;
 };