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, jer...@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.
