On Fri, 2016-12-23 at 00:40 +0000, maowenan wrote: > > -----Original Message----- > > From: Alexander Duyck [mailto:alexander.du...@gmail.com] > > Sent: Thursday, December 22, 2016 11:54 PM > > To: maowenan > > Cc: Stephen Hemminger; netdev@vger.kernel.org; jeffrey.t.kirsher@intel. > > com; > > weiyongjun (A); Dingtianhong > > Subject: Re: [PATCH] ethtool: add one ethtool option to set relax > > ordering mode > > > > On Wed, Dec 21, 2016 at 5:39 PM, maowenan <maowe...@huawei.com> > > wrote: > > > > > > > > > > -----Original Message----- > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org] > > > > Sent: Thursday, December 22, 2016 9:28 AM > > > > To: maowenan > > > > Cc: netdev@vger.kernel.org; jeffrey.t.kirs...@intel.com > > > > Subject: Re: [PATCH] ethtool: add one ethtool option to set relax > > > > ordering mode > > > > > > > > On Thu, 8 Dec 2016 14:51:38 +0800 > > > > Mao Wenan <maowe...@huawei.com> wrote: > > > > > > > > > This patch provides one way to set/unset IXGBE NIC TX and RX > > > > > relax > > > > > ordering mode, which can be set by ethtool. > > > > > Relax ordering is one mode of 82599 NIC, to enable this mode can > > > > > enhance the performance for some cpu architecure. > > > > > > > > Then it should be done by CPU architecture specific quirks > > > > (preferably in PCI > > > > layer) so that all users get the option without having to do manual > > > > intervention. > > > > > > > > > example: > > > > > ethtool -s enp1s0f0 relaxorder off > > > > > ethtool -s enp1s0f0 relaxorder on > > > > > > > > Doing it via ethtool is a developer API (for testing) not something > > > > that makes sense in production. > > > > > > > > > This feature is not mandatory for all users, acturally relax ordering > > > default configuration of 82599 is 'disable', So this patch gives one > > > way to > > > > enable relax ordering to be selected in some performance condition. > > > > That isn't quite true. The default for Sparc systems is to have it > > enabled. > > > > Really this is something that is platform specific. I agree with > > Stephen that it > > would work better if this was handled as a series of platform specific > > quirks > > handled at something like the PCI layer rather than be a switch the > > user can > > toggle on and off. > > > > With that being said there are changes being made that should help to > > improve > > the situation. Specifically I am looking at adding support for the > > DMA_ATTR_WEAK_ORDERING which may also allow us to identify cases where > > you might be able to specify the DMA behavior via the DMA mapping > > instead of > > having to make the final decision in the device itself. > > > > - Alex > > Yes, Sparc is a special case. From the NIC driver point of view, It is no > need for > some ARCHs to do particular operation and compiling branch, ethtool is a > flexible > method for user to make decision whether on|off this feature. > I think Jeff as maintainer of 82599 has some comments about this.
My original comment/objection was that you attempted to do this change as a module parameter to the ixgbe driver, where I directed you to use ethtool so that other drivers could benefit from the ability to enable/disable relaxed ordering. As far as how it gets implemented in ethtool or PCI layer, makes little difference to me, I only had issues with the driver specific module parameter implementation, which is not acceptable.
signature.asc
Description: This is a digitally signed message part