From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id B75DD3230 for ; Mon, 25 Sep 2017 11:43:26 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Sep 2017 02:43:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,435,1500966000"; d="scan'208";a="903551552" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by FMSMGA003.fm.intel.com with ESMTP; 25 Sep 2017 02:43:24 -0700 To: "Zhao1, Wei" , "dev@dpdk.org" , "Wu, Jingjing" References: <1505282644-40415-1-git-send-email-wei.zhao1@intel.com> <1505445188-70251-1-git-send-email-wei.zhao1@intel.com> <1505445188-70251-3-git-send-email-wei.zhao1@intel.com> From: Ferruh Yigit Message-ID: <1e407605-15e8-4a2f-65ea-078d80593e68@intel.com> Date: Mon, 25 Sep 2017 10:43:24 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: add API for configuration of queue region X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Sep 2017 09:43:27 -0000 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 ; dev@dpdk.org; Wu, Jingjing >> >> 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 >>> --- >>> 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] :) [1] https://blog.codinghorror.com/the-broken-window-theory/ <...>