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: <F95AC9340317A84688A5F0DF0246F3F20151FE86@szxeml504-mbs.china.huawei.com>
Date:   Fri, 23 Dec 2016 06:14:24 +0000
From:   maowenan <maowenan@...wei.com>
To:     Jeff Kirsher <jeffrey.t.kirsher@...el.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



> -----Original Message-----
> From: Jeff Kirsher [mailto:jeffrey.t.kirsher@...el.com]
> Sent: Friday, December 23, 2016 9:07 AM
> To: maowenan; Alexander Duyck
> Cc: Stephen Hemminger; netdev@...r.kernel.org; weiyongjun (A);
> Dingtianhong
> 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.


Thank you Jeff and Alex.
And then I have gone through mail thread about "i40e: enable PCIe relax ordering for SPARC",
It only works for SPARC, any other ARCH who wants to enable DMA_ATTR_WEAK_ORDERING 
feature, should define the new macro, recompile the driver module.

Because of the above reasons, we implement in ethtool to give the final user a convenient
way to on|off special feature, no need define new macro, easy to extend the new features,
and also good benefit for other driver as Jeff referred.