netdev - Re: [PATCH] ethtool: add one ethtool option to set relax ordering mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1482455203.2481.31.camel@intel.com>
Date:   Thu, 22 Dec 2016 17:06:43 -0800
From:   Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:     maowenan <maowenan@...wei.com>,
        Alexander Duyck <alexander.duyck@...il.com>
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "weiyongjun (A)" <weiyongjun1@...wei.com>,
        Dingtianhong <dingtianhong@...wei.com>
Subject: Re: [PATCH] ethtool: add one ethtool option to set relax ordering
 mode

On Fri, 2016-12-23 at 00:40 +0000, maowenan wrote:
> > -----Original Message-----
> > From: Alexander Duyck [mailto:alexander.duyck@...il.com]
> > Sent: Thursday, December 22, 2016 11:54 PM
> > To: maowenan
> > Cc: Stephen Hemminger; netdev@...r.kernel.org; jeffrey.t.kirsher@...el.
> > 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 <maowenan@...wei.com>
> > wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Stephen Hemminger [mailto:stephen@...workplumber.org]
> > > > Sent: Thursday, December 22, 2016 9:28 AM
> > > > To: maowenan
> > > > Cc: netdev@...r.kernel.org; jeffrey.t.kirsher@...el.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 <maowenan@...wei.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.
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)