Re: [dpdk-dev] [PATCH v12 7/7] app/testpmd: add forwarding engine for shared Rx queue

["Li, Xiaoyun" <xiaoyun.li@intel.com>]

> -----Original Message-----
> From: Xueming Li <xuemingl@nvidia.com>
> Sent: Thursday, October 21, 2021 13:09
> To: dev@dpdk.org; Zhang, Yuying <yuying.zhang@intel.com>; Li, Xiaoyun
> <xiaoyun.li@intel.com>
> Cc: xuemingl@nvidia.com; Jerin Jacob <jerinjacobk@gmail.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Viacheslav Ovsiienko
> <viacheslavo@nvidia.com>; Thomas Monjalon <thomas@monjalon.net>; Lior
> Margalit <lmargalit@nvidia.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>
> Subject: [PATCH v12 7/7] app/testpmd: add forwarding engine for shared Rx
> queue
> 
> To support shared Rx queue, this patch introduces dedicate forwarding engine.
> The engine groups received packets by mbuf->port into sub-group, updates
> stream statistics and simply frees packets.
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

I didn't ack you on this patch. I remember I added "+1" to the comment about your includes issue.
It will confuse reviewers not to review new versions.

> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

I didn't see he ack this patch as well.
Please remove these acks.

> ---
>  app/test-pmd/meson.build                    |   1 +
>  app/test-pmd/shared_rxq_fwd.c               | 113 ++++++++++++++++++++
>  app/test-pmd/testpmd.c                      |   1 +
>  app/test-pmd/testpmd.h                      |   5 +
>  doc/guides/testpmd_app_ug/run_app.rst       |   5 +-
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   5 +-
>  6 files changed, 128 insertions(+), 2 deletions(-)  create mode 100644 app/test-
> pmd/shared_rxq_fwd.c
> 
> diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build index
> 1ad54caef2c..b5a0f7b6209 100644
> --- a/app/test-pmd/meson.build
> +++ b/app/test-pmd/meson.build
> @@ -22,6 +22,7 @@ sources = files(
>          'noisy_vnf.c',
>          'parameters.c',
>          'rxonly.c',
> +        'shared_rxq_fwd.c',
>          'testpmd.c',
>          'txonly.c',
>          'util.c',
> diff --git a/app/test-pmd/shared_rxq_fwd.c b/app/test-pmd/shared_rxq_fwd.c
> new file mode 100644 index 00000000000..c4684893674
> --- /dev/null
> +++ b/app/test-pmd/shared_rxq_fwd.c
> @@ -0,0 +1,113 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2021 NVIDIA Corporation & Affiliates  */
> +

Please add "#include <rte_ethdev.h>" here.
Your shared_rxq_fwd.c only needs this include.

> +#include "testpmd.h"
> +
> +/*
> + * Rx only sub-burst forwarding.
> + */
> +static void
> +forward_rx_only(uint16_t nb_rx, struct rte_mbuf **pkts_burst) {
> +	rte_pktmbuf_free_bulk(pkts_burst, nb_rx); }
> +
> +/**
> + * Get packet source stream by source port and queue.
> + * All streams of same shared Rx queue locates on same core.
> + */
> +static struct fwd_stream *
> +forward_stream_get(struct fwd_stream *fs, uint16_t port) {
<snip>
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 9482dab3071..ef7a6199313 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -12,6 +12,10 @@
>  #include <rte_gro.h>
>  #include <rte_gso.h>
>  #include <rte_os_shim.h>
> +#include <rte_mbuf_dyn.h>
> +#include <rte_flow.h>
> +#include <rte_ethdev.h>
> +

Please remove these includes and this blank line.
You only need to add the lib you need in your file like I said above.

>  #include <cmdline.h>
>  #include <sys/queue.h>
>  #ifdef RTE_HAS_JANSSON
> @@ -339,6 +343,7 @@ extern struct fwd_engine five_tuple_swap_fwd_engine;
> #ifdef RTE_LIBRTE_IEEE1588  extern struct fwd_engine ieee1588_fwd_engine;
> #endif
> +extern struct fwd_engine shared_rxq_engine;
> 
>  extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
> extern cmdline_parse_inst_t cmd_set_raw; diff --git
> a/doc/guides/testpmd_app_ug/run_app.rst
> b/doc/guides/testpmd_app_ug/run_app.rst
> index faa3efb902c..74412bb82ca 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -258,6 +258,7 @@ The command line options are:
>         tm
>         noisy
>         5tswap
> +       shared-rxq
> 
>  *   ``--rss-ip``
> 
> @@ -399,7 +400,9 @@ The command line options are:
> 
>      Create queues in shared Rx queue mode if device supports.
>      Shared Rx queues are grouped per X ports. X defaults to UINT32_MAX,
> -    implies all ports join share group 1.
> +    implies all ports join share group 1. A new forwarding engine
> +    "shared-rxq" should be used for shared Rx queues. This engine does
> +    Rx only and update stream statistics accordingly.
> 
>  *   ``--eth-link-speed``
> 
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 6d127d9a7bc..78d23429c42 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -314,7 +314,7 @@ set fwd
>  Set the packet forwarding mode::
> 
>     testpmd> set fwd (io|mac|macswap|flowgen| \
> -                     rxonly|txonly|csum|icmpecho|noisy|5tswap) (""|retry)
> +
> + rxonly|txonly|csum|icmpecho|noisy|5tswap|shared-rxq) (""|retry)
> 
>  ``retry`` can be specified for forwarding engines except ``rx_only``.
> 
> @@ -357,6 +357,9 @@ The available information categories are:
> 
>    L4 swaps the source port and destination port of transport layer (TCP and UDP).
> 
> +* ``shared-rxq``: Receive only for shared Rx queue.
> +  Resolve packet source port from mbuf and update stream statistics
> accordingly.
> +
>  Example::
> 
>     testpmd> set fwd rxonly
> --
> 2.33.0