Message ID | 20200329144342.1543749-1-jerinj@marvell.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9C42CA0562; Sun, 29 Mar 2020 16:43:30 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3E8941AFF; Sun, 29 Mar 2020 16:43:29 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by dpdk.org (Postfix) with ESMTP id C5E78FFA for <dev@dpdk.org>; Sun, 29 Mar 2020 16:43:27 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 02TEeRWn013022; Sun, 29 Mar 2020 07:43:27 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-type : content-transfer-encoding; s=pfpt0818; bh=owBRLSL+7+Ncj+N8nTY5A1hBUpXCq9jyGc2s1vZM7ZE=; b=H5sRBX6oVyhw6wSMSqR8ZYcwfgBj0t+JLRFBi2ZayWmsYdqdKndJSs+Dnb/n/ctTXMyg nwBzC1fZzh68s026DzIjOQh18X4Vb+QZqJ809XxPuOW/EW8r/w45xtDpmw9rK04yozzS KubUv2UaElci1u+WrAox/+sMtATBDtqkVFt+AVkhFt8uDoc1fML7fdQ+Cbw+OK9x5AjF 5i7JGNRUAaZbIbJ8htbY1NSrYddTQRnlUo8ORRy2KoPu47+cX+4KinQg7hx0lM7XueqM 7r9FIdzfz/x4IijBKQy7mynQQ54oijee4k99O9dM0dteObAYgCNl2Q+96VHi03pViZSC Iw== Received: from sc-exch03.marvell.com ([199.233.58.183]) by mx0b-0016f401.pphosted.com with ESMTP id 30263kb5g1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Sun, 29 Mar 2020 07:43:26 -0700 Received: from SC-EXCH03.marvell.com (10.93.176.83) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sun, 29 Mar 2020 07:43:24 -0700 Received: from maili.marvell.com (10.93.176.43) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Sun, 29 Mar 2020 07:43:24 -0700 Received: from jerin-lab.marvell.com (jerin-lab.marvell.com [10.28.34.14]) by maili.marvell.com (Postfix) with ESMTP id 3D1063F703F; Sun, 29 Mar 2020 07:43:21 -0700 (PDT) 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> X-Mailer: git-send-email 2.25.1 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 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138, 18.0.645 definitions=2020-03-29_05:2020-03-27, 2020-03-29 signatures=0 Subject: [dpdk-dev] [PATCH v3 00/33] DPDK Trace support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> 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
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.
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 >
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.
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.
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.
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
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
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.
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 >
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 > >
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.
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. > >