Re: [PATCH v3 2/3] graph: pcap capture for graph nodes

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Jan 12, 2023 at 3:31 PM Amit Prakash Shukla
<amitprakashs@marvell.com> wrote:
>
> Implementation adds support to capture packets at each node with
> packet metadata and node name.
>
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
> v2:
>  - Fixed code style issue
>  - Fixed CI compilation issue on github-robot
>
>
> diff --git a/lib/graph/graph_pcap_trace.c b/lib/graph/graph_pcap_trace.c
> new file mode 100644
> index 0000000000..f7b81a7ad1
> --- /dev/null
> +++ b/lib/graph/graph_pcap_trace.c
> @@ -0,0 +1,155 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2022 Marvell International Ltd.

2023

> +
> +#define GRAPH_PCAP_BUF_SZ      128
> +#define GRAPH_PCAP_NUM_PACKETS 1024
> +#define GRAPH_PCAP_FILE_NAME_SZ        128
> +#define GRAPH_PCAP_FILE_NAME   "/tmp/graph_pcap_capture.pcapng"

Expecting the filename from application. So no need to define here.

> +
> +static char file_name[GRAPH_PCAP_FILE_NAME_SZ];
> +static uint32_t pkt_buf_sz = RTE_MBUF_DEFAULT_BUF_SIZE;
> +static uint64_t packet_to_capture = GRAPH_PCAP_NUM_PACKETS;
> +static rte_pcapng_t *pcapng_fd;
> +static struct rte_mempool *mp;
> +static uint16_t port_id;
> +static uint64_t packet_captured[RTE_MAX_LCORE];
> +static int pcap_trace_enable;

See what it takes for multiprocess. If anything needs to be available
in graph walk(which runs on secondary process) needs to be part of
huge page. i.e part of rte_graph structure.

>
> +       rte_node_process_t original_process; /**< Pcap enabled node callback */

Add comment like, "Used to store the original process when pcap is
enabled"  or so

> +
>         /* Fast path area  */
>  #define RTE_NODE_CTX_SZ 16
>         uint8_t ctx[RTE_NODE_CTX_SZ] __rte_cache_aligned; /**< Node Context. */
> diff --git a/lib/graph/version.map b/lib/graph/version.map
> index 13b838752d..36153a75b2 100644
> --- a/lib/graph/version.map
> +++ b/lib/graph/version.map
> @@ -43,5 +43,12 @@ EXPERIMENTAL {
>         rte_node_next_stream_put;
>         rte_node_next_stream_move;
>
> +       rte_pcap_trace_is_enable;
> +       rte_pcap_trace_enable;
> +       rte_graph_pcap_trace_init;
> +       rte_num_pkt_to_capture;
> +       rte_filename_to_capture_pkt;
> +       rte_graph_pcap_trace_exit;

All of these public APIs are not needed. Since rte_graph is still
experimental to make it clean, please add file name and number of
packet to capture
in  struct rte_graph_param and update doc/guides/rel_notes/release_23_03.rst.

See following example in doc/guides/rel_notes/release_22_07.rst

API Changes
-----------

* Experimental structures ``struct rte_mtr_params``
  and ``struct rte_mtr_capabilities`` were updated to support
  protocol based input color for meter.

> +
>         local: *;
>  };
> --
> 2.25.1
>