On 3/14/2018 6:53 PM, Ananyev, Konstantin wrote: > > >> -----Original Message----- >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit >> Sent: Wednesday, March 14, 2018 5:52 PM >> To: Shreyansh Jain <shreyansh.j...@nxp.com>; Horton, Remy >> <remy.hor...@intel.com>; dev@dpdk.org >> Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Wu, Jingjing >> <jingjing...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; Xing, Beilei >> <beilei.x...@intel.com>; Thomas Monjalon <tho...@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.yi...@intel.com] >>>> Sent: Wednesday, March 14, 2018 10:13 PM >>>> To: Remy Horton <remy.hor...@intel.com>; dev@dpdk.org >>>> Cc: Wenzhuo Lu <wenzhuo...@intel.com>; Jingjing Wu >>>> <jingjing...@intel.com>; Qi Zhang <qi.z.zh...@intel.com>; Beilei Xing >>>> <beilei.x...@intel.com>; Shreyansh Jain <shreyansh.j...@nxp.com>; >>>> Thomas Monjalon <tho...@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. >> >>> >
