[v3,00/33] DPDK Trace support

Message ID	20200329144342.1543749-1-jerinj@marvell.com (mailing list archive)
Headers	From: <jerinj@marvell.com> To: CC: <dev@dpdk.org>, <thomas@monjalon.net>, <bruce.richardson@intel.com>, <david.marchand@redhat.com>, <mattias.ronnblom@ericsson.com>, <skori@marvell.com>, Jerin Jacob <jerinj@marvell.com> Date: Sun, 29 Mar 2020 20:13:09 +0530 Message-ID: <20200329144342.1543749-1-jerinj@marvell.com> In-Reply-To: <20200325211603.240288-1-jerinj@marvell.com> References: <20200325211603.240288-1-jerinj@marvell.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v3 00/33] DPDK Trace support Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>
Series	DPDK Trace support \| [v3,00/33] DPDK Trace support [v3,01/33] meson: add libatomic as a global dependency for i686 clang [v3,02/33] eal: introduce API for getting thread name [v3,03/33] eal: define the public API for trace support [v3,04/33] eal/trace: implement trace register API [v3,05/33] eal/trace: implement trace operation APIs [v3,06/33] eal/trace: add internal trace init and fini interface [v3,07/33] eal/trace: get bootup timestamp for trace [v3,08/33] eal/trace: create CTF TDSL metadata in memory [v3,09/33] eal/trace: handle CTF keyword collision [v3,10/33] eal/trace: implement trace memory allocation [v3,11/33] eal/trace: implement debug dump function [v3,12/33] eal/trace: implement trace save [v3,13/33] eal/trace: implement registration payload [v3,14/33] eal/trace: implement provider payload [v3,15/33] eal/trace: hook internal trace APIs to Linux [v3,16/33] eal/trace: hook internal trace APIs to FreeBSD [v3,17/33] eal/trace: add generic tracepoints [v3,18/33] eal/trace: add alarm tracepoints [v3,19/33] eal/trace: add memory tracepoints [v3,20/33] eal/trace: add memzone tracepoints [v3,21/33] eal/trace: add thread tracepoints [v3,22/33] eal/trace: add interrupt tracepoints [v3,23/33] eal/trace: add trace level configuration parameter [v3,24/33] eal/trace: add trace dir configuration parameter [v3,25/33] eal/trace: add trace bufsize configuration parameter [v3,26/33] eal/trace: add trace mode configuration parameter [v3,27/33] eal/trace: add unit test cases [v3,28/33] eal/trace: add trace performance test cases [v3,29/33] ethdev: add tracepoints [v3,30/33] eventdev: add tracepoints [v3,31/33] cryptodev: add tracepoints [v3,32/33] mempool: add tracepoints [v3,33/33] doc: add trace library guide

Message ID

20200329144342.1543749-1-jerinj@marvell.com (mailing list archive)

Headers

From: <jerinj@marvell.com>
To: 
CC: <dev@dpdk.org>, <thomas@monjalon.net>, <bruce.richardson@intel.com>,
 <david.marchand@redhat.com>, <mattias.ronnblom@ericsson.com>,
 <skori@marvell.com>, Jerin Jacob <jerinj@marvell.com>
Date: Sun, 29 Mar 2020 20:13:09 +0530
Message-ID: <20200329144342.1543749-1-jerinj@marvell.com>
In-Reply-To: <20200325211603.240288-1-jerinj@marvell.com>
References: <20200325211603.240288-1-jerinj@marvell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Subject: [dpdk-dev]  [PATCH v3 00/33] DPDK Trace support
Precedence: list
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Series

DPDK Trace support |

Message

Jerin Jacob Kollanukkaran March 29, 2020, 2:43 p.m. UTC

  From: Jerin Jacob <jerinj@marvell.com>


v3:
~~

1) Fix the following build issues reported by CI
http://mails.dpdk.org/archives/test-report/2020-March/122060.html

a) clang + i686 meson build issue(Fixed in the the patch meson: add libatomic as a global dependency for i686 clang)
b) fixed build issue with FreeBSD with top of tree change.
c) fixed missing experimental API for iavf and ice with meson build in avx512 files.

v2:
~~
Addressed the following review comments from Mattias Rönnblom:

1) Changed 

from:
typedef uint64_t* rte_trace_t;
to
typedef uint64_t rte_trace_t;

Initially thought to make the handle as
struct rte_trace {
    uint64_t val;
}

but changed to uint64_t for the following reasons
a) It is opaque to the application and it will fix the compile-time type
check as well.
b) The handle has an index that will point to an internal slow-path
structure so no ABI change required in the future.
c) RTE_TRACE_POINT_DEFINE need to expose trace object. So it is better
to keep as uint64_t and avoid one more indirection for no use.

2)
Changed:
from:
enum rte_trace_mode_e {
to:
enum rte_trace_mode {


3) removed [out] "found" param from rte_trace_pattern() and
rte_trace_regexp()


4) Changed rte_trace_from_name to rte_trace_by_name

5) rte_trace_is_dp_enabled() return bool now


6) in __rte_trace_point_register() the argument fn change to register_fn

7) removed !! from rte_trace_is_enabled()

8) Remove uninitialized "rc warning" from rte_trace_pattern() and
rte_trace_regexp()

9) fixup bool return type for trace_entry_compare()

10) fixup calloc casting in trace_mkdir()

11) check fclose() return in trace_meta_save() and trace_mem_save()

12) rte_trace_ctf_* macro cleanup

13) added release notes

14) fix build issues reported by CI
http://mails.dpdk.org/archives/test-report/2020-March/121235.html


This patch set contains 
~~~~~~~~~~~~~~~~~~~~~~~~

# The native implementation of common trace format(CTF)[1] based tracer
# Public API to create the trace points.
# Add tracepoints to eal, ethdev, mempool, eventdev and cryptodev 
library for tracing support
# A unit test case
# Performance test case to measure the trace overhead. (See eal/trace:
# add trace performance test cases, patch)
# Programmers guide for Trace support(See doc: add trace library guide,
# patch)

# Tested OS:
~~~~~~~~~~~
- Linux
- FreeBSD

# Tested open source CTF trace viewers
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- Babeltrace
- Tracecompass

# Trace overhead comparison with LTTng
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

trace overhead data on x86:[2]
# 236 cycles with LTTng(>100ns)
# 18 cycles(7ns) with Native DPDK CTF emitter.(See eal/trace: add trace
# performance test cases patch)

trace overhead data on arm64:
#  312  cycles to  1100 cycles with LTTng based on the class of arm64
#  CPU.
#  11 cycles to 13 cycles with Native DPDK CTF emitter based on the
class of arm64 CPU.

18 cycles(on x86) vs 11 cycles(on arm64) is due to rdtsc() overhead in
x86. It seems  rdtsc takes around 15cycles in x86.


Items that needs to be sort it out 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# Makefile and meson.build are updated to allow experimental APIs.

As multiple EXPERIMENTAL symbols, exported by trace library, are
used in various drivers, lib, app and examples. So to fix compilation
warning/error, Makefile and meson.build are updated for all required
components to support EXPERIMENTAL APIs.
It results same code changes at multiple components as well as
increases source code line changes in patchset too.
Suggestions are welcome to resolve this issue with lesser code changes.
	
More details:
~~~~~~~~~~~~~

# The Native DPDK CTF trace support does not have any dependency on
third-party library.
The generated output file is compatible with LTTng as both are using
CTF trace format.

The performance gain comes from:
1) exploit dpdk worker thread usage model to avoid atomics and use per
core variables
2) use hugepage,
3) avoid a lot function pointers in fast-path etc
4) avoid unaligned store for arm64 etc

Features:
~~~~~~~~~
- APIs and Features are similar to rte_log dynamic framework
API(expect log prints on stdout vs it dumps on trace file)
- No specific limit on the events. A string-based event like rte_log
for pattern matching
- Dynamic enable/disable support.
- Instructmention overhead is ~1 cycle. i.e cost of adding the code
wth out using trace feature.
- Timestamp support for all the events using DPDK rte_rtdsc
- No dependency on another library. Clean room native implementation of
  CTF.

Functional test case:
a) echo "trace_autotest" | sudo ./build/app/test/dpdk-test  -c 0x3
--trace-level=8

The above command emits the following trace events
<code>
        uint8_t i;

        rte_trace_lib_eal_generic_void();
        rte_trace_lib_eal_generic_u64(0x10000000000000);
        rte_trace_lib_eal_generic_u32(0x10000000);
        rte_trace_lib_eal_generic_u16(0xffee);
        rte_trace_lib_eal_generic_u8(0xc);
        rte_trace_lib_eal_generic_i64(-1234);
        rte_trace_lib_eal_generic_i32(-1234567);
        rte_trace_lib_eal_generic_i16(12);
        rte_trace_lib_eal_generic_i8(-3);
        rte_trace_lib_eal_generic_string("my string");
        rte_trace_lib_eal_generic_function(__func__);

        for (i = 0; i < 128; i++)
                rte_trace_lib_eal_generic_u8(i);
</code>

Install babeltrace package in Linux and point the generated trace file
to babel trace. By default trace file created under
<user>/dpdk-traces/time_stamp/

example:
# babeltrace /root/dpdk-traces/rte-2020-02-15-PM-02-56-51 | more

[13:27:36.138468807] (+?.?????????) lib.eal.generic.void: { cpu_id =
0, name = "dpdk-test" }, { }
[13:27:36.138468851] (+0.000000044) lib.eal.generic.u64: { cpu_id = 0,
name = "dpdk-test" }, { in = 4503599627370496 }
[13:27:36.138468860] (+0.000000009) lib.eal.generic.u32: { cpu_id = 0,
name = "dpdk-test" }, { in = 268435456 }
[13:27:36.138468934] (+0.000000074) lib.eal.generic.u16: { cpu_id = 0,
name = "dpdk-test" }, { in = 65518 }
[13:27:36.138468949] (+0.000000015) lib.eal.generic.u8: { cpu_id = 0,
name = "dpdk-test" }, { in = 12 }
[13:27:36.138468956] (+0.000000007) lib.eal.generic.i64: { cpu_id = 0,
name = "dpdk-test" }, { in = -1234 }
[13:27:36.138468963] (+0.000000007) lib.eal.generic.i32: { cpu_id = 0,
name = "dpdk-test" }, { in = -1234567 }
[13:27:36.138469024] (+0.000000061) lib.eal.generic.i16: { cpu_id = 0,
name = "dpdk-test" }, { in = 12 }
[13:27:36.138469044] (+0.000000020) lib.eal.generic.i8: { cpu_id = 0,
name = "dpdk-test" }, { in = -3 }
[13:27:36.138469051] (+0.000000007) lib.eal.generic.string: { cpu_id =
0, name = "dpdk-test" }, { str = "my string" }
[13:27:36.138469203] (+0.000000152) lib.eal.generic.func: { cpu_id =
0, name = "dpdk-test" }, { func = "test_trace_points" }
[13:27:36.138469239] (+0.000000036) lib.eal.generic.u8: { cpu_id = 0,
name = "dpdk-test" }, { in = 0 }
[13:27:36.138469246] (+0.000000007) lib.eal.generic.u8: { cpu_id = 0,
name = "dpdk-test" }, { in = 1 }
[13:27:36.138469252] (+0.000000006) lib.eal.generic.u8: { cpu_id = 0,
name = "dpdk-test" }, { in = 2 }
[13:27:36.138469262] (+0.000000010) lib.eal.generic.u8: { cpu_id = 0,
name = "dpdk-test" }, { in = 3 }
[13:27:36.138469269] (+0.000000007) lib.eal.generic.u8: { cpu_id = 0,
name = "dpdk-test" }, { in = 4 }
[13:27:36.138469276] (+0.000000007) lib.eal.generic.u8: { cpu_id = 0,
name = "dpdk-test" }, { in = 5 }

# There is a  GUI based trace viewer available in Windows, Linux and
# Mac.
It is called as tracecompass.(https://www.eclipse.org/tracecompass/)

The example screenshot and Histogram of above DPDK trace using
Tracecompass.

https://github.com/jerinjacobk/share/blob/master/dpdk_trace.JPG



File walk through:
~~~~~~~~~~~~~~~~~~

lib/librte_eal/common/include/rte_trace.h - Public API for Trace
provider and Trace control
lib/librte_eal/common/eal_common_trace.c - main trace implementation
lib/librte_eal/common/eal_common_trace_ctf.c - CTF metadata spec
implementation
lib/librte_eal/common/eal_common_trace_utils.c - command line utils
and filesystem operations.
lib/librte_eal/common/eal_common_trace_points.c -  trace points for EAL
library
lib/librte_eal/common/include/rte_trace_eal.h - EAL tracepoint public
API.
lib/librte_eal/common/eal_trace.h - Private trace header file.


[1] https://diamon.org/ctf/

[2] The above test is ported to LTTng for finding the LTTng trace
overhead. It available at
https://github.com/jerinjacobk/lttng-overhead
https://github.com/jerinjacobk/lttng-overhead/blob/master/README

Jerin Jacob (21):
  eal: introduce API for getting thread name
  eal: define the public API for trace support
  eal/trace: implement trace register API
  eal/trace: implement trace operation APIs
  eal/trace: add internal trace init and fini interface
  eal/trace: get bootup timestamp for trace
  eal/trace: create CTF TDSL metadata in memory
  eal/trace: implement trace memory allocation
  eal/trace: implement debug dump function
  eal/trace: implement trace save
  eal/trace: implement registration payload
  eal/trace: implement provider payload
  eal/trace: hook internal trace APIs to Linux
  eal/trace: hook internal trace APIs to FreeBSD
  eal/trace: add generic tracepoints
  eal/trace: add alarm tracepoints
  eal/trace: add memory tracepoints
  eal/trace: add memzone tracepoints
  eal/trace: add thread tracepoints
  eal/trace: add interrupt tracepoints
  eal/trace: add trace performance test cases

Pavan Nikhilesh (1):
  meson: add libatomic as a global dependency for i686 clang

Sunil Kumar Kori (11):
  eal/trace: handle CTF keyword collision
  eal/trace: add trace level configuration parameter
  eal/trace: add trace dir configuration parameter
  eal/trace: add trace bufsize configuration parameter
  eal/trace: add trace mode configuration parameter
  eal/trace: add unit test cases
  ethdev: add tracepoints
  eventdev: add tracepoints
  cryptodev: add tracepoints
  mempool: add tracepoints
  doc: add trace library guide

 MAINTAINERS                                   |  15 +
 app/pdump/Makefile                            |   1 +
 app/pdump/meson.build                         |   1 +
 app/proc-info/Makefile                        |   1 +
 app/proc-info/meson.build                     |   1 +
 app/test-acl/Makefile                         |   1 +
 app/test-acl/meson.build                      |   1 +
 app/test-cmdline/Makefile                     |   1 +
 app/test-cmdline/meson.build                  |   1 +
 app/test-eventdev/Makefile                    |   1 +
 app/test-eventdev/meson.build                 |   1 +
 app/test-pipeline/Makefile                    |   1 +
 app/test-pipeline/meson.build                 |   1 +
 app/test/Makefile                             |   4 +-
 app/test/meson.build                          |   3 +
 app/test/test_trace.c                         | 618 ++++++++++++++++++
 app/test/test_trace.h                         |  52 ++
 app/test/test_trace_perf.c                    | 179 +++++
 app/test/test_trace_register.c                |  46 ++
 config/common_base                            |   1 +
 config/meson.build                            |  10 +
 config/rte_config.h                           |   1 +
 doc/api/doxy-api-index.md                     |   3 +-
 doc/guides/linux_gsg/eal_args.include.rst     |  55 ++
 doc/guides/prog_guide/index.rst               |   1 +
 doc/guides/prog_guide/trace_lib.rst           | 265 ++++++++
 doc/guides/rel_notes/release_20_05.rst        |   9 +
 drivers/common/cpt/Makefile                   |   1 +
 drivers/crypto/ccp/Makefile                   |   1 +
 drivers/crypto/ccp/meson.build                |   2 +
 drivers/crypto/mvsam/Makefile                 |   1 +
 drivers/crypto/mvsam/meson.build              |   1 +
 drivers/crypto/null/Makefile                  |   1 +
 drivers/crypto/null/meson.build               |   1 +
 drivers/crypto/scheduler/Makefile             |   1 +
 drivers/crypto/scheduler/meson.build          |   1 +
 drivers/crypto/virtio/Makefile                |   1 +
 drivers/crypto/virtio/meson.build             |   1 +
 drivers/event/octeontx/Makefile               |   1 +
 drivers/event/octeontx/meson.build            |   6 +-
 drivers/event/octeontx2/meson.build           |   5 -
 drivers/event/opdl/meson.build                |   5 -
 drivers/event/skeleton/Makefile               |   1 +
 drivers/event/skeleton/meson.build            |   1 +
 drivers/event/sw/Makefile                     |   1 +
 drivers/event/sw/meson.build                  |   1 +
 drivers/mempool/ring/Makefile                 |   1 +
 drivers/mempool/ring/meson.build              |   1 +
 drivers/net/af_packet/Makefile                |   1 +
 drivers/net/af_packet/meson.build             |   1 +
 drivers/net/af_xdp/Makefile                   |   1 +
 drivers/net/af_xdp/meson.build                |   1 +
 drivers/net/ark/Makefile                      |   1 +
 drivers/net/ark/meson.build                   |   1 +
 drivers/net/bnxt/Makefile                     |   1 +
 drivers/net/bnxt/meson.build                  |   1 +
 drivers/net/cxgbe/Makefile                    |   1 +
 drivers/net/cxgbe/meson.build                 |   1 +
 drivers/net/enetc/Makefile                    |   1 +
 drivers/net/enetc/meson.build                 |   1 +
 drivers/net/hinic/Makefile                    |   1 +
 drivers/net/hinic/base/meson.build            |   3 +-
 drivers/net/hinic/meson.build                 |   1 +
 drivers/net/iavf/meson.build                  |   3 +
 drivers/net/ice/meson.build                   |   3 +
 drivers/net/ionic/ionic_dev.c                 |   1 +
 drivers/net/ionic/ionic_mac_api.c             |   1 +
 drivers/net/ionic/ionic_main.c                |   1 +
 drivers/net/kni/Makefile                      |   1 +
 drivers/net/kni/meson.build                   |   1 +
 drivers/net/liquidio/Makefile                 |   1 +
 drivers/net/liquidio/meson.build              |   1 +
 drivers/net/mvneta/Makefile                   |   1 +
 drivers/net/mvneta/meson.build                |   1 +
 drivers/net/mvpp2/Makefile                    |   1 +
 drivers/net/mvpp2/meson.build                 |   1 +
 drivers/net/nfb/Makefile                      |   1 +
 drivers/net/nfb/meson.build                   |   1 +
 drivers/net/null/Makefile                     |   1 +
 drivers/net/null/meson.build                  |   1 +
 drivers/net/octeontx/Makefile                 |   1 +
 drivers/net/octeontx2/Makefile                |   1 +
 drivers/net/octeontx2/meson.build             |   1 +
 drivers/net/pcap/Makefile                     |   1 +
 drivers/net/pcap/meson.build                  |   1 +
 drivers/net/ring/Makefile                     |   1 +
 drivers/net/ring/meson.build                  |   1 +
 drivers/net/szedata2/Makefile                 |   1 +
 drivers/net/szedata2/meson.build              |   1 +
 drivers/net/thunderx/base/meson.build         |   1 +
 drivers/net/vhost/Makefile                    |   1 +
 drivers/net/vhost/meson.build                 |   1 +
 drivers/raw/ioat/Makefile                     |   1 +
 drivers/raw/ioat/meson.build                  |   1 +
 drivers/raw/octeontx2_dma/Makefile            |   1 +
 drivers/raw/octeontx2_dma/meson.build         |   1 +
 drivers/raw/octeontx2_ep/Makefile             |   1 +
 drivers/raw/octeontx2_ep/meson.build          |   1 +
 drivers/raw/skeleton/Makefile                 |   1 +
 drivers/raw/skeleton/meson.build              |   1 +
 examples/cmdline/Makefile                     |   1 +
 examples/cmdline/meson.build                  |   1 +
 examples/distributor/Makefile                 |   1 +
 examples/distributor/meson.build              |   1 +
 examples/ethtool/ethtool-app/Makefile         |   1 +
 examples/eventdev_pipeline/meson.build        |   1 +
 examples/flow_filtering/Makefile              |   1 +
 examples/flow_filtering/meson.build           |   1 +
 examples/helloworld/Makefile                  |   1 +
 examples/helloworld/meson.build               |   1 +
 examples/ioat/Makefile                        |   1 +
 examples/ioat/meson.build                     |   1 +
 examples/ip_fragmentation/Makefile            |   2 +
 examples/ip_fragmentation/meson.build         |   1 +
 examples/ip_reassembly/Makefile               |   1 +
 examples/ip_reassembly/meson.build            |   1 +
 examples/ipv4_multicast/Makefile              |   1 +
 examples/ipv4_multicast/meson.build           |   1 +
 examples/l2fwd-cat/Makefile                   |   1 +
 examples/l2fwd-cat/meson.build                |   1 +
 examples/l2fwd-event/Makefile                 |   1 +
 examples/l2fwd-event/meson.build              |   6 +-
 examples/l2fwd-jobstats/Makefile              |   1 +
 examples/l2fwd-jobstats/meson.build           |   1 +
 examples/l2fwd-keepalive/Makefile             |   1 +
 examples/l2fwd-keepalive/ka-agent/Makefile    |   1 +
 examples/l2fwd-keepalive/meson.build          |   1 +
 examples/l3fwd-acl/Makefile                   |   1 +
 examples/l3fwd-acl/meson.build                |   1 +
 examples/l3fwd/Makefile                       |   1 +
 examples/l3fwd/meson.build                    |   1 +
 examples/link_status_interrupt/Makefile       |   1 +
 examples/link_status_interrupt/meson.build    |   1 +
 .../client_server_mp/mp_client/Makefile       |   1 +
 .../client_server_mp/mp_client/meson.build    |   1 +
 .../client_server_mp/mp_server/meson.build    |   1 +
 examples/multi_process/hotplug_mp/Makefile    |   1 +
 examples/multi_process/hotplug_mp/meson.build |   1 +
 examples/multi_process/simple_mp/Makefile     |   1 +
 examples/multi_process/simple_mp/meson.build  |   1 +
 examples/multi_process/symmetric_mp/Makefile  |   1 +
 .../multi_process/symmetric_mp/meson.build    |   1 +
 examples/ntb/Makefile                         |   1 +
 examples/ntb/meson.build                      |   1 +
 examples/packet_ordering/Makefile             |   1 +
 examples/packet_ordering/meson.build          |   1 +
 .../performance-thread/l3fwd-thread/Makefile  |   1 +
 .../l3fwd-thread/meson.build                  |   1 +
 .../performance-thread/pthread_shim/Makefile  |   1 +
 .../pthread_shim/meson.build                  |   1 +
 examples/ptpclient/Makefile                   |   1 +
 examples/ptpclient/meson.build                |   1 +
 examples/qos_meter/Makefile                   |   1 +
 examples/qos_meter/meson.build                |   1 +
 examples/qos_sched/Makefile                   |   1 +
 examples/qos_sched/meson.build                |   1 +
 examples/server_node_efd/node/Makefile        |   1 +
 examples/server_node_efd/node/meson.build     |   1 +
 examples/server_node_efd/server/Makefile      |   1 +
 examples/server_node_efd/server/meson.build   |   1 +
 examples/service_cores/Makefile               |   1 +
 examples/service_cores/meson.build            |   1 +
 examples/skeleton/Makefile                    |   1 +
 examples/skeleton/meson.build                 |   1 +
 examples/timer/Makefile                       |   1 +
 examples/timer/meson.build                    |   1 +
 examples/vm_power_manager/Makefile            |   1 +
 examples/vm_power_manager/meson.build         |   1 +
 examples/vmdq/Makefile                        |   1 +
 examples/vmdq/meson.build                     |   1 +
 examples/vmdq_dcb/Makefile                    |   1 +
 examples/vmdq_dcb/meson.build                 |   1 +
 lib/librte_bitratestats/Makefile              |   1 +
 lib/librte_bitratestats/meson.build           |   1 +
 lib/librte_cryptodev/Makefile                 |   4 +-
 lib/librte_cryptodev/cryptodev_trace_points.c |  70 ++
 lib/librte_cryptodev/meson.build              |   7 +-
 lib/librte_cryptodev/rte_cryptodev.c          |  18 +
 lib/librte_cryptodev/rte_cryptodev.h          |   6 +
 .../rte_cryptodev_version.map                 |  18 +
 lib/librte_cryptodev/rte_trace_cryptodev.h    | 133 ++++
 lib/librte_cryptodev/rte_trace_cryptodev_fp.h |  34 +
 lib/librte_distributor/Makefile               |   1 +
 lib/librte_distributor/meson.build            |   6 +-
 lib/librte_eal/common/Makefile                |   1 +
 lib/librte_eal/common/eal_common_log.c        |   9 +-
 lib/librte_eal/common/eal_common_memzone.c    |   9 +
 lib/librte_eal/common/eal_common_options.c    |  68 +-
 lib/librte_eal/common/eal_common_thread.c     |   3 +-
 lib/librte_eal/common/eal_common_trace.c      | 610 +++++++++++++++++
 lib/librte_eal/common/eal_common_trace_ctf.c  | 488 ++++++++++++++
 .../common/eal_common_trace_points.c          | 115 ++++
 .../common/eal_common_trace_utils.c           | 523 +++++++++++++++
 lib/librte_eal/common/eal_options.h           |   8 +
 lib/librte_eal/common/eal_private.h           |  11 +
 lib/librte_eal/common/eal_trace.h             | 122 ++++
 lib/librte_eal/common/include/rte_lcore.h     |  17 +
 lib/librte_eal/common/include/rte_trace.h     | 584 +++++++++++++++++
 lib/librte_eal/common/include/rte_trace_eal.h | 247 +++++++
 .../common/include/rte_trace_provider.h       | 137 ++++
 .../common/include/rte_trace_register.h       |  53 ++
 lib/librte_eal/common/meson.build             |   8 +
 lib/librte_eal/common/rte_malloc.c            |  60 +-
 lib/librte_eal/freebsd/eal/Makefile           |   4 +
 lib/librte_eal/freebsd/eal/eal.c              |  10 +
 lib/librte_eal/freebsd/eal/eal_alarm.c        |   3 +
 lib/librte_eal/freebsd/eal/eal_interrupts.c   |  54 +-
 lib/librte_eal/freebsd/eal/eal_thread.c       |  21 +-
 lib/librte_eal/linux/eal/Makefile             |   4 +
 lib/librte_eal/linux/eal/eal.c                |   9 +
 lib/librte_eal/linux/eal/eal_alarm.c          |   4 +
 lib/librte_eal/linux/eal/eal_interrupts.c     |  84 ++-
 lib/librte_eal/linux/eal/eal_thread.c         |  27 +-
 lib/librte_eal/rte_eal_version.map            |  59 ++
 lib/librte_ethdev/Makefile                    |   3 +
 lib/librte_ethdev/ethdev_trace_points.c       |  43 ++
 lib/librte_ethdev/meson.build                 |   5 +-
 lib/librte_ethdev/rte_ethdev.c                |  12 +
 lib/librte_ethdev/rte_ethdev.h                |   5 +
 lib/librte_ethdev/rte_ethdev_version.map      |  10 +
 lib/librte_ethdev/rte_trace_ethdev.h          |  90 +++
 lib/librte_ethdev/rte_trace_ethdev_fp.h       |  40 ++
 lib/librte_eventdev/Makefile                  |   3 +
 lib/librte_eventdev/eventdev_trace_points.c   | 173 +++++
 lib/librte_eventdev/meson.build               |   3 +
 .../rte_event_crypto_adapter.c                |  10 +
 .../rte_event_eth_rx_adapter.c                |  11 +
 .../rte_event_eth_tx_adapter.c                |  13 +-
 .../rte_event_eth_tx_adapter.h                |   2 +
 lib/librte_eventdev/rte_event_timer_adapter.c |   8 +-
 lib/librte_eventdev/rte_event_timer_adapter.h |   8 +
 lib/librte_eventdev/rte_eventdev.c            |   9 +
 lib/librte_eventdev/rte_eventdev.h            |   5 +-
 lib/librte_eventdev/rte_eventdev_version.map  |  42 ++
 lib/librte_eventdev/rte_trace_eventdev.h      | 278 ++++++++
 lib/librte_eventdev/rte_trace_eventdev_fp.h   |  75 +++
 lib/librte_gro/Makefile                       |   1 +
 lib/librte_gro/meson.build                    |   1 +
 lib/librte_gso/Makefile                       |   1 +
 lib/librte_gso/meson.build                    |   1 +
 lib/librte_ip_frag/Makefile                   |   1 +
 lib/librte_ip_frag/meson.build                |   1 +
 lib/librte_kni/Makefile                       |   1 +
 lib/librte_kni/meson.build                    |   1 +
 lib/librte_latencystats/Makefile              |   1 +
 lib/librte_latencystats/meson.build           |   1 +
 lib/librte_mempool/Makefile                   |   3 +
 lib/librte_mempool/mempool_trace_points.c     | 108 +++
 lib/librte_mempool/meson.build                |   7 +-
 lib/librte_mempool/rte_mempool.c              |  16 +
 lib/librte_mempool/rte_mempool.h              |  13 +
 lib/librte_mempool/rte_mempool_ops.c          |   7 +
 lib/librte_mempool/rte_mempool_version.map    |  26 +
 lib/librte_mempool/rte_trace_mempool.h        | 148 +++++
 lib/librte_mempool/rte_trace_mempool_fp.h     | 102 +++
 lib/librte_port/Makefile                      |   1 +
 lib/librte_port/meson.build                   |   1 +
 lib/librte_rcu/meson.build                    |   5 -
 lib/librte_reorder/Makefile                   |   1 +
 lib/librte_reorder/meson.build                |   1 +
 lib/librte_sched/Makefile                     |   1 +
 lib/librte_sched/meson.build                  |   1 +
 lib/librte_security/Makefile                  |   1 +
 lib/librte_security/meson.build               |   1 +
 lib/librte_table/Makefile                     |   1 +
 lib/librte_table/meson.build                  |   1 +
 266 files changed, 6286 insertions(+), 113 deletions(-)
 create mode 100644 app/test/test_trace.c
 create mode 100644 app/test/test_trace.h
 create mode 100644 app/test/test_trace_perf.c
 create mode 100644 app/test/test_trace_register.c
 create mode 100644 doc/guides/prog_guide/trace_lib.rst
 create mode 100644 lib/librte_cryptodev/cryptodev_trace_points.c
 create mode 100644 lib/librte_cryptodev/rte_trace_cryptodev.h
 create mode 100644 lib/librte_cryptodev/rte_trace_cryptodev_fp.h
 create mode 100644 lib/librte_eal/common/eal_common_trace.c
 create mode 100644 lib/librte_eal/common/eal_common_trace_ctf.c
 create mode 100644 lib/librte_eal/common/eal_common_trace_points.c
 create mode 100644 lib/librte_eal/common/eal_common_trace_utils.c
 create mode 100644 lib/librte_eal/common/eal_trace.h
 create mode 100644 lib/librte_eal/common/include/rte_trace.h
 create mode 100644 lib/librte_eal/common/include/rte_trace_eal.h
 create mode 100644 lib/librte_eal/common/include/rte_trace_provider.h
 create mode 100644 lib/librte_eal/common/include/rte_trace_register.h
 create mode 100644 lib/librte_ethdev/ethdev_trace_points.c
 create mode 100644 lib/librte_ethdev/rte_trace_ethdev.h
 create mode 100644 lib/librte_ethdev/rte_trace_ethdev_fp.h
 create mode 100644 lib/librte_eventdev/eventdev_trace_points.c
 create mode 100644 lib/librte_eventdev/rte_trace_eventdev.h
 create mode 100644 lib/librte_eventdev/rte_trace_eventdev_fp.h
 create mode 100644 lib/librte_mempool/mempool_trace_points.c
 create mode 100644 lib/librte_mempool/rte_trace_mempool.h
 create mode 100644 lib/librte_mempool/rte_trace_mempool_fp.h

Comments

David Marchand April 1, 2020, 8:18 a.m. UTC | #1

Hello Jerin,

On Sun, Mar 29, 2020 at 4:43 PM <jerinj@marvell.com> wrote:
> Items that needs to be sort it out
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> # Makefile and meson.build are updated to allow experimental APIs.
>
> As multiple EXPERIMENTAL symbols, exported by trace library, are
> used in various drivers, lib, app and examples. So to fix compilation
> warning/error, Makefile and meson.build are updated for all required
> components to support EXPERIMENTAL APIs.
> It results same code changes at multiple components as well as
> increases source code line changes in patchset too.
> Suggestions are welcome to resolve this issue with lesser code changes.

- Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag
gates access to APIs so that external users are aware of their status.
I have been considering setting this flag unconditionally for internal
users in the top Makefile/meson for app/ lib/ and drivers/.
I could look at this and prepare a patch about this, but this is not
enough here.

With the trace framework, we add experimental symbols in mempool or
ethdev inlines, which then inherit the experimental check.
This would break compilation of external code. Users are then forced
to enable experimental API.

How about:
* we introduce a global config switch that enables/disables the trace
framework (off by default): the people who want to test it and help
stabilize it will have to deal with the experimental flag for now,
* in 20.11, the trace points are put into the stable ABI, and the
option is removed,


- With the patchset rebased on the current master (could be my fault,
so take it with a grain of salt), the ALLOW_EXPERIMENTAL_API flag is
not passed when compiling the l3fwd example against an installed dpdk.


- On the MAINTAINERS update:

Trace
M: Jerin Jacob <jerinj@marvell.com>
M: Sunil Kumar Kori <skori@marvell.com>
F: lib/librte_eal/include/rte_trace*.h
F: lib/librte_eal/common/eal_common_trace*.c
F: lib/librte_eal/common/eal_trace.h
F: lib/librte_ethdev/ethdev_trace_points.c
F: lib/librte_ethdev/rte_trace_ethdev*.h
F: lib/librte_eventdev/eventdev_trace_points.c
F: lib/librte_eventdev/rte_trace_eventdev*.h
F: lib/librte_cryptodev/cryptodev_trace_points.c
F: lib/librte_cryptodev/rte_trace_cryptodev*.h
F: lib/librte_mempool/mempool_trace_points.c
F: lib/librte_mempool/rte_trace_mempool*.h

Once we exclude the trace framework, the trace points themselves
should be the responsibility of the component (ethdev, eventdev...)
maintainers (copied them).


- I had something more dynamic in mind for gathering traces: like
enabling/disabling trace points in a running process and getting the
traces on the fly.
But I suppose the current framework is enough and we can work on this later.



>
> More details:
> ~~~~~~~~~~~~~
>
> # The Native DPDK CTF trace support does not have any dependency on
> third-party library.
> The generated output file is compatible with LTTng as both are using
> CTF trace format.
>
> The performance gain comes from:
> 1) exploit dpdk worker thread usage model to avoid atomics and use per
> core variables
> 2) use hugepage,
> 3) avoid a lot function pointers in fast-path etc
> 4) avoid unaligned store for arm64 etc
>
> Features:
> ~~~~~~~~~
> - APIs and Features are similar to rte_log dynamic framework
> API(expect log prints on stdout vs it dumps on trace file)

I am not sure about the global and per tracepoint levels.
We must enable each tracepoint anyway, the level is just a commodity.

I don't have strong arguments against it, but it feels odd to
copy/paste the rte_log framework.

Jerin Jacob April 1, 2020, 10:04 a.m. UTC | #2

On Wed, Apr 1, 2020 at 1:49 PM David Marchand <david.marchand@redhat.com> wrote:
>
> Hello Jerin,

Hello David,

Thanks for the review.

>
> On Sun, Mar 29, 2020 at 4:43 PM <jerinj@marvell.com> wrote:
> > Items that needs to be sort it out
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > # Makefile and meson.build are updated to allow experimental APIs.
> >
> > As multiple EXPERIMENTAL symbols, exported by trace library, are
> > used in various drivers, lib, app and examples. So to fix compilation
> > warning/error, Makefile and meson.build are updated for all required
> > components to support EXPERIMENTAL APIs.
> > It results same code changes at multiple components as well as
> > increases source code line changes in patchset too.
> > Suggestions are welcome to resolve this issue with lesser code changes.
>
> - Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag
> gates access to APIs so that external users are aware of their status.
> I have been considering setting this flag unconditionally for internal
> users in the top Makefile/meson for app/ lib/ and drivers/.
> I could look at this and prepare a patch about this, but this is not
> enough here.

It makes sense to me. Let me know when you are planning to send that patch,
I will rebase this series on top of that.

If you don't have time then I can send the patch too.
I assume the patch content will be:
1) Removing experimental API from app, lib, drivers, examples with
make and meson
2) Have it enabled at the global level.

> With the trace framework, we add experimental symbols in mempool or
> ethdev inlines, which then inherit the experimental check.
> This would break compilation of external code. Users are then forced
> to enable experimental API.

I agree with the analysis.

>
> How about:
> * we introduce a global config switch that enables/disables the trace
> framework (off by default): the people who want to test it and help
> stabilize it will have to deal with the experimental flag for now,
> * in 20.11, the trace points are put into the stable ABI, and the
> option is removed,

IMO, the better alternative  would be:

Since the trace changes in the "inline" functions of the specific
library already
disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and
it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path
trace like RTE_TRACE_POINT().
So I can improve RTE_TRACE_POINT_DP() to make absolute NOP if
ALLOW_EXPERIMENTAL_API not defined.

On the upside,
The tracing code will be enabled by default(runtime it is disabled by
default anyway).
If some need to fastpath API tracing then  ALLOW_EXPERIMENTAL_API need
to enable.
So this won't break applications.

>
>
> - With the patchset rebased on the current master (could be my fault,
> so take it with a grain of salt), the ALLOW_EXPERIMENTAL_API flag is
> not passed when compiling the l3fwd example against an installed dpdk.

I will check. We have added ALLOW_EXPERIMENTAL_API flag where we got
compilation issues.

>
>
> - On the MAINTAINERS update:
>
> Trace
> M: Jerin Jacob <jerinj@marvell.com>
> M: Sunil Kumar Kori <skori@marvell.com>
> F: lib/librte_eal/include/rte_trace*.h
> F: lib/librte_eal/common/eal_common_trace*.c
> F: lib/librte_eal/common/eal_trace.h
> F: lib/librte_ethdev/ethdev_trace_points.c
> F: lib/librte_ethdev/rte_trace_ethdev*.h
> F: lib/librte_eventdev/eventdev_trace_points.c
> F: lib/librte_eventdev/rte_trace_eventdev*.h
> F: lib/librte_cryptodev/cryptodev_trace_points.c
> F: lib/librte_cryptodev/rte_trace_cryptodev*.h
> F: lib/librte_mempool/mempool_trace_points.c
> F: lib/librte_mempool/rte_trace_mempool*.h
>
> Once we exclude the trace framework, the trace points themselves
> should be the responsibility of the component (ethdev, eventdev...)
> maintainers (copied them).

Yes. I will remove the following from the MAINTAINERS update in the
next version.

 F: lib/librte_ethdev/ethdev_trace_points.c
 F: lib/librte_ethdev/rte_trace_ethdev*.h
 F: lib/librte_eventdev/eventdev_trace_points.c
 F: lib/librte_eventdev/rte_trace_eventdev*.h
 F: lib/librte_cryptodev/cryptodev_trace_points.c
 F: lib/librte_cryptodev/rte_trace_cryptodev*.h
 F: lib/librte_mempool/mempool_trace_points.c
 F: lib/librte_mempool/rte_trace_mempool*.h


>
>
> - I had something more dynamic in mind for gathering traces: like
> enabling/disabling trace points in a running process and getting the
> traces on the fly.

rte_trace_enable() and rte_trace_disable() already avilable for this.

> But I suppose the current framework is enough and we can work on this later.

Yes. for all new features.

>
>
>
> >
> > More details:
> > ~~~~~~~~~~~~~
> >
> > # The Native DPDK CTF trace support does not have any dependency on
> > third-party library.
> > The generated output file is compatible with LTTng as both are using
> > CTF trace format.
> >
> > The performance gain comes from:
> > 1) exploit dpdk worker thread usage model to avoid atomics and use per
> > core variables
> > 2) use hugepage,
> > 3) avoid a lot function pointers in fast-path etc
> > 4) avoid unaligned store for arm64 etc
> >
> > Features:
> > ~~~~~~~~~
> > - APIs and Features are similar to rte_log dynamic framework
> > API(expect log prints on stdout vs it dumps on trace file)
>
> I am not sure about the global and per tracepoint levels.
> We must enable each tracepoint anyway, the level is just a commodity.
>
> I don't have strong arguments against it, but it feels odd to
> copy/paste the rte_log framework.

I have intentionally kept public API similar to rte_log wherever it is
possible. Reason being,
1) In the future, it is easy to replace rte_log with trace _if needed_.
2) Avoid the new API learning curve.

/Jerin


>
>
> --
> David Marchand
>

David Marchand April 1, 2020, 2:12 p.m. UTC | #3

On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> On Wed, Apr 1, 2020 at 1:49 PM David Marchand <david.marchand@redhat.com> wrote:
> > - Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag
> > gates access to APIs so that external users are aware of their status.
> > I have been considering setting this flag unconditionally for internal
> > users in the top Makefile/meson for app/ lib/ and drivers/.
> > I could look at this and prepare a patch about this, but this is not
> > enough here.
>
> It makes sense to me. Let me know when you are planning to send that patch,
> I will rebase this series on top of that.

Feel free to take over, thanks.


>
> If you don't have time then I can send the patch too.
> I assume the patch content will be:
> 1) Removing experimental API from app, lib, drivers, examples with
> make and meson
> 2) Have it enabled at the global level.

examples are a special case as they can be compiled out of the dpdk sources.
This is why I excluded them of the list in my mail before.


> > How about:
> > * we introduce a global config switch that enables/disables the trace
> > framework (off by default): the people who want to test it and help
> > stabilize it will have to deal with the experimental flag for now,
> > * in 20.11, the trace points are put into the stable ABI, and the
> > option is removed,
>
> IMO, the better alternative  would be:
>
> Since the trace changes in the "inline" functions of the specific
> library already
> disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and
> it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path
> trace like RTE_TRACE_POINT().

Ah indeed.
Note: RTE_ENABLE_TRACE_DP is not plugged with meson.


> So I can improve RTE_TRACE_POINT_DP() to make absolute NOP if
> ALLOW_EXPERIMENTAL_API not defined.
>
> On the upside,
> The tracing code will be enabled by default(runtime it is disabled by
> default anyway).
> If some need to fastpath API tracing then  ALLOW_EXPERIMENTAL_API need
> to enable.
> So this won't break applications.

So either keep the RTE_ENABLE_TRACE_DP flag or use
ALLOW_EXPERIMENTAL_API... no opinion.
Thomas?


> > - With the patchset rebased on the current master (could be my fault,
> > so take it with a grain of salt), the ALLOW_EXPERIMENTAL_API flag is
> > not passed when compiling the l3fwd example against an installed dpdk.
>
> I will check. We have added ALLOW_EXPERIMENTAL_API flag where we got
> compilation issues.

No compilation issue, just big fat warnings reporting that l3fwd did
not enable ALLOW_EXPERIMENTAL_API and it is not built with -Werror.
I caught it with ./devtools/test-*build*.sh scripts.

Thomas Monjalon April 1, 2020, 2:16 p.m. UTC | #4

01/04/2020 16:12, David Marchand:
> On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > On Wed, Apr 1, 2020 at 1:49 PM David Marchand <david.marchand@redhat.com> wrote:
> > > - Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag
> > > gates access to APIs so that external users are aware of their status.
> > > I have been considering setting this flag unconditionally for internal
> > > users in the top Makefile/meson for app/ lib/ and drivers/.
> > > I could look at this and prepare a patch about this, but this is not
> > > enough here.
> >
> > It makes sense to me. Let me know when you are planning to send that patch,
> > I will rebase this series on top of that.
> 
> Feel free to take over, thanks.
> 
> 
> >
> > If you don't have time then I can send the patch too.
> > I assume the patch content will be:
> > 1) Removing experimental API from app, lib, drivers, examples with
> > make and meson
> > 2) Have it enabled at the global level.
> 
> examples are a special case as they can be compiled out of the dpdk sources.
> This is why I excluded them of the list in my mail before.
> 
> 
> > > How about:
> > > * we introduce a global config switch that enables/disables the trace
> > > framework (off by default): the people who want to test it and help
> > > stabilize it will have to deal with the experimental flag for now,
> > > * in 20.11, the trace points are put into the stable ABI, and the
> > > option is removed,
> >
> > IMO, the better alternative  would be:
> >
> > Since the trace changes in the "inline" functions of the specific
> > library already
> > disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and
> > it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path
> > trace like RTE_TRACE_POINT().
> 
> Ah indeed.
> Note: RTE_ENABLE_TRACE_DP is not plugged with meson.
> 
> 
> > So I can improve RTE_TRACE_POINT_DP() to make absolute NOP if
> > ALLOW_EXPERIMENTAL_API not defined.
> >
> > On the upside,
> > The tracing code will be enabled by default(runtime it is disabled by
> > default anyway).
> > If some need to fastpath API tracing then  ALLOW_EXPERIMENTAL_API need
> > to enable.
> > So this won't break applications.
> 
> So either keep the RTE_ENABLE_TRACE_DP flag or use
> ALLOW_EXPERIMENTAL_API... no opinion.
> Thomas?

Anyway we need a compile-time option?
The option is just for compatibility?
Then ALLOW_EXPERIMENTAL_API looks to be the right option.

Jerin Jacob April 1, 2020, 4:25 p.m. UTC | #5

On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > On Wed, Apr 1, 2020 at 1:49 PM David Marchand <david.marchand@redhat.com> wrote:
> > > - Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag
> > > gates access to APIs so that external users are aware of their status.
> > > I have been considering setting this flag unconditionally for internal
> > > users in the top Makefile/meson for app/ lib/ and drivers/.
> > > I could look at this and prepare a patch about this, but this is not
> > > enough here.
> >
> > It makes sense to me. Let me know when you are planning to send that patch,
> > I will rebase this series on top of that.
>
> Feel free to take over, thanks.

OK.

>
>
> >
> > If you don't have time then I can send the patch too.
> > I assume the patch content will be:
> > 1) Removing experimental API from app, lib, drivers, examples with
> > make and meson
> > 2) Have it enabled at the global level.
>
> examples are a special case as they can be compiled out of the dpdk sources.
> This is why I excluded them of the list in my mail before.

There are a lot of examples already that have ALLOW_EXPERIMENTAL_API.
What is the issue in enabling ALLOW_EXPERIMENTAL_API  in the in tree
example application?

> >
> > Since the trace changes in the "inline" functions of the specific
> > library already
> > disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and
> > it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path
> > trace like RTE_TRACE_POINT().
>
> Ah indeed.
> Note: RTE_ENABLE_TRACE_DP is not plugged with meson.

If you see the " eal: define the public API for trace support" patch,
I have updated rte_config.h for meson likewise for other configuration
such log for DP.
Would you like to have a meson option? and update in meson_options.txt.

I don't have a strong opinion on either way? Bruce ,any thoughts?

diff --git a/config/rte_config.h b/config/rte_config.h
index d30786bc0..6b250288c 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -42,6 +42,7 @@
 #define RTE_MAX_MEMZONE 2560
 #define RTE_MAX_TAILQ 32
 #define RTE_LOG_DP_LEVEL RTE_LOG_INFO
+#define RTE_ENABLE_TRACE_DP 0
 #define RTE_BACKTRACE 1
 #define RTE_MAX_VFIO_CONTAINERS 64

>
>
> > So I can improve RTE_TRACE_POINT_DP() to make absolute NOP if
> > ALLOW_EXPERIMENTAL_API not defined.
> >
> > On the upside,
> > The tracing code will be enabled by default(runtime it is disabled by
> > default anyway).
> > If some need to fastpath API tracing then  ALLOW_EXPERIMENTAL_API need
> > to enable.
> > So this won't break applications.
>
> So either keep the RTE_ENABLE_TRACE_DP flag or use
> ALLOW_EXPERIMENTAL_API... no opinion.
> Thomas?
> Anyway we need a compile-time option?
> The option is just for compatibility?
> Then ALLOW_EXPERIMENTAL_API looks to be the right option.

Just like log there is a compile-time option( CONFIG_RTE_LOG_DP_LEVEL for log)
to disable trace generation in fastpath using RTE_ENABLE_TRACE_DP.
This is to avoid "any" performance impact in fastpath code due to tracer.

ALLOW_EXPERIMENTAL_API  and RTE_ENABLE_TRACE_DP has different scope.
I will add ALLOW_EXPERIMENTAL_API  and RTE_ENABLE_TRACE_DP.
ALLOW_EXPERIMENTAL_API  can be removed later when it is stable.

NOTE:
I have used if (0) c style approach in the code to disable the code
with RTE_ENABLE_TRACE_DP.
This is to make sure the code always has compilation check respective
RTE_ENABLE_TRACE_DP status.
So ALLOW_EXPERIMENTAL_API   need to avoid code compilation issues with
external applications.

Bruce Richardson April 1, 2020, 4:46 p.m. UTC | #6

On Wed, Apr 01, 2020 at 09:55:20PM +0530, Jerin Jacob wrote:
> On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.marchand@redhat.com> wrote:
> >
> > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > On Wed, Apr 1, 2020 at 1:49 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > - Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag
> > > > gates access to APIs so that external users are aware of their status.
> > > > I have been considering setting this flag unconditionally for internal
> > > > users in the top Makefile/meson for app/ lib/ and drivers/.
> > > > I could look at this and prepare a patch about this, but this is not
> > > > enough here.
> > >
> > > It makes sense to me. Let me know when you are planning to send that patch,
> > > I will rebase this series on top of that.
> >
> > Feel free to take over, thanks.
> 
> OK.
> 
> >
> >
> > >
> > > If you don't have time then I can send the patch too.
> > > I assume the patch content will be:
> > > 1) Removing experimental API from app, lib, drivers, examples with
> > > make and meson
> > > 2) Have it enabled at the global level.
> >
> > examples are a special case as they can be compiled out of the dpdk sources.
> > This is why I excluded them of the list in my mail before.
> 
> There are a lot of examples already that have ALLOW_EXPERIMENTAL_API.
> What is the issue in enabling ALLOW_EXPERIMENTAL_API  in the in tree
> example application?
> 
> 
> 
> > >
> > > Since the trace changes in the "inline" functions of the specific
> > > library already
> > > disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and
> > > it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path
> > > trace like RTE_TRACE_POINT().
> >
> > Ah indeed.
> > Note: RTE_ENABLE_TRACE_DP is not plugged with meson.
> 
> If you see the " eal: define the public API for trace support" patch,
> I have updated rte_config.h for meson likewise for other configuration
> such log for DP.
> Would you like to have a meson option? and update in meson_options.txt.
> 
> I don't have a strong opinion on either way? Bruce ,any thoughts?
> 
Since it's likely that developers will be wanting to switch this option on
and off, I think an explicit option rather than hard-coded constant should
be the way to go.

However, an alternative might be to add in the DP tracing for debug builds
but not for others. This would save a build option, but may be more
restrictive, since debug builds can also be optimized ones.

/Bruce

Jerin Jacob April 1, 2020, 4:58 p.m. UTC | #7

On Wed, Apr 1, 2020 at 10:16 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:

> > > >
> > > > Since the trace changes in the "inline" functions of the specific
> > > > library already
> > > > disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and
> > > > it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path
> > > > trace like RTE_TRACE_POINT().
> > >
> > > Ah indeed.
> > > Note: RTE_ENABLE_TRACE_DP is not plugged with meson.
> >
> > If you see the " eal: define the public API for trace support" patch,
> > I have updated rte_config.h for meson likewise for other configuration
> > such log for DP.
> > Would you like to have a meson option? and update in meson_options.txt.
> >
> > I don't have a strong opinion on either way? Bruce ,any thoughts?
> >
> Since it's likely that developers will be wanting to switch this option on
> and off, I think an explicit option rather than hard-coded constant should
> be the way to go.

OK. I will add an explicit option in the next version.

>
> However, an alternative might be to add in the DP tracing for debug builds
> but not for others. This would save a build option, but may be more
> restrictive, since debug builds can also be optimized ones.

I think it will be restrictive.


>
> /Bruce

David Marchand April 1, 2020, 5:32 p.m. UTC | #8

On Wed, Apr 1, 2020 at 6:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.marchand@redhat.com> wrote:
> > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > I assume the patch content will be:
> > > 1) Removing experimental API from app, lib, drivers, examples with
> > > make and meson
> > > 2) Have it enabled at the global level.
> >
> > examples are a special case as they can be compiled out of the dpdk sources.
> > This is why I excluded them of the list in my mail before.
>
> There are a lot of examples already that have ALLOW_EXPERIMENTAL_API.
> What is the issue in enabling ALLOW_EXPERIMENTAL_API  in the in tree
> example application?

I did not say there is an issue in keeping this flag in the example makefiles.

examples makefile should be self sufficient (like an external
application makefile would be) and are called directly in our building
test scripts: see devtools/test-meson-builds.sh.
So I don't see how it would work if you remove this flag from all
examples makefiles and rely on some global thing.

Jerin Jacob April 1, 2020, 5:52 p.m. UTC | #9

On Wed, Apr 1, 2020 at 11:02 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Wed, Apr 1, 2020 at 6:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.marchand@redhat.com> wrote:
> > > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > I assume the patch content will be:
> > > > 1) Removing experimental API from app, lib, drivers, examples with
> > > > make and meson
> > > > 2) Have it enabled at the global level.
> > >
> > > examples are a special case as they can be compiled out of the dpdk sources.
> > > This is why I excluded them of the list in my mail before.
> >
> > There are a lot of examples already that have ALLOW_EXPERIMENTAL_API.
> > What is the issue in enabling ALLOW_EXPERIMENTAL_API  in the in tree
> > example application?
>
> I did not say there is an issue in keeping this flag in the example makefiles.
>
> examples makefile should be self sufficient (like an external
> application makefile would be) and are called directly in our building
> test scripts: see devtools/test-meson-builds.sh.
> So I don't see how it would work if you remove this flag from all
> examples makefiles and rely on some global thing.

Thanks for the clarification. I will update meson.build and Makefile in
examples.

devtools/test-meson-builds.sh tests[1] the the below apps,
"cmdline helloworld l2fwd l3fwd skeleton time"

What is your preference, Adding in all the examples or above selected ones?


[1]
# if pkg-config defines the necessary flags, test building some examples
if pkg-config --define-prefix libdpdk >/dev/null 2>&1; then
        export PKGCONF="pkg-config --define-prefix"
        for example in cmdline helloworld l2fwd l3fwd skeleton timer; do
                echo "## Building $example"
                $MAKE -C
$DESTDIR/usr/local/share/dpdk/examples/$example clean all
        done
fi



>
>
> --
> David Marchand
>

Jerin Jacob April 1, 2020, 7:14 p.m. UTC | #10

On Wed, Apr 1, 2020 at 11:22 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Wed, Apr 1, 2020 at 11:02 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Wed, Apr 1, 2020 at 6:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > I assume the patch content will be:
> > > > > 1) Removing experimental API from app, lib, drivers, examples with
> > > > > make and meson
> > > > > 2) Have it enabled at the global level.
> > > >
> > > > examples are a special case as they can be compiled out of the dpdk sources.
> > > > This is why I excluded them of the list in my mail before.
> > >
> > > There are a lot of examples already that have ALLOW_EXPERIMENTAL_API.
> > > What is the issue in enabling ALLOW_EXPERIMENTAL_API  in the in tree
> > > example application?
> >
> > I did not say there is an issue in keeping this flag in the example makefiles.
> >
> > examples makefile should be self sufficient (like an external
> > application makefile would be) and are called directly in our building
> > test scripts: see devtools/test-meson-builds.sh.
> > So I don't see how it would work if you remove this flag from all
> > examples makefiles and rely on some global thing.
>
> Thanks for the clarification. I will update meson.build and Makefile in
> examples.
>
> devtools/test-meson-builds.sh tests[1] the the below apps,
> "cmdline helloworld l2fwd l3fwd skeleton time"
>
> What is your preference, Adding in all the examples or above selected ones?


Since meson installing example application as well. I will update the
effected examples
with ALLOW_EXPERIMENTAL_API

>
>
> [1]
> # if pkg-config defines the necessary flags, test building some examples
> if pkg-config --define-prefix libdpdk >/dev/null 2>&1; then
>         export PKGCONF="pkg-config --define-prefix"
>         for example in cmdline helloworld l2fwd l3fwd skeleton timer; do
>                 echo "## Building $example"
>                 $MAKE -C
> $DESTDIR/usr/local/share/dpdk/examples/$example clean all
>         done
> fi
>
>
>
> >
> >
> > --
> > David Marchand
> >

Thomas Monjalon April 1, 2020, 7:43 p.m. UTC | #11

01/04/2020 21:14, Jerin Jacob:
> On Wed, Apr 1, 2020 at 11:22 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Wed, Apr 1, 2020 at 11:02 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > On Wed, Apr 1, 2020 at 6:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > I assume the patch content will be:
> > > > > > 1) Removing experimental API from app, lib, drivers, examples with
> > > > > > make and meson
> > > > > > 2) Have it enabled at the global level.
> > > > >
> > > > > examples are a special case as they can be compiled out of the dpdk sources.
> > > > > This is why I excluded them of the list in my mail before.
> > > >
> > > > There are a lot of examples already that have ALLOW_EXPERIMENTAL_API.
> > > > What is the issue in enabling ALLOW_EXPERIMENTAL_API  in the in tree
> > > > example application?
> > >
> > > I did not say there is an issue in keeping this flag in the example makefiles.
> > >
> > > examples makefile should be self sufficient (like an external
> > > application makefile would be) and are called directly in our building
> > > test scripts: see devtools/test-meson-builds.sh.
> > > So I don't see how it would work if you remove this flag from all
> > > examples makefiles and rely on some global thing.
> >
> > Thanks for the clarification. I will update meson.build and Makefile in
> > examples.
> >
> > devtools/test-meson-builds.sh tests[1] the the below apps,
> > "cmdline helloworld l2fwd l3fwd skeleton time"
> >
> > What is your preference, Adding in all the examples or above selected ones?
> 
> 
> Since meson installing example application as well. I will update the
> effected examples
> with ALLOW_EXPERIMENTAL_API

I think you need to update all examples.

Jerin Jacob April 1, 2020, 8 p.m. UTC | #12

On Thu, Apr 2, 2020 at 1:13 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 01/04/2020 21:14, Jerin Jacob:
> > On Wed, Apr 1, 2020 at 11:22 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > On Wed, Apr 1, 2020 at 11:02 PM David Marchand
> > > <david.marchand@redhat.com> wrote:
> > > >
> > > > On Wed, Apr 1, 2020 at 6:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > On Wed, Apr 1, 2020 at 7:42 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > I assume the patch content will be:
> > > > > > > 1) Removing experimental API from app, lib, drivers, examples with
> > > > > > > make and meson
> > > > > > > 2) Have it enabled at the global level.
> > > > > >
> > > > > > examples are a special case as they can be compiled out of the dpdk sources.
> > > > > > This is why I excluded them of the list in my mail before.
> > > > >
> > > > > There are a lot of examples already that have ALLOW_EXPERIMENTAL_API.
> > > > > What is the issue in enabling ALLOW_EXPERIMENTAL_API  in the in tree
> > > > > example application?
> > > >
> > > > I did not say there is an issue in keeping this flag in the example makefiles.
> > > >
> > > > examples makefile should be self sufficient (like an external
> > > > application makefile would be) and are called directly in our building
> > > > test scripts: see devtools/test-meson-builds.sh.
> > > > So I don't see how it would work if you remove this flag from all
> > > > examples makefiles and rely on some global thing.
> > >
> > > Thanks for the clarification. I will update meson.build and Makefile in
> > > examples.
> > >
> > > devtools/test-meson-builds.sh tests[1] the the below apps,
> > > "cmdline helloworld l2fwd l3fwd skeleton time"
> > >
> > > What is your preference, Adding in all the examples or above selected ones?
> >
> >
> > Since meson installing example application as well. I will update the
> > effected examples
> > with ALLOW_EXPERIMENTAL_API
>
> I think you need to update all examples.

OK. Will do next version.

>
>