[v12,7/7] app/testpmd: add forwarding engine for shared Rx queue
Checks
Commit Message
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>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
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
Comments
> -----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
On Thu, 2021-10-21 at 06:33 +0000, Li, Xiaoyun wrote:
> > -----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.
Yes, they there by mistake.
>
> > 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.
As explained below, testpmd relies on rte_ethdev.h.
>
> > +#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.
From test, testpmd.h used these headers, otherwise compile error if not
included by fwd engine.
>
> > #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
>
> -----Original Message-----
> From: Xueming(Steven) Li <xuemingl@nvidia.com>
> Sent: Thursday, October 21, 2021 15:59
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; dev@dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> jerinjacobk@gmail.com; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> ajit.khaparde@broadcom.com; Yigit, Ferruh <ferruh.yigit@intel.com>;
> andrew.rybchenko@oktetlabs.ru; Lior Margalit <lmargalit@nvidia.com>
> Subject: Re: [PATCH v12 7/7] app/testpmd: add forwarding engine for shared Rx
> queue
>
> On Thu, 2021-10-21 at 06:33 +0000, Li, Xiaoyun wrote:
> > > -----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.
>
> Yes, they there by mistake.
>
> >
> > > 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.
>
> As explained below, testpmd relies on rte_ethdev.h.
>
> >
> > > +#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.
>
> From test, testpmd.h used these headers, otherwise compile error if not
> included by fwd engine.
Have you tried my way? Include "#include <rte_ethdev.h>" in shared_rxq_fwd.c.
Please try this and see if there's any compiling issues.
>
> >
> > > #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
> >
On Thu, 2021-10-21 at 08:01 +0000, Li, Xiaoyun wrote:
>
> > -----Original Message-----
> > From: Xueming(Steven) Li <xuemingl@nvidia.com>
> > Sent: Thursday, October 21, 2021 15:59
> > To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhang, Yuying
> > <yuying.zhang@intel.com>; dev@dpdk.org
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> > jerinjacobk@gmail.com; NBU-Contact-Thomas Monjalon
> > <thomas@monjalon.net>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> > ajit.khaparde@broadcom.com; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > andrew.rybchenko@oktetlabs.ru; Lior Margalit <lmargalit@nvidia.com>
> > Subject: Re: [PATCH v12 7/7] app/testpmd: add forwarding engine for shared Rx
> > queue
> >
> > On Thu, 2021-10-21 at 06:33 +0000, Li, Xiaoyun wrote:
> > > > -----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.
> >
> > Yes, they there by mistake.
> >
> > >
> > > > 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.
> >
> > As explained below, testpmd relies on rte_ethdev.h.
> >
> > >
> > > > +#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.
> >
> > From test, testpmd.h used these headers, otherwise compile error if not
> > included by fwd engine.
>
> Have you tried my way? Include "#include <rte_ethdev.h>" in shared_rxq_fwd.c.
> Please try this and see if there's any compiling issues.
It works, seems rte_ethdev.h has everything needed, thanks!
>
> >
> > >
> > > > #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
> > >
>
21/10/2021 07:08, Xueming Li:
> + implies all ports join share group 1. A new forwarding engine
Don't say "new" as it will not be new forever.
You can say a "specific" or "specialized".
> + "shared-rxq" should be used for shared Rx queues. This engine does
> + Rx only and update stream statistics accordingly.
@@ -22,6 +22,7 @@ sources = files(
'noisy_vnf.c',
'parameters.c',
'rxonly.c',
+ 'shared_rxq_fwd.c',
'testpmd.c',
'txonly.c',
'util.c',
new file mode 100644
@@ -0,0 +1,113 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2021 NVIDIA Corporation & Affiliates
+ */
+
+#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)
+{
+ streamid_t sm_id;
+ struct fwd_lcore *fc;
+ struct fwd_stream **fsm;
+ streamid_t nb_fs;
+
+ fc = fs->lcore;
+ fsm = &fwd_streams[fc->stream_idx];
+ nb_fs = fc->stream_nb;
+ for (sm_id = 0; sm_id < nb_fs; sm_id++) {
+ if (fsm[sm_id]->rx_port == port &&
+ fsm[sm_id]->rx_queue == fs->rx_queue)
+ return fsm[sm_id];
+ }
+ return NULL;
+}
+
+/**
+ * Forward packet by source port and queue.
+ */
+static void
+forward_sub_burst(struct fwd_stream *src_fs, uint16_t port, uint16_t nb_rx,
+ struct rte_mbuf **pkts)
+{
+ struct fwd_stream *fs = forward_stream_get(src_fs, port);
+
+ if (fs != NULL) {
+ fs->rx_packets += nb_rx;
+ forward_rx_only(nb_rx, pkts);
+ } else {
+ /* Source stream not found, drop all packets. */
+ src_fs->fwd_dropped += nb_rx;
+ while (nb_rx > 0)
+ rte_pktmbuf_free(pkts[--nb_rx]);
+ }
+}
+
+/**
+ * Forward packets from shared Rx queue.
+ *
+ * Source port of packets are identified by mbuf->port.
+ */
+static void
+forward_shared_rxq(struct fwd_stream *fs, uint16_t nb_rx,
+ struct rte_mbuf **pkts_burst)
+{
+ uint16_t i, nb_sub_burst, port, last_port;
+
+ nb_sub_burst = 0;
+ last_port = pkts_burst[0]->port;
+ /* Locate sub-burst according to mbuf->port. */
+ for (i = 0; i < nb_rx - 1; ++i) {
+ rte_prefetch0(pkts_burst[i + 1]);
+ port = pkts_burst[i]->port;
+ if (i > 0 && last_port != port) {
+ /* Forward packets with same source port. */
+ forward_sub_burst(fs, last_port, nb_sub_burst,
+ &pkts_burst[i - nb_sub_burst]);
+ nb_sub_burst = 0;
+ last_port = port;
+ }
+ nb_sub_burst++;
+ }
+ /* Last sub-burst. */
+ nb_sub_burst++;
+ forward_sub_burst(fs, last_port, nb_sub_burst,
+ &pkts_burst[nb_rx - nb_sub_burst]);
+}
+
+static void
+shared_rxq_fwd(struct fwd_stream *fs)
+{
+ struct rte_mbuf *pkts_burst[nb_pkt_per_burst];
+ uint16_t nb_rx;
+ uint64_t start_tsc = 0;
+
+ get_start_cycles(&start_tsc);
+ nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
+ nb_pkt_per_burst);
+ inc_rx_burst_stats(fs, nb_rx);
+ if (unlikely(nb_rx == 0))
+ return;
+ forward_shared_rxq(fs, nb_rx, pkts_burst);
+ get_end_cycles(fs, start_tsc);
+}
+
+struct fwd_engine shared_rxq_engine = {
+ .fwd_mode_name = "shared_rxq",
+ .port_fwd_begin = NULL,
+ .port_fwd_end = NULL,
+ .packet_fwd = shared_rxq_fwd,
+};
@@ -188,6 +188,7 @@ struct fwd_engine * fwd_engines[] = {
#ifdef RTE_LIBRTE_IEEE1588
&ieee1588_fwd_engine,
#endif
+ &shared_rxq_engine,
NULL,
};
@@ -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>
+
#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;
@@ -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``
@@ -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