RE: [PATCH] ethtool: add one ethtool option to set relax ordering mode

-----Original Message-----
From: maowenan
Sent: Saturday, December 24, 2016 4:30 PM
To: 'Alexander Duyck'
Cc: Jeff Kirsher; Stephen Hemminger; netdev@vger.kernel.org; weiyongjun (A);
Dingtianhong; Wangzhou (B)
Subject: RE: [PATCH] ethtool: add one ethtool option to set relax ordering mode

quoted

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
Sent: Friday, December 23, 2016 11:43 PM
To: maowenan
Cc: Jeff Kirsher; Stephen Hemminger; netdev@vger.kernel.org;
weiyongjun (A); Dingtianhong
Subject: Re: [PATCH] ethtool: add one ethtool option to set relax
ordering mode

On Thu, Dec 22, 2016 at 10:14 PM, maowenan [off-list ref]
wrote:

quoted

quoted

-----Original Message-----
From: Jeff Kirsher [mailto:jeffrey.t.kirsher@intel.com]
Sent: Friday, December 23, 2016 9:07 AM
To: maowenan; Alexander Duyck
Cc: Stephen Hemminger; netdev@vger.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:

quoted

quoted

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

quoted

quoted

quoted

quoted

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

[off-list ref]

quoted

quoted

quoted

quoted

wrote:

quoted

quoted

-----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
[off-list ref] wrote:

quoted

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.

quoted

quoted

quoted

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.

quoted

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.

quoted

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.

quoted

I think the point is we shouldn't base the decision on user input.
The fact is the PCIe device control register should have a bit that
indicates if the device is allowed to enable relaxed ordering or not.
If we can guarantee that the bit is set in all the cases where it
should be set, and cleared in all the cases where it should not then
we could use something like that to determine if the device is
supposed to enable relaxed ordering instead of trying to make the decision

ourselves.

quoted

- Alex

ok. We are focusing on the register.
And yes, to enable relax ordering for 82599 should be set by one or more bits of
Rx/TX DCA Control Register, these bits should be set in many cpu architectures,
such as arm64, sparc, and so on, and should be cleared in other ARCHs.
By the way, how do you enable SPARC macro, how and where to define this
compiling macro when user one to enable relax ordering under SPARC system?
#ifndef CONFIG_SPARC

Hi, Alex,
Have you already sent out the patches about DMA_ATTR_WEAK_ORDERING?
We want to get you how to enable DMA_ATTR_WEAK_ORDERING by PCIe layer,
and we can refer to that.