<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> Hello Ferruh,</div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> </div> <blockquote style="margin-left: 0.8ex; padding-left: 1ex; border-left: 3px solid rgb(200, 200, 200);"> <div class="elementToProof"><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">As a brainstorm, what do you think move hairpin related code to its own <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">file, and have a kind of register logic to the commands, argument</div> <div class="elementToProof"><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">parsing and usage() functions etc.</div> </blockquote> <div class="elementToProof"><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> </div> <div class="elementToProof">Please consider an option for the testpmd to dynamically add supported commands.</div> <div class="elementToProof">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.</div> <div class="elementToProof"> </div> <div class="elementToProof">Regards,</div> <div class="elementToProof">Gregory</div> <div id="appendonsend"></div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> </div> <hr style="display: inline-block; width: 98%;"> <div id="divRplyFwdMsg" dir="ltr">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 <div> </div> </div> <div>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? </div> </body> </html>