From: Gregory Etelson <getelson@nvidia.com>
To: Ferruh Yigit <ferruh.yigit@amd.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: Maayan Kashani <mkashani@nvidia.com>,
"NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
Aman Singh <aman.deep.singh@intel.com>,
Yuying Zhang <yuying.zhang@intel.com>,
David Marchand <david.marchand@redhat.com>
Subject: Re: [PATCH v3] testpmd: add hairpin-map parameter
Date: Tue, 5 Mar 2024 03:04:44 +0000 [thread overview]
Message-ID: <IA1PR12MB63321EBEDE6160C2F6B4E0AFA5222@IA1PR12MB6332.namprd12.prod.outlook.com> (raw)
In-Reply-To: <f726aa50-97b0-4e7b-82fc-d51ee2090d47@amd.com>
[-- Attachment #1: Type: text/plain, Size: 5733 bytes --]
Hello Ferruh,
As a brainstorm, what do you think move hairpin related code to its own
file, and have a kind of register logic to the commands, argument
parsing and usage() functions etc.
Please consider an option for the testpmd to dynamically add supported commands.
Testpmd can still provide some build-in core functionality, but custom test commands
can be added and removed on demand.
In addition, the custom command code does not have to be part of the testpmd.
That will reduce maintanance.
Regards,
Gregory
________________________________
From: Ferruh Yigit <ferruh.yigit@amd.com>
Sent: Friday, March 1, 2024 20:54
To: Gregory Etelson <getelson@nvidia.com>; dev@dpdk.org <dev@dpdk.org>
Cc: Maayan Kashani <mkashani@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; David Marchand <david.marchand@redhat.com>
Subject: Re: [PATCH v3] testpmd: add hairpin-map parameter
External email: Use caution opening links or attachments
On 9/28/2023 4:36 PM, Gregory Etelson wrote:
> Hairpin offloads packet forwarding between ports.
> Packet is expected on Rx port <rp>, Rx queue <rq> and is delivered
> to Tx port <tp> Tx queue <tq>.
>
> Testpmd implements a static hairpin configuration scheme.
> The scheme implicitly matches next valid port for given <rp> or <tp>.
> That approach can be used in a single or double port setups only.
>
> The new parameter allows explicit selection of Rx and Tx ports and
> queues in hairpin configuration.
> The new `hairpin-map` parameter is provided with 5 parameters,
> separated by `:`
>
> `--hairpin-map=Rx port id:Rx queue:Tx port id:Tx queue:queues number`
>
> Testpmd operator can provide several `hairpin-map` parameters for
> different hairpin maps.
> Example:
>
> dpdk-testpmd <EAL params> -- \
> <testpmd params> \
> --rxq=2 --txq=2 --hairpinq=2 --hairpin-mode=0x12 \
> --hairpin-map=0:2:1:2:1 \ # [1]
> --hairpin-map=0:3:2:2:3 # [2]
>
> Hairpin map [1] binds Rx port 0, queue 2 with Tx port 1, queue 2.
> Hairpin map [2] binds
> Rx port 0, queue 3 with Tx port 2, queue 2,
> Rx port 0, queue 4 with Tx port 2, queue 3,
> Rx port 0, queue 5 with Tx port 2, queue 4.
>
> The new `hairpin-map` parameter is optional.
> If omitted, testpmd will create "default" hairpin maps.
>
Hi Gregory,
This patch is waiting for a while in the patchwork, not because there is
a problem with the implementation, but I have a feeling that it is
bloating the testpmd code with the hairpin feature which is not commonly
supported, and I don't know what to do.
Also there was no review on the code ...
As a brainstorm, what do you think move hairpin related code to its own
file, and have a kind of register logic to the commands, argument
parsing and usage() functions etc.
@David did something similar to move PMD specific testpmd code to the
drivers [1].
This can be initial steps for the more modular testpmd design and we can
introduce more features with this approach in the future, I think
hairpin support is a good instance to start this work, because of its
limited impact to users.
[1]
Commit 592ab76f9f0f ("app/testpmd: register driver specific commands")
Commit 94b3c1a72507 ("net/i40e: move testpmd commands")
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---
> v2: Fix Windows Server 2019 compilation failure.
> v3: Update the patch comment.
> Fix typo.
> ---
> app/test-pmd/parameters.c | 63 ++++++++
> app/test-pmd/testpmd.c | 212 ++++++++++++++++++--------
> app/test-pmd/testpmd.h | 18 +++
> doc/guides/testpmd_app_ug/run_app.rst | 3 +
> 4 files changed, 230 insertions(+), 66 deletions(-)
>
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index a9ca58339d..c6bdfdf06f 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -206,6 +206,12 @@ usage(char* progname)
> printf(" --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n"
> " 0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
> " 0x01 - hairpin ports loop, 0x00 - hairpin port self\n");
> + printf(" --hairpin-map=rxpi:rxq:txpi:txq:n: hairpin map.\n"
> + " rxpi - Rx port index.\n"
> + " rxq - Rx queue.\n"
> + " txpi - Tx port index.\n"
> + " txq - Tx queue.\n"
> + " n - hairpin queues number.\n");
>
'rxpi' or 'port index' are not common usage in DPDK, what about
'rx_port' & 'Rx port id' or similar?
> }
>
> #ifdef RTE_LIB_CMDLINE
> @@ -588,6 +594,55 @@ parse_link_speed(int n)
> return speed;
> }
>
> +static __rte_always_inline
> +char *parse_hairpin_map_entry(char *input, char **next)
> +{
>
Why forcing this function to be inline, can make it static and left
decision to compiler.
<...>
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> index 6e9c552e76..a202c98b4c 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -566,6 +566,9 @@ The command line options are:
>
> The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
>
> +* ``--hairpin-map=Rx port id:Rx queue:Tx port id:Tx queue:queues number``
> +
> + Set explicit hairpin configuration.
>
What 'queues number' does is not intuitive, sample in the commit log
makes it easier to understand.
Can you please either explain or provide sample to clarify what "queues
number" is for?
[-- Attachment #2: Type: text/html, Size: 11365 bytes --]
prev parent reply other threads:[~2024-03-05 3:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-19 10:10 [PATCH] " Gregory Etelson
2023-09-20 12:22 ` [PATCH v2] " Gregory Etelson
2023-09-28 15:36 ` [PATCH v3] " Gregory Etelson
2023-12-14 8:06 ` Dariusz Sosnowski
2024-03-01 18:54 ` Ferruh Yigit
2024-03-05 3:04 ` Gregory Etelson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=IA1PR12MB63321EBEDE6160C2F6B4E0AFA5222@IA1PR12MB6332.namprd12.prod.outlook.com \
--to=getelson@nvidia.com \
--cc=aman.deep.singh@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=mkashani@nvidia.com \
--cc=thomas@monjalon.net \
--cc=yuying.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).