Re: [dpdk-dev] [PATCH] doc: update pdump documentation

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jul 13, 2020 at 03:25:25PM +0100, Reshma Pattan wrote:
> Update the pdump library programmers guide and Howto doc
> with the use of multi process channel replacing socket
> based communication.
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---

Some minor adjustments I'd suggest below. Otherwise:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

>  doc/guides/howto/packet_capture_framework.rst | 16 ++++-----
>  doc/guides/prog_guide/pdump_lib.rst           | 49 +++++++++++----------------
>  2 files changed, 28 insertions(+), 37 deletions(-)
> 
> diff --git a/doc/guides/howto/packet_capture_framework.rst b/doc/guides/howto/packet_capture_framework.rst
> index 946a21c..c941c01 100644
> --- a/doc/guides/howto/packet_capture_framework.rst
> +++ b/doc/guides/howto/packet_capture_framework.rst
> @@ -19,7 +19,7 @@ Introduction
>  
<snip>  
> -In DPDK the ``testpmd`` application can be used to initialize the packet
> -capture framework and act as a server, and the ``dpdk-pdump`` tool acts as a
> -client. To view Rx or Tx packets of ``testpmd``, the application should be
> +In DPDK the ``testpmd`` application has been initialize with the packet
> +capture framework and acts as primary process, and the ``dpdk-pdump`` tool acts as a
> +secondary process. To view Rx or Tx packets of ``testpmd``, the application should be

I think the original text was better for this block.

>  launched first, and then the ``dpdk-pdump`` tool. Packets from ``testpmd``
>  will be sent to the tool, which then sends them on to the Pcap PMD device and
>  that device writes them to the Pcap file or to an external interface depending
> diff --git a/doc/guides/prog_guide/pdump_lib.rst b/doc/guides/prog_guide/pdump_lib.rst
> index 2a0f1f3..227eb1a 100644
> --- a/doc/guides/prog_guide/pdump_lib.rst
> +++ b/doc/guides/prog_guide/pdump_lib.rst
> @@ -11,8 +11,12 @@ The library does the complete copy of the Rx and Tx mbufs to a new mempool and
>  hence it slows down the performance of the applications, so it is recommended
>  to use this library for debugging purposes.
>  
> +The library uses generic multi process channel to facilitate communication
                 ^^a

Missing word "a" in sentence.

> +between primary and secondary process for enabling/disabling packet capture on
> +ports.
> +
<snip>

>  The library APIs ``rte_pdump_disable()`` and ``rte_pdump_disable_by_deviceid()`` disables the packet capture.
> -On each call to these APIs, the library creates a separate client socket, creates the "pdump disable" request and sends
> -the request to the server. The server that is listening on the socket will take the request and disable the packet
> -capture by removing the Ethernet RX and TX callbacks for the given port or device_id and queue combinations. The server
> -also sends the response back to the client about the status of the request that was processed. After the response is
> -received from the server, the client socket is closed.
> +For the calls to these APIs from secondary process, the library creates the "pdump disable" request and sends
> +the request to the primary process over the multi process channel, the primary process takes this request and disable
                                                                  ^^.                                            ^^disables

Split sentence as indicated and also add "s" to disable.

> +the packet capture by removing the Ethernet RX and TX callbacks for the given port or device_id and queue combinations.
>  
>  The library API ``rte_pdump_uninit()``, uninitializes the packet capture framework by calling ``rte_mp_action_unregister()``
>  function.
> -- 
> 1.8.3.1
>