From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 4CF12A0561;
Tue, 21 Apr 2020 02:19:12 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 686C21D64E;
Tue, 21 Apr 2020 02:19:11 +0200 (CEST)
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
[66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 275CE1D64D
for <dev@dpdk.org>; Tue, 21 Apr 2020 02:19:09 +0200 (CEST)
Received: from compute7.internal (compute7.nyi.internal [10.202.2.47])
by mailout.nyi.internal (Postfix) with ESMTP id B65875C01BD;
Mon, 20 Apr 2020 20:19:08 -0400 (EDT)
Received: from mailfrontend1 ([10.202.2.162])
by compute7.internal (MEProxy); Mon, 20 Apr 2020 20:19:08 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
from:to:cc:subject:date:message-id:in-reply-to:references
:mime-version:content-transfer-encoding:content-type; s=fm1; bh=
PtlcpQXbUdbwyn4LcGGG0CjqFMwXNNf2kXn/qtCMOhY=; b=uI6wTTeLjh+K6xhR
2hfxhzXgDfDtVajRNC5eGlRmAOYbmIc5HdD9L+QY+qYm8hScm0c88FnZizX48aTi
4mzNB+5QoG5NM49LR5ETymH8TdUPk2PkQELHWdmYW/775Lsi3vKNxbzMBInQMhbo
ZnFN5g+C56MStKSIDOX1v8iPVj9TfJvUVfMoDcB5noRFAny+QnJvVaiBv+98b6Hl
0F5wpttDmiteLjJiZZixp5Xu5oLO5SxC52hP1KucNtCPR0m/MJcEOiXMv/g17H+M
raownKNKolNCacD8J9Ec+LQOyMMHhAFhH+v6ys2JwoeMoctpRMtUqSwbrbEMzTaA
/BOxxg==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
messagingengine.com; h=cc:content-transfer-encoding:content-type
:date:from:in-reply-to:message-id:mime-version:references
:subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender
:x-sasl-enc; s=fm2; bh=PtlcpQXbUdbwyn4LcGGG0CjqFMwXNNf2kXn/qtCMO
hY=; b=VLDTyPJGYeybU9Rxb2MHhvBqOjhDYToe90WJKm1zQZ0/sVOM2v5d0DhFV
LAJpcvQiJsJNF2e9rXjU1dVXEDSV+H3glh2nOpQFi63cEjUed34aOyFw4Rw2IiLq
yn2l6a0k0MjcvTBjivCa3L2lfruSwCX7OJmo6zQj9O+UYKL3jObKFBsQMCQne5jR
LBVT63M9S1o1jWOUEFEE26LVPi1qXKjq3bUA4T8LG0zyI6xn/3f1K0dQVTWji5La
PHiXuAB47rDPBiKpUlt0PQO4uBE/Ksuk3AMBdWVKBQXskyKgLRz8iKTdy5v/cfd+
zKbd5aqfg+OBY0xj2/U4ScEUQr4bA==
X-ME-Sender: <xms:_DueXofuB_ZYi1yTwRyhaKNVaXGobNUQXcfXY15ORmZKWgFNpyxnVw>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrgeeggddvlecutefuodetggdotefrodftvf
curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs
ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucffoh
hmrghinhepughirghmohhnrdhorhhgpdhsohhurhgtvghfohhrghgvrdhiohenucfkphep
jeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrg
hmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth
X-ME-Proxy: <xmx:_DueXvGw81TIzQ4YRPVA8GcE8ATXrEt0N1XVE0o2OS4UexNkzWhURA>
<xmx:_DueXiElM2y4fYPpt9CcZjaf_fHyRlBZgzCWPAt76iVKeYlHgJcDmg>
<xmx:_DueXvPDkbnF21OOyfqFPlRm_IUApTuCSRrvrHctuvVEqSrB6n9l1Q>
<xmx:_DueXjVhtfMyPGP7cp6hq6R5psPnSy0uKsIeXYHPMyBnlv0tXetC5g>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
by mail.messagingengine.com (Postfix) with ESMTPA id 4F485328006A;
Mon, 20 Apr 2020 20:19:07 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Jerin Jacob <jerinj@marvell.com>, Sunil Kumar Kori <skori@marvell.com>
Cc: John McNamara <john.mcnamara@intel.com>,
Marko Kovacevic <marko.kovacevic@intel.com>, dev@dpdk.org,
bruce.richardson@intel.com, david.marchand@redhat.com,
mattias.ronnblom@ericsson.com
Date: Tue, 21 Apr 2020 02:19:01 +0200
Message-ID: <3354690.gORTcIGjah@thomas>
In-Reply-To: <20200419100133.3232316-34-jerinj@marvell.com>
References: <20200413150116.734047-1-jerinj@marvell.com>
<20200419100133.3232316-1-jerinj@marvell.com>
<20200419100133.3232316-34-jerinj@marvell.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v6 33/33] doc: add trace library guide
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>
Hi,
Below is a doc review.
General comment: it is better to split lines after punctuation signs
in order to make future patches easier to read.
19/04/2020 12:01, jerinj@marvell.com:
> +DPDK tracing library features
> +-----------------------------
> +
> +- A framework to add tracepoints in control and fast APIs with minimum impact on
fast, you mean fast path?
> + performance. Typical trace overhead is ~20 cycles and instrumentation
> + overhead is 1 cycle.
> +- Enable and disable the tracepoints at runtime.
> +- Save the trace buffer to the filesystem at any point in time.
> +- Supports ``overwrite`` and ``discard`` trace mode operations.
> +- String-based tracepoint object lookup.
> +- Enable and disable a set of tracepoints based on regular expression and/or
> + globbing.
> +- Generate trace in ``common trace format(CTF)``. ``CTF`` is an open-source trace
common trace format(CTF) -> Common Trace Format (CTF)
> + format and it is compatible with ``LTTng``.
it is -> is
> + For detailed information, refer `Common Trace Format <https://diamon.org/ctf/>`_
refer -> refer to
> +
> +How to add a tracepoint?
> +------------------------
> +
> +This section steps you through the details of adding a simple tracepoint.
> +
> +.. _create_provider_header_file:
> +
> +Create the tracepoint provider header file
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +.. code-block:: c
> +
> + #include <rte_trace_point.h>
> +
> + RTE_TRACE_POINT(app_trace_string,
> + RTE_TRACE_POINT_ARGS(const char *str),
> + rte_trace_point_emit_string(str);
> + )
> +
> +The above macro creates ``app_trace_string`` tracepoint. The user can choose
> +any name for the tracepoint. However, when adding the tracepoint in the DPDK
> +library, the ``rte_trace_lib_<library_name>_[<domain>]_<name>`` naming convention
> +must be followed. The examples are ``rte_trace_lib_eal_generic_str``,
> +``rte_trace_lib_mempool_create``.
If adding a symbol in a lib, the namespace should the lib's namespace.
So the format must be rte_<library_name>_trace_[<domain>_]<name>.
Examples will become rte_eal_trace_generic_str and rte_mempool_trace_create.
> +
> +When ``RTE_TRACE_POINT`` expands for above the definition, it creates the
expands for above the definition -> expands from above definition
> +following function template. The consumer of this tracepoint can invoke
> +``app_trace_string(const char *str)`` to emit the trace event to the trace buffer.
This last sentence should be below the following code block.
> +
> +.. code-block:: c
> +
> + static __rte_always_inline void
> + app_trace_string(const char *str)
> + {
> + /* Trace subsystem hooks */
> + ...
> + rte_trace_point_emit_string(str);
> + }
> +
> +Register the tracepoint
> +~~~~~~~~~~~~~~~~~~~~~~~
> +
> +.. code-block:: c
> +
> + #define RTE_TRACE_POINT_REGISTER_SELECT /* Select trace point register macros */
I don't understand this #define.
> +
> + #include <tracepoint_provider_header_file.h>
Is it the header file created in previous section?
A reference is missing. Maybe call it my_tracepoint_provider.h
> +
> + RTE_TRACE_POINT_DEFINE(app_trace_string);
> +
> + RTE_INIT(eal_trace_init)
Why "eal" in the name of this constructor?
> + {
> + RTE_TRACE_POINT_REGISTER(app_trace_string, app.trace.string);
> + }
> +
> +The above code snippet registers the ``app_trace_string`` tracepoint to
> +trace library. Here, the ``tracepoint_provider_header_file.h`` is the header file
> +that user created in the first step :ref:`create_provider_header_file`
> +
> +The second argument for the ``RTE_TRACE_POINT_REGISTER`` is the name for the
> +tracepoint. This string will be used for tracepoint lookup or regular expression
> +and/or glob based tracepoint operations. There is no requirement for
> +the trace function and its name to be similar. However, it is recommended to
> +have a similar name for a better naming convention.
> +
> +The user must register the tracepoint before the ``rte_eal_init`` invocation.
> +The user can use the ``RTE_INIT`` construction scheme to achieve the same.
> +
> +.. note::
> +
> + The ``RTE_TRACE_POINT_DEFINE`` defines the tracepoint of ``rte_trace_point_t``
> + type. When the tracepoint defined in the shared library, the user must
tracepoint defined in the shared library -> tracepoint is defined in a shared library
> + update the ``.map`` file with ``__<trace_function_name>`` symbol.
All libraries can be shared, so it should be reworded to make it mandatory.
> + For example, ``__app_trace_string`` will be the exported symbol in the
> + above example.
> +
> +Datapath tracepoint
> +-------------------
> +
> +In order to avoid performance impact for the datapath tracepoint, the library
> +introduced ``RTE_TRACE_POINT_FP``. When adding the tracepoint in datapath code,
> +The user must use ``RTE_TRACE_POINT_FP`` instead of ``RTE_TRACE_POINT``.
> +
> +``RTE_TRACE_POINT_FP`` is compiled out by default and it can be enabled using
> +``CONFIG_RTE_ENABLE_TRACE_FP`` configuration parameter. The ``enable_trace_fp``
> +build option shall be used for the same for meson build .The application may use
Typo with the spaces around the dot.
> +``rte_trace_fp_is_enabled()`` to get status of ``CONFIG_RTE_ENABLE_TRACE_FP``.
> +
> +Event record mode
> +-----------------
> +
> +Event record mode is an attribute of trace buffers. Trace library exposes the
> +following modes:
> +
> +.. _table_event_record_modes:
> +
> +.. table:: Event record modes.
> +
> + +-----------+-----------------------------------------------------------------+
> + | Mode | Description |
> + | | |
> + +===========+=================================================================+
> + | Overwrite | When trace buffer is full, new trace events overwrites the |
> + | | existing captured events in the trace buffer. |
> + +-----------+-----------------------------------------------------------------+
> + | Discard | When trace buffer is full, new trace events will be discarded. |
> + +-----------+-----------------------------------------------------------------+
Please don't use a table for definition.
It will be better rendered as a definition list:
https://docutils.sourceforge.io/docs/user/rst/quickref.html#definition-lists
> +The mode can be configured either using EAL command line parameters on
Which EAL option?
> +application boot up or use ``rte_trace_mode_set()`` API to configure at runtime.
> +Refer :doc:`../linux_gsg/linux_eal_parameters` for EAL command line options.
> +
> +Trace file location
> +-------------------
> +
> +On ``rte_trace_save()`` or ``rte_eal_cleanup()`` invocation, the library saves
> +the trace buffers to the filesystem. By default, library saves trace files at
> +``$HOME/dpdk-traces/rte-yyyy-mm-dd-[AP]M-hh-mm-ss/``. It can be overridden by
Please don't create a specific directory, but use rte_eal_get_runtime_dir().
> +the ``--trace-dir=<directory path>`` EAL command line option.
I don't think this option is needed.
XDG_RUNTIME_DIR environment variable can do the same.
> +
> +For more information, refer :doc:`../linux_gsg/linux_eal_parameters` for trace
> +EAL command line options.
> +
> +
> +View and analyze the recorded events
> +------------------------------------
> +
> +Once the trace directory is available, the user can view/inspect the recorded events.
> +
> +There are many tools you can use to read DPDK traces:
> +
> +1. ``babeltrace`` is a command-line utility that converts trace formats; it
> +supports the format that DPDK trace library produces, CTF, as well as a
> +basic text output that can be grep ed. The babeltrace command is part of the
grep ed -> grep'ed
> +opensource ``Babeltrace`` project.
opensource -> Open Source
Why quotes around Babeltrace?
> +
> +2. ``Trace Compass`` is a graphical user interface for viewing and analyzing any
> +type of logs or traces, including DPDK traces.
The rest looks good, thanks.