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

Subsystems: intel ethernet drivers, networking drivers, networking [ethtool], networking [general], the rest

STALE3533d

16 messages, 5 authors, 2017-01-04 · open the first message on its own page

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

From: Mao Wenan <hidden>
Date: 2016-12-08 06:58:31

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.
example:
ethtool -s enp1s0f0 relaxorder off
ethtool -s enp1s0f0 relaxorder on

Signed-off-by: Mao Wenan <redacted>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 34 ++++++++++++++++++++++++
 include/linux/ethtool.h                          |  2 ++
 include/uapi/linux/ethtool.h                     |  6 +++++
 net/core/ethtool.c                               |  5 ++++
 4 files changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index f49f803..9650539 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c

@@ -493,6 +493,39 @@ static void ixgbe_set_msglevel(struct net_device *netdev, u32 data)
 	adapter->msg_enable = data;
 }
 
+static void ixgbe_set_relaxorder(struct net_device *netdev, u32 data)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(netdev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 i = 0;
+	pr_info("set relax ordering mode : %s\n",data?"on":"off");
+	
+	for (i = 0; i < hw->mac.max_tx_queues; i++) {
+		u32 regval;
+
+		regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
+		if (data)
+			regval |= IXGBE_DCA_TXCTRL_DESC_WRO_EN;
+		else
+			regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
+		IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
+	}
+
+	for (i = 0; i < hw->mac.max_rx_queues; i++) {
+		u32 regval;
+		
+		regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
+		if (data)
+			regval |= (IXGBE_DCA_RXCTRL_DATA_WRO_EN |
+					IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
+		else
+			regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
+					IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
+		IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+	}
+
+}
+
 static int ixgbe_get_regs_len(struct net_device *netdev)
 {
 #define IXGBE_REGS_LEN  1139

@@ -3274,6 +3307,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = {
 	.get_ts_info		= ixgbe_get_ts_info,
 	.get_module_info	= ixgbe_get_module_info,
 	.get_module_eeprom	= ixgbe_get_module_eeprom,
+	.set_relaxorder         = ixgbe_set_relaxorder,
 };
 
 void ixgbe_set_ethtool_ops(struct net_device *netdev)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9ded8c6..0fae148 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h

@@ -286,6 +286,7 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
  *	fields should be ignored (use %__ETHTOOL_LINK_MODE_MASK_NBITS
  *	instead of the latter), any change to them will be overwritten
  *	by kernel. Returns a negative error code or zero.
+ * @set_relaxorder: set relax ordering mode, on|off.
  *
  * All operations are optional (i.e. the function pointer may be set
  * to %NULL) and callers must take this into account.  Callers must

@@ -372,5 +373,6 @@ struct ethtool_ops {
 				      struct ethtool_link_ksettings *);
 	int	(*set_link_ksettings)(struct net_device *,
 				      const struct ethtool_link_ksettings *);
+	void    (*set_relaxorder)(struct net_device *, u32);
 };
 #endif /* _LINUX_ETHTOOL_H */

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 8e54723..86349b9 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h

@@ -1314,6 +1314,8 @@ struct ethtool_per_queue_op {
 #define ETHTOOL_GLINKSETTINGS	0x0000004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS	0x0000004d /* Set ethtool_link_settings */
 
+#define ETHTOOL_SRELAXORDER	0x00000050 /* Set relax ordering mode, on or off*/
+
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET

@@ -1494,6 +1496,10 @@ static inline int ethtool_validate_speed(__u32 speed)
 #define DUPLEX_FULL		0x01
 #define DUPLEX_UNKNOWN		0xff
 
+/* Relax Ordering mode, on or off. */
+#define RELAXORDER_OFF          0x00
+#define RELAXORDER_ON           0x01
+
 static inline int ethtool_validate_duplex(__u8 duplex)
 {
 	switch (duplex) {

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 047a175..b7629d1 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c

@@ -2685,6 +2685,11 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SLINKSETTINGS:
 		rc = ethtool_set_link_ksettings(dev, useraddr);
 		break;
+	case ETHTOOL_SRELAXORDER:
+		rc = ethtool_set_value_void(dev, useraddr,
+					dev->ethtool_ops->set_relaxorder);
+		 break;
+
 	default:
 		rc = -EOPNOTSUPP;
 	}

-- 
2.7.0

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

From: Mao Wenan <hidden>
Date: 2016-12-08 06:58:32

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.
example:
ethtool -s enp1s0f0 relaxorder off
ethtool -s enp1s0f0 relaxorder on

Signed-off-by: Mao Wenan <redacted>
---
 ethtool-copy.h |  6 ++++++
 ethtool.c      | 24 +++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 3d299e3..37d93be 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h

@@ -1329,6 +1329,8 @@ struct ethtool_per_queue_op {
 #define ETHTOOL_PHY_GTUNABLE	0x0000004e /* Get PHY tunable configuration */
 #define ETHTOOL_PHY_STUNABLE	0x0000004f /* Set PHY tunable configuration */
 
+#define ETHTOOL_SRELAXORDER	0x00000050 /* Set relax ordering mode, on or off*/
+
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
 #define SPARC_ETH_SSET		ETHTOOL_SSET

@@ -1558,6 +1560,10 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
 #define WAKE_MAGIC		(1 << 5)
 #define WAKE_MAGICSECURE	(1 << 6) /* only meaningful if WAKE_MAGIC */
 
+/* Relax Ordering mode, on or off. */
+#define RELAXORDER_OFF		0x00
+#define RELAXORDER_ON		0x01
+
 /* L2-L4 network traffic flow types */
 #define	TCP_V4_FLOW	0x01	/* hash or spec (tcp_ip4_spec) */
 #define	UDP_V4_FLOW	0x02	/* hash or spec (udp_ip4_spec) */

diff --git a/ethtool.c b/ethtool.c
index 7af039e..acafd71 100644
--- a/ethtool.c
+++ b/ethtool.c

@@ -2738,6 +2738,8 @@ static int do_sset(struct cmd_context *ctx)
 	int msglvl_changed = 0;
 	u32 msglvl_wanted = 0;
 	u32 msglvl_mask = 0;
+	int relaxorder_wanted = -1;
+	int relaxorder_changed = 0;
 	struct cmdline_info cmdline_msglvl[ARRAY_SIZE(flags_msglvl)];
 	int argc = ctx->argc;
 	char **argp = ctx->argp;

@@ -2873,6 +2875,16 @@ static int do_sset(struct cmd_context *ctx)
 					ARRAY_SIZE(cmdline_msglvl));
 				break;
 			}
+		} else if (!strcmp(argp[i], "relaxorder")) {
+			relaxorder_changed = 1;
+			i += 1;
+			if (i >= argc)
+				exit_bad_args();
+			if (!strcmp(argp[i], "on"))
+				relaxorder_wanted = RELAXORDER_ON;
+			else if (!strcmp(argp[i], "off"))
+				relaxorder_wanted = RELAXORDER_OFF;
+			else	exit_bad_args();
 		} else {
 			exit_bad_args();
 		}

@@ -3093,6 +3105,15 @@ static int do_sset(struct cmd_context *ctx)
 		}
 	}
 
+	if (relaxorder_changed) {
+		struct ethtool_value edata;
+
+		edata.cmd = ETHTOOL_SRELAXORDER;
+		edata.data = relaxorder_wanted;
+		err = send_ioctl(ctx, &edata);
+		if (err < 0)
+			perror("Cannot set relax ordering mode");
+	}
 	return 0;
 }

@@ -4690,7 +4711,8 @@ static const struct option {
 	  "		[ xcvr internal|external ]\n"
 	  "		[ wol p|u|m|b|a|g|s|d... ]\n"
 	  "		[ sopass %x:%x:%x:%x:%x:%x ]\n"
-	  "		[ msglvl %d | msglvl type on|off ... ]\n" },
+	  "		[ msglvl %d | msglvl type on|off ... ]\n"
+	  "		[ relaxorder on|off ]\n" },
 	{ "-a|--show-pause", 1, do_gpause, "Show pause options" },
 	{ "-A|--pause", 1, do_spause, "Set pause options",
 	  "		[ autoneg on|off ]\n"

-- 
2.7.0

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

From: Andrew Lunn <andrew@lunn.ch>
Date: 2016-12-08 14:12:00

On Thu, Dec 08, 2016 at 02:51:37PM +0800, Mao Wenan 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.
example:
ethtool -s enp1s0f0 relaxorder off
ethtool -s enp1s0f0 relaxorder on

Since this is a simple on/off, could it not be done with a feature?
ethtool --feature?

	Andrew

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

From: maowenan <hidden>
Date: 2016-12-12 08:32:38

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of Andrew Lunn
Sent: Thursday, December 08, 2016 10:12 PM
To: maowenan
Cc: netdev@vger.kernel.org; jeffrey.t.kirsher@intel.com
Subject: Re: [PATCH] net: add one ethtool option to set relax ordering mode

On Thu, Dec 08, 2016 at 02:51:37PM +0800, Mao Wenan 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.
example:
ethtool -s enp1s0f0 relaxorder off
ethtool -s enp1s0f0 relaxorder on

Since this is a simple on/off, could it not be done with a feature?
ethtool --feature?

	Andrew

Hello Andrew, 
	Thank you for your comments.
	I get your idea about using ethtool -K|--feature is good for this feature, right? 
My original concert is about this is a relax ordering mode exist in 82599, it is the hardware 
related feature. And ethtool -s option is related hardware of phy and other (e.g: speed, duplex...),
it is very easy to implement in do_sset().
But ethtool -K is mainly used for protocol offload,
        ethtool -K|--features|--offload DEVNAME Set protocol offload and other features
                FEATURE on|off ... 
@Jeff Kirsher, what's your comments?

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

From: maowenan <hidden>
Date: 2016-12-22 00:10:43

Hi,

-----Original Message-----
From: maowenan
Sent: Monday, December 12, 2016 4:29 PM
To: 'Andrew Lunn'
Cc: netdev@vger.kernel.org; jeffrey.t.kirsher@intel.com; weiyongjun (A)
Subject: RE: [PATCH] net: add one ethtool option to set relax ordering mode

quoted

-----Original Message-----
From: netdev-owner@vger.kernel.org
[mailto:netdev-owner@vger.kernel.org]
On Behalf Of Andrew Lunn
Sent: Thursday, December 08, 2016 10:12 PM
To: maowenan
Cc: netdev@vger.kernel.org; jeffrey.t.kirsher@intel.com
Subject: Re: [PATCH] net: add one ethtool option to set relax ordering
mode

On Thu, Dec 08, 2016 at 02:51:37PM +0800, Mao Wenan 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.
example:
ethtool -s enp1s0f0 relaxorder off
ethtool -s enp1s0f0 relaxorder on

Since this is a simple on/off, could it not be done with a feature?
ethtool --feature?

	Andrew

Hello Andrew,
	Thank you for your comments.
	I get your idea about using ethtool -K|--feature is good for this feature,
right?
My original concert is about this is a relax ordering mode exist in 82599, it is
the hardware related feature. And ethtool -s option is related hardware of phy
and other (e.g: speed, duplex...), it is very easy to implement in do_sset().
But ethtool -K is mainly used for protocol offload,
        ethtool -K|--features|--offload DEVNAME Set protocol offload and
other features
                FEATURE on|off ...
@Jeff Kirsher, what's your comments?

@Jeff Kirsher and @David Miller, do you have any other comments about this patch which wants to add ethtool option to on|off relax ordering with 82599?  Thank you.
Here Andrew have comment about using -K or --features on|off relax ordering.

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

From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2016-12-22 01:28:07

On Thu, 8 Dec 2016 14:51:38 +0800
Mao Wenan [off-list ref] 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.

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

From: maowenan <hidden>
Date: 2016-12-22 01:39:56

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

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.

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

From: Alexander Duyck <hidden>
Date: 2016-12-22 15:53:48

On Wed, Dec 21, 2016 at 5:39 PM, maowenan [off-list ref] wrote:

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

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

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

From: maowenan <hidden>
Date: 2016-12-23 00:43:51

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

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

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

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

From: Jeff Kirsher <hidden>
Date: 2016-12-23 01:06:45

On Fri, 2016-12-23 at 00:40 +0000, maowenan wrote:

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.
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]
wrote:

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

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.

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

From: maowenan <hidden>
Date: 2016-12-23 06:14:52

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

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

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

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.

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

From: Alexander Duyck <hidden>
Date: 2016-12-23 15:50:23

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

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

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

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

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.

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.

- Alex

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

From: maowenan <hidden>
Date: 2016-12-24 08:34:24

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

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

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

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

wrote:

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

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.

- 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

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

From: maowenan <hidden>
Date: 2016-12-26 08:34:01

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

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

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

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

wrote:

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

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.

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

From: maowenan <hidden>
Date: 2017-01-04 09:02:30

-----Original Message-----
From: maowenan
Sent: Monday, December 26, 2016 4:33 PM
To: maowenan; '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: 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

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

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

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

wrote:

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

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.

quoted

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

quoted

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

quoted

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.

quoted

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?

quoted

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

I have verified DMA_ATTR_WEAK_ORDERING is not usable for our system(arm64 and 82599),
We should enable relax ordering in 82599 DCA control register to improve performance. 
As Stephen Hemminger do not suggest use ethtool to set relax ordering feature, 
@Jeff, do you agree with using erratum config to enable RO mode in 82599.  
Codes like below:
In Kconfig:
+config HI_ERRATUM_xxxx

In ixgbe_82599.c
#if !defined (CONFIG_SPARC) || !defined(HI_ERRATUM_xxxx)
	/* Disable relaxed ordering */
	for (i = 0; ((i < hw->mac.max_tx_queues) &&
	     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
		regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
		regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
		IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
	}

	for (i = 0; ((i < hw->mac.max_rx_queues) &&
	     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
		regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
		regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
			    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
		IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
	}
#endif

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

From: Alexander Duyck <hidden>
Date: 2017-01-04 16:50:15

On Wed, Jan 4, 2017 at 1:02 AM, maowenan [off-list ref] wrote:

quoted

-----Original Message-----
From: maowenan
Sent: Monday, December 26, 2016 4:33 PM
To: maowenan; '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: 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

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

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

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

wrote:

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

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.

quoted

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

quoted

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

quoted

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.

quoted

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?

quoted

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

I have verified DMA_ATTR_WEAK_ORDERING is not usable for our system(arm64 and 82599),
We should enable relax ordering in 82599 DCA control register to improve performance.
As Stephen Hemminger do not suggest use ethtool to set relax ordering feature,
@Jeff, do you agree with using erratum config to enable RO mode in 82599.
Codes like below:
In Kconfig:
+config HI_ERRATUM_xxxx

In ixgbe_82599.c
#if !defined (CONFIG_SPARC) || !defined(HI_ERRATUM_xxxx)
        /* Disable relaxed ordering */
        for (i = 0; ((i < hw->mac.max_tx_queues) &&
             (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
                regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
                regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
                IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
        }

        for (i = 0; ((i < hw->mac.max_rx_queues) &&
             (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
                regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
                regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
                            IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
                IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
        }
#endif

Instead of having the driver add a flag per architecture maybe it
would be worthwhile to look at adding a config flag that can be set
specifically for the architectures that need it.  Maybe something like
a "ARCH_WANT_RO_DMA" that then drivers could modify their relaxed
ordering flag based on.  Then you could just add the flag to the SPARC
and your arm64 kconfig options and not have to involve the other
architectures.

- Alex

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help