[dpdk-dev] [PATCH v3 2/2] app/testpmd: add API for configuration of queue region

Zhao1, Wei wei.zhao1 at intel.com
Tue Sep 26 07:30:34 CEST 2017


Hi,Ferruh

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, September 25, 2017 5:43 PM
> To: Zhao1, Wei <wei.zhao1 at intel.com>; dev at dpdk.org; Wu, Jingjing
> <jingjing.wu at 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.zhao1 at intel.com>; dev at dpdk.org; Wu, Jingjing
> >> <jingjing.wu at 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.zhao1 at 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/
> 
> <...>



More information about the dev mailing list