> -----Original Message----- > From: Yigit, Ferruh > Sent: Wednesday, March 14, 2018 9:03 PM > To: Ananyev, Konstantin <konstantin.anan...@intel.com>; 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 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.
That's ok by me, but it means every PMD has to provide some preferred values? Konstantin