From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <ferruh.yigit@intel.com> Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 39CA15F2B for <dev@dpdk.org>; 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" <konstantin.ananyev@intel.com>, Shreyansh Jain <shreyansh.jain@nxp.com>, "Horton, Remy" <remy.horton@intel.com>, "dev@dpdk.org" <dev@dpdk.org> Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>, "Wu, Jingjing" <jingjing.wu@intel.com>, "Zhang, Qi Z" <qi.z.zhang@intel.com>, "Xing, Beilei" <beilei.xing@intel.com>, Thomas Monjalon <thomas@monjalon.net> 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> <c9f54a60-4c22-1b02-df87-f21789e9b8a4@intel.com> <HE1PR0402MB27809256C942FA6F9B4836FC90D10@HE1PR0402MB2780.eurprd04.prod.outlook.com> <591e1a23-8d27-0c59-fd39-0bde9e48e96f@intel.com> <2601191342CEEE43887BDE71AB9772589E28FD57@irsmsx105.ger.corp.intel.com> From: Ferruh Yigit <ferruh.yigit@intel.com> 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 <dev.dpdk.org> List-Unsubscribe: <https://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=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 <shreyansh.jain@nxp.com>; Horton, Remy <remy.horton@intel.com>; dev@dpdk.org >> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei >> <beilei.xing@intel.com>; Thomas Monjalon <thomas@monjalon.net> >> 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 <remy.horton@intel.com>; dev@dpdk.org >>>> Cc: Wenzhuo Lu <wenzhuo.lu@intel.com>; Jingjing Wu >>>> <jingjing.wu@intel.com>; Qi Zhang <qi.z.zhang@intel.com>; Beilei Xing >>>> <beilei.xing@intel.com>; Shreyansh Jain <shreyansh.jain@nxp.com>; >>>> Thomas Monjalon <thomas@monjalon.net> >>>> 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. >> >>> >