Hi，Ferruh > -----Original Message----- > From: Yigit, Ferruh > Sent: Monday, September 25, 2017 5:43 PM > To: Zhao1, Wei <wei.zh...@intel.com>; dev@dpdk.org; Wu, Jingjing > <jingjing...@intel.com> > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: add API for > configuration of queue region > > On 9/25/2017 10:25 AM, Zhao1, Wei wrote: > > Hi, Ferruh > > > >> -----Original Message----- > >> From: Yigit, Ferruh > >> Sent: Wednesday, September 20, 2017 6:46 PM > >> To: Zhao1, Wei <wei.zh...@intel.com>; dev@dpdk.org; Wu, Jingjing > >> <jingjing...@intel.com> > >> Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: add API for > >> configuration of queue region > >> > >> On 9/15/2017 4:13 AM, Wei Zhao wrote: > >>> This patch add a API configuration of queue region in rss. > >>> It can parse the parameters of region index, queue number, queue > >>> start index, user priority, traffic classes and so on. > >>> According to commands from command line, it will call i40e private > >>> API and start the process of set or flush queue region configure. As > >>> this feature is specific for i40e, so private API will be used. > >>> > >>> Signed-off-by: Wei Zhao <wei.zh...@intel.com> > >>> --- > >>> app/test-pmd/cmdline.c | 328 > >>> +++++++++++++++++++++++++++++++++++++++++++++++++ > >> > >> Testpmd documentation also needs to be updated. > > > > Do you mean the following doc or others? > > dpdk\doc\guides\testpmd_app_ug.rst > > Yes this one, thanks. > > > > > > >> > >>> 1 file changed, 328 insertions(+) > >>> > >>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index > >>> 0144191..060fcb1 100644 > >>> --- a/app/test-pmd/cmdline.c > >>> +++ b/app/test-pmd/cmdline.c > >>> @@ -637,6 +637,21 @@ static void cmd_help_long_parsed(void > >> *parsed_result, > >>> "ptype mapping update (port_id) (hw_ptype) > >> (sw_ptype)\n" > >>> " Update a ptype mapping item on a port\n\n" > >>> > >>> + "queue-region set port (port_id) region_id (value) " > >>> + "queue_start_index (value) queue_num (value)\n" > >>> + " Set a queue region on a port\n\n" > >>> + > >>> + "queue-region set (pf|vf) port (port_id) region_id > >> (value) " > >>> + "flowtype (value)\n" > >>> + " Set a flowtype region index on a port\n\n" > >>> + > >>> + "queue-region set port (port_id) UP (value) > >> region_id (value)\n" > >>> + " Set the mapping of User Priority to " > >>> + "queue region on a port\n\n" > >>> + > >>> + "queue-region flush (on|off) port (port_id)\n" > >>> + " flush all queue region related configuration\n\n" > >> > >> I keep doing same comment but I will do it again... > >> > >> Each patch adding a new feature looking from its own context and > >> adding a new root level command and this is making overall testpmd > confusing. > >> > >> Since this is to set an option of the port, what do you think making > >> this command part of existing commands, like: > >> "port config #P queue-region ...." > >> OR > >> "set port #P queue-region ..." ? > > > > What you said is very meaningful, but other feature liake ptype mapping > use the same mode and so on. > > maybe we should do a whole work to make CLI command style consistent. > > Yes ptype does it, is seems it is one of the missed ones. Although we can do a > whole work for CLI commands, meanwhile I think new ones can be added > properly. > > This may be good opportunity to remember broken window theory [1] :)
But this type of CLI for queue region has been discussion when this feature skype meeting We have do a ppt, which review by DPDK-ENG-TECH-COMMITTEE, they have support and approve this type of command for this feature. I think we should respect their review wok and decision of other committee member. AND also, The minority is subordinate to the majority, do you think so ? I do not want do hold a second meeting later to persuade them accept the new type of CLI command. They may not also not change their idea too. > > [1] > https://blog.codinghorror.com/the-broken-window-theory/ > > <...>
