> -----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. 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. 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. > > >
