From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id EBC244C73 for ; Tue, 20 Mar 2018 16:03:51 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Mar 2018 08:03:50 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,336,1517904000"; d="scan'208";a="35367031" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.63]) ([10.237.221.63]) by FMSMGA003.fm.intel.com with ESMTP; 20 Mar 2018 08:03:48 -0700 To: Remy Horton , Shreyansh Jain Cc: Bruce Richardson , "Ananyev, Konstantin" , "dev@dpdk.org" , "Lu, Wenzhuo" , "Wu, Jingjing" , "Zhang, Qi Z" , "Xing, Beilei" , Thomas Monjalon 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> <297728c5-0a02-d13a-8c5d-c556258c55a5@intel.com> From: Ferruh Yigit Message-ID: <5eb4a1e9-9a90-e078-29e3-f61286b1b673@intel.com> Date: Tue, 20 Mar 2018 15:03:47 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <297728c5-0a02-d13a-8c5d-c556258c55a5@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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: Tue, 20 Mar 2018 15:03:52 -0000 On 3/16/2018 3:36 PM, Remy Horton wrote: > > 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; > }; Hi Remy, There are already two members in "struct rte_eth_dev_info": "struct rte_eth_rxconf default_rxconf;" "struct rte_eth_txconf default_txconf;" These two are filled by PMDs. I think we can say these are PMD preferred values for rte_eth_[rt]xconf structs. Right now we are extending the preferred values that PMDs can provide. So what about using same naming convention to be consistent with existing usage? Something like "struct rte_eth_portconf default_portconf"? > > >> 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. >>