From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 2618A199B3 for ; Wed, 20 Sep 2017 12:46:01 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Sep 2017 03:45:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,421,1500966000"; d="scan'208";a="902168979" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by FMSMGA003.fm.intel.com with ESMTP; 20 Sep 2017 03:45:50 -0700 To: Wei Zhao , dev@dpdk.org, Jingjing Wu 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: Date: Wed, 20 Sep 2017 11:45:49 +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: <1505445188-70251-3-git-send-email-wei.zhao1@intel.com> 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: Wed, 20 Sep 2017 10:46:02 -0000 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. > 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 ..." ? > + > , list_pkt_forwarding_modes() > ); > } > @@ -8224,6 +8239,315 @@ cmdline_parse_inst_t cmd_syn_filter = { > NULL, > }, > }; > +/* *** queue region set *** */ > +struct cmd_queue_region_result { > + cmdline_fixed_string_t cmd; > + cmdline_fixed_string_t set; > + cmdline_fixed_string_t port; > + uint8_t port_id; > + cmdline_fixed_string_t region; > + uint8_t region_id; > + cmdline_fixed_string_t queue_start_index; > + uint8_t queue_id; > + cmdline_fixed_string_t queue_num; > + uint8_t queue_num_value; > +}; > + > +static void > +cmd_queue_region_parsed(void *parsed_result, > + __attribute__((unused)) struct cmdline *cl, > + __attribute__((unused)) void *data) > +{ > + struct cmd_queue_region_result *res = parsed_result; > + struct rte_i40e_rss_region_conf region_conf; > + int ret = 0; > + > + memset(®ion_conf, 0, sizeof(region_conf)); > + region_conf.op = RTE_PMD_I40E_QUEUE_REGION_SET; > + region_conf.region_id = res->region_id; > + region_conf.queue_num = res->queue_num_value; > + region_conf.queue_start_index = res->queue_id; > + > + ret = rte_pmd_i40e_queue_region_conf(res->port_id, ®ion_conf); It is not safe to directly call PMD specific APIs from testpmd? What if that PMD is not enabled? There are samples how to do this, can you please check them? <...>