public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff Kirsher <jeffrey.t.kirsher@intel•com>
To: maowenan <maowenan@huawei•com>,
	Alexander Duyck <alexander.duyck@gmail•com>
Cc: Stephen Hemminger <stephen@networkplumber•org>,
	"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
	"weiyongjun (A)" <weiyongjun1@huawei•com>,
	Dingtianhong <dingtianhong@huawei•com>
Subject: Re: [PATCH] ethtool: add one ethtool option to set relax ordering mode
Date: Thu, 22 Dec 2016 17:06:43 -0800	[thread overview]
Message-ID: <1482455203.2481.31.camel@intel.com> (raw)
In-Reply-To: <F95AC9340317A84688A5F0DF0246F3F20151FD1C@szxeml504-mbs.china.huawei.com>

[-- Attachment #1: Type: text/plain, Size: 3540 bytes --]

On Fri, 2016-12-23 at 00:40 +0000, maowenan wrote:
> > -----Original Message-----
> > From: Alexander Duyck [mailto:alexander.duyck@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 <maowenan@huawei•com>
> > wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Stephen Hemminger [mailto:stephen@networkplumber•org]
> > > > Sent: Thursday, December 22, 2016 9:28 AM
> > > > To: maowenan
> > > > Cc: netdev@vger•kernel.org; jeffrey.t.kirsher@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 <maowenan@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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2016-12-23  1:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-08  6:51 [PATCH] net: add one ethtool option to set relax ordering mode Mao Wenan
2016-12-08  6:51 ` [PATCH] ethtool: " Mao Wenan
2016-12-22  1:27   ` Stephen Hemminger
2016-12-22  1:39     ` maowenan
2016-12-22 15:53       ` Alexander Duyck
2016-12-23  0:40         ` maowenan
2016-12-23  1:06           ` Jeff Kirsher [this message]
2016-12-23  6:14             ` maowenan
2016-12-23 15:42               ` Alexander Duyck
2016-12-24  8:30                 ` maowenan
2016-12-26  8:33                 ` maowenan
2017-01-04  9:02                 ` maowenan
2017-01-04 16:50                   ` Alexander Duyck
2016-12-08 14:11 ` [PATCH] net: " Andrew Lunn
2016-12-12  8:30   ` maowenan
2016-12-22  0:10   ` maowenan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1482455203.2481.31.camel@intel.com \
    --to=jeffrey.t.kirsher@intel$(echo .)com \
    --cc=alexander.duyck@gmail$(echo .)com \
    --cc=dingtianhong@huawei$(echo .)com \
    --cc=maowenan@huawei$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=stephen@networkplumber$(echo .)org \
    --cc=weiyongjun1@huawei$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox