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 C3E895F11 for ; Fri, 16 Mar 2018 16:36:11 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Mar 2018 08:36:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,316,1517904000"; d="scan'208";a="212220867" Received: from rhorton-mobl1.ger.corp.intel.com (HELO [163.33.230.232]) ([163.33.230.232]) by fmsmga005.fm.intel.com with ESMTP; 16 Mar 2018 08:36:08 -0700 To: Shreyansh Jain , Ferruh Yigit References: <20180307120851.5822-2-remy.horton@intel.com> <023fbd6c-7cac-6c8b-9a40-7a62e5d47bb7@intel.com> <30b8575d-4aeb-912d-6f74-c49ad7ce879a@intel.com> <591e1a23-8d27-0c59-fd39-0bde9e48e96f@intel.com> <2601191342CEEE43887BDE71AB9772589E28FD57@irsmsx105.ger.corp.intel.com> <2b3a2579-6bce-55f5-6e03-0974729cc95b@intel.com> <20180314213658.GA108@bricha3-MOBL.ger.corp.intel.com> <20180315143924.GA9172@bricha3-MOBL.ger.corp.intel.com> <97dc9f9d-041b-ef99-2ca6-1f557c4f6039@intel.com> Cc: Bruce Richardson , "Ananyev, Konstantin" , "dev@dpdk.org" , "Lu, Wenzhuo" , "Wu, Jingjing" , "Zhang, Qi Z" , "Xing, Beilei" , Thomas Monjalon From: Remy Horton Organization: Intel Shannon Limited Message-ID: <297728c5-0a02-d13a-8c5d-c556258c55a5@intel.com> Date: Fri, 16 Mar 2018 15:36:08 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH v1 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters 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: Fri, 16 Mar 2018 15:36:13 -0000 On 16/03/2018 13:54, Shreyansh Jain wrote: > On Thu, Mar 15, 2018 at 8:27 PM, Ferruh Yigit wrote: >> On 3/15/2018 2:39 PM, Bruce Richardson wrote: >>> On Thu, Mar 15, 2018 at 01:57:13PM +0000, Ferruh Yigit wrote: [..] >> Hi Remy, Shreyansh, >> >> What do you think about using a variable name consistent with existing >> "default_[rt]xconf" in dev_info? > > It just turned out to be much more complex than I initially thought :) > Is this what the above conversation merging at (for Rx, as example): > > 1. 'default_rx_size_conf' is added in rte_eth_dev_info (and this > includes I/O params like burst size, besides configure time nb_queue, > nb_desc etc). Driver would return these values filled in when > info_get() is called. At the moment thinking of the names below, based in what I've read so far.. struct rte_eth_dev_preferred_size { uint16_t rx_burst; uint16_t tx_burst; uint16_t rx_ring; uint16_t tx_ring; uint16_t rx_nb_queues; uint16_t tx_nb_queues; }; struct rte_eth_dev_info { /* ... */ struct rte_eth_dev_preferred_size preferred_size; }; Since Rx and Tx use the same parameters, a possible alternative is below, although such separation of Rx & Tx was not something I was planning on doing: struct rte_eth_dev_preferred_size { uint16_t burst; uint16_t ring; uint16_t nb_queues; }; struct rte_eth_dev_info { /* ... */ struct rte_eth_dev_preferred_size preferred_rx; struct rte_eth_dev_preferred_size preferred_tx; }; > 2a. If an application needs the defaults, it would perform info_get() > and get the values. then, use the values in configuration APIs > (rx_queue_setup for nb_rx_desc, eth_dev_dev_configure for > nb_rx_queues). > For rx_burst calls, it would use the burst_size fields obtained from info_get(). > This is good enough for configuration and datapath (rx_burst). There was also the suggestion of adding a completely new API function rather than using info_get() although there might be some resistance as struct eth_dev_ops is already pretty large. > OR, another case > > 2b. Application wants to use default vaules provided by driver without > calling info_get. In which case, it would call > rx_queue_setup(nb_rx_desc=0..) or eth_dev_configure(nb_rx_queue=0, > nb_tx_queue=0). The implementation would query the value from > 'default_rx_size_conf' through info_get() and use those values. > Though, in this case, rte_eth_rx_burst(burst=0) might not work for > picking up the default within rte_ethdev.h. Since rte_eth_*_burst() are fast-path functions, they are places I would prefer not to put conditionals. > :Four observations: > A). For burst size (or any other I/O time value added in future), > values would have to be explicitly used by application - always. If > value reported by info_get() is '0' (see (B) below), application to > use its own judgement. No default override by lib_eal. > IMO, This is good enough assumption. > > B). '0' as an indicator for 'no-default-value-available-from-driver' > is still an open point. It is good enough for current proposed > parameters, but may be a valid numerical value in future. > IMO, this can be ignored for now. > > C) Unlike the original proposal, this would add two separate members > to rte_eth_dev_info - one each for Rx and Tx. They both are still > expected to be populated through the info_get() implementation but not > by lib_eal. > IMO, doesn't matter. There's been quite a bit of discussion whether ethdev should provide fall-back values if the PMD doesn't. In this case applications can assume the value is always valid and it makes the 0-as-indicator issue disappear, but it comes with its own set of issues. > D) Would there be no non-Rx and non-Tx defaults which need to be shared? > I am not sure about this, though. I can't think of any off-hand. > > Sorry if I am repeating everything again, but I got lost in the > conversation and needed to break it again. >