From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 39CA15F2B for ; Wed, 14 Mar 2018 22:02:52 +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 fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Mar 2018 14:02:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,307,1517904000"; d="scan'208";a="211622322" Received: from aduterqu-mobl.ger.corp.intel.com (HELO [10.252.20.45]) ([10.252.20.45]) by fmsmga005.fm.intel.com with ESMTP; 14 Mar 2018 14:02:48 -0700 To: "Ananyev, Konstantin" , Shreyansh Jain , "Horton, Remy" , "dev@dpdk.org" Cc: "Lu, Wenzhuo" , "Wu, Jingjing" , "Zhang, Qi Z" , "Xing, Beilei" , Thomas Monjalon References: <20180307120851.5822-1-remy.horton@intel.com> <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> From: Ferruh Yigit Message-ID: <2b3a2579-6bce-55f5-6e03-0974729cc95b@intel.com> Date: Wed, 14 Mar 2018 21:02: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: <2601191342CEEE43887BDE71AB9772589E28FD57@irsmsx105.ger.corp.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: Wed, 14 Mar 2018 21:02:53 -0000 On 3/14/2018 6:53 PM, Ananyev, Konstantin wrote: > > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit >> Sent: Wednesday, March 14, 2018 5:52 PM >> To: Shreyansh Jain ; Horton, Remy ; dev@dpdk.org >> Cc: Lu, Wenzhuo ; Wu, Jingjing ; Zhang, Qi Z ; Xing, Beilei >> ; Thomas Monjalon >> Subject: Re: [dpdk-dev] [RFC PATCH v1 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters >> >> On 3/14/2018 5:23 PM, Shreyansh Jain wrote: >>>> -----Original Message----- >>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] >>>> Sent: Wednesday, March 14, 2018 10:13 PM >>>> To: Remy Horton ; dev@dpdk.org >>>> Cc: Wenzhuo Lu ; Jingjing Wu >>>> ; Qi Zhang ; Beilei Xing >>>> ; Shreyansh Jain ; >>>> Thomas Monjalon >>>> Subject: Re: [dpdk-dev] [RFC PATCH v1 1/4] ethdev: add support for PMD- >>>> tuned Tx/Rx parameters >>>> >>>> On 3/14/2018 3:48 PM, Remy Horton wrote: >>>>> >>>>> On 14/03/2018 14:43, Ferruh Yigit wrote: >>>>> [..] >>>>>>> lib/librte_ether/rte_ethdev.c | 18 ++++++++++++++++++ >>>>>>> lib/librte_ether/rte_ethdev.h | 15 +++++++++++++++ >>>>>> >>>>>> Can you please remove deprecation notice in this patch. >>>>> >>>>> Done. >>>>> >>>>>>> + /* Defaults for drivers that don't implement preferred >>>>>>> + * queue parameters. >>>>> [..] >>>>>> Not sure about having these defaults here. It is OK to have defaults >>>> in driver, >>>>>> in application or in config file, but I am not sure if putting them >>>> into device >>>>>> abstraction layer hides them. >>>>>> >>>>>> What about not providing any default in ethdev layer, and get zero >>>> as invalid >>>>>> when using them? >>>>> >>>>> This is something I have been thinking about, and I am going to >>>> remove >>>>> them for the V2. Original motive was to avoid breaking testpmd for >>>> PMDs >>>>> that don't give defaults (i.e. almost all of them). The alternative >>>> is >>>>> to put place-holders into all the PMDs themselves, but I am not sure >>>> if >>>>> this is appropriate. >>>> >>>> I think preferred values should be optional, PMD should have right to >>>> not >>>> provide any. Implementation in 4/4 forces preferred values, either in >>>> all PMDs >>>> or in ethdev layer. >>>> >>>> What about changing approach in application: >>>> is preferred value provided [1] ? >>>> yes => use it by sending value 0 >>>> no => use application provided value, same as now, so control should >>>> be in >>>> application. >>>> >>>> I am aware this breaks the comfort of just providing 0 and PMD values >>>> will be >>>> used but covers the case there is no PMD values. >>>> >>>> [1] >>>> it can be possible to check if preferred value provided by comparing 0, >>>> but if 0 >>>> is a valid value that can be problem. It may not be problem with >>>> current >>>> variables but it may be when this struct extended, it may be good to >>>> think about >>>> alternative here. >>> >>> I don't think we should use the condition of "yes => use it by sending value 0". That is non-intuitive. Ideally, the application should query >> and then if query responds with value as '0' (which can be valid for some variables in future), it sends its own value to setup functions >> (whether '0' or something else, in case of 0 response, would depend on the knob). >> >> Right, at that stage application already knows what is the preferred value and >> can directly use it. >> >> >> Will it be too much to: >> >> 1) Adding a new field into "rte_eth_[rt]xconf" to say if exists prefer PMD >> values. "prefer_device_values" ? >> Application can provide values as usual, but if that field is set, abstraction >> layer overwrites the application values with PMD preferred ones. If there is no >> PMD preferred values continue using application ones. >> >> >> 2) Add a bitwise "is_set" field to new "preferred_size" struct, which may show >> status of other fields in the struct, if PMD set a valid value for them or not, >> so won't have to rely on the 0 check. > > That all seems like too much hassle for such small thing. Fair enough. > If we really want to allow PMD not to provide preferred values - > then instead of adding rte_eth_dev_pref_info into dev_info we can simply > introduce a new optional ethdev API call: > rte_eth_get_pref_params() or so. > If the PMD doesn’t want to provide preferred params to the user it simply > wouldn't implement that function. Same can be done with updated rte_eth_dev_info. Only application needs to check and use PMD preferred values, so this will mean dropping "pass 0 to get preferred values" feature in initial set. > > Konstantin > >> >>> >>> Existing example applications should be changed for this. It is tedious, but gives a true example usage. >> >> Applications already needs to be updated to use this, important part is >> modification is optional. >> >>> >