On Thu, 2021-10-21 at 08:01 +0000, Li, Xiaoyun wrote:
>
> > -----Original Message-----
> > From: Xueming(Steven) Li <xuemi...@nvidia.com>
> > Sent: Thursday, October 21, 2021 15:59
> > To: Li, Xiaoyun <xiaoyun...@intel.com>; Zhang, Yuying
> > <yuying.zh...@intel.com>; dev@dpdk.org
> > Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>;
> > jerinjac...@gmail.com; NBU-Contact-Thomas Monjalon
> > <tho...@monjalon.net>; Slava Ovsiienko <viachesl...@nvidia.com>;
> > ajit.khapa...@broadcom.com; Yigit, Ferruh <ferruh.yi...@intel.com>;
> > andrew.rybche...@oktetlabs.ru; Lior Margalit <lmarga...@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 <xuemi...@nvidia.com>
> > > > Sent: Thursday, October 21, 2021 13:09
> > > > To: dev@dpdk.org; Zhang, Yuying <yuying.zh...@intel.com>; Li,
> > > > Xiaoyun <xiaoyun...@intel.com>
> > > > Cc: xuemi...@nvidia.com; Jerin Jacob <jerinjac...@gmail.com>; Yigit,
> > > > Ferruh <ferruh.yi...@intel.com>; Andrew Rybchenko
> > > > <andrew.rybche...@oktetlabs.ru>; Viacheslav Ovsiienko
> > > > <viachesl...@nvidia.com>; Thomas Monjalon <tho...@monjalon.net>;
> > > > Lior Margalit <lmarga...@nvidia.com>; Ananyev, Konstantin
> > > > <konstantin.anan...@intel.com>; Ajit Khaparde
> > > > <ajit.khapa...@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 <xuemi...@nvidia.com>
> > > > Acked-by: Xiaoyun Li <xiaoyun...@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.khapa...@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
> > >
>
