[RFC 1/4] net: support per queue tx_usecs in sysfs

[RFC 1/4] net: support per queue tx_usecs in sysfs

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Network devices usually have many queues. Each queue has its own
tx_usecs options. Currently, we can only set all the queues with same
value by ethtool. This patch expose the tx_usecs in sysfs. So the user
can set/get per queue coalesce parameter tx_usecs by sysfs.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 Documentation/networking/scaling.txt | 12 ++++++++++++
 include/linux/netdevice.h            |  8 ++++++++
 net/core/net-sysfs.c                 | 38 ++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
index 59f4db2..636192d 100644
--- a/Documentation/networking/scaling.txt
+++ b/Documentation/networking/scaling.txt
@@ -431,6 +431,18 @@ a max-rate attribute is supported, by setting a Mbps value to
 
 A value of zero means disabled, and this is the default.
 
+Per Queue interrupt moderation:
+=============================
+
+The interrupt moderation mechanism, which implemented by HW, employs
+a series of timers to limit the number of interrupts it generates.
+TX queue absolute delay timer can be set to a microseconds value with
+
+/sys/class/net/<dev>/queues/tx-<n>/tx_usecs
+
+For the device which doesn't support per queue interrupt moderation,
+it shows "N/A".
+
 Further Information
 ===================
 RPS and RFS were introduced in kernel 2.6.35. XPS was incorporated into
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7d2d1d7..9db5c57 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1059,6 +1059,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  *	This function is used to get egress tunnel information for given skb.
  *	This is useful for retrieving outer tunnel header parameters while
  *	sampling packet.
+ * void (*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
+ * 				      int index, u32 val);
+ * void (*ndo_get_per_queue_tx_usecs)(struct net_device *dev, int index);
+ * 	This function is used to set/get per queue coalesce parameter tx_usecs.
  *
  */
 struct net_device_ops {
@@ -1236,6 +1240,10 @@ struct net_device_ops {
 							 bool proto_down);
 	int			(*ndo_fill_metadata_dst)(struct net_device *dev,
 						       struct sk_buff *skb);
+	void			(*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
+							      int index, u32 val);
+	u32			(*ndo_get_per_queue_tx_usecs)(struct net_device *dev,
+							      int index);
 };
 
 /**
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index f88a62a..48016b8 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1239,12 +1239,50 @@ static struct netdev_queue_attribute xps_cpus_attribute =
     __ATTR(xps_cpus, S_IRUGO | S_IWUSR, show_xps_map, store_xps_map);
 #endif /* CONFIG_XPS */
 
+static ssize_t tx_usecs_show(struct netdev_queue *queue,
+			     struct netdev_queue_attribute *attr,
+			     char *buf)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_tx;
+	u32 val;
+
+	if (dev->netdev_ops->ndo_get_per_queue_tx_usecs) {
+		val = dev->netdev_ops->ndo_get_per_queue_tx_usecs(dev, index);
+		return sprintf(buf, "%u\n", val);
+	}
+
+	return sprintf(buf, "N/A\n");
+}
+
+static ssize_t tx_usecs_store(struct netdev_queue *queue,
+			      struct netdev_queue_attribute *attr,
+			      const char *buf, size_t len)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_tx;
+	u32 val, ret;
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret < 0)
+		return -EINVAL;
+
+	if (dev->netdev_ops->ndo_set_per_queue_tx_usecs)
+		dev->netdev_ops->ndo_set_per_queue_tx_usecs(dev, index, val);
+
+	return len;
+}
+
+static struct netdev_queue_attribute tx_usecs_attribute =
+    __ATTR(tx_usecs, S_IRUGO | S_IWUSR, tx_usecs_show, tx_usecs_store);
+
 static struct attribute *netdev_queue_default_attrs[] = {
 	&queue_trans_timeout.attr,
 #ifdef CONFIG_XPS
 	&xps_cpus_attribute.attr,
 	&queue_tx_maxrate.attr,
 #endif
+	&tx_usecs_attribute.attr,
 	NULL
 };
 
-- 
1.7.11.7

[Intel-wired-lan] [RFC 1/4] net: support per queue tx_usecs in sysfs

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Network devices usually have many queues. Each queue has its own
tx_usecs options. Currently, we can only set all the queues with same
value by ethtool. This patch expose the tx_usecs in sysfs. So the user
can set/get per queue coalesce parameter tx_usecs by sysfs.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 Documentation/networking/scaling.txt | 12 ++++++++++++
 include/linux/netdevice.h            |  8 ++++++++
 net/core/net-sysfs.c                 | 38 ++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
index 59f4db2..636192d 100644
--- a/Documentation/networking/scaling.txt
+++ b/Documentation/networking/scaling.txt
@@ -431,6 +431,18 @@ a max-rate attribute is supported, by setting a Mbps value to
 
 A value of zero means disabled, and this is the default.
 
+Per Queue interrupt moderation:
+=============================
+
+The interrupt moderation mechanism, which implemented by HW, employs
+a series of timers to limit the number of interrupts it generates.
+TX queue absolute delay timer can be set to a microseconds value with
+
+/sys/class/net/<dev>/queues/tx-<n>/tx_usecs
+
+For the device which doesn't support per queue interrupt moderation,
+it shows "N/A".
+
 Further Information
 ===================
 RPS and RFS were introduced in kernel 2.6.35. XPS was incorporated into
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7d2d1d7..9db5c57 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1059,6 +1059,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  *	This function is used to get egress tunnel information for given skb.
  *	This is useful for retrieving outer tunnel header parameters while
  *	sampling packet.
+ * void (*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
+ * 				      int index, u32 val);
+ * void (*ndo_get_per_queue_tx_usecs)(struct net_device *dev, int index);
+ * 	This function is used to set/get per queue coalesce parameter tx_usecs.
  *
  */
 struct net_device_ops {
@@ -1236,6 +1240,10 @@ struct net_device_ops {
 							 bool proto_down);
 	int			(*ndo_fill_metadata_dst)(struct net_device *dev,
 						       struct sk_buff *skb);
+	void			(*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
+							      int index, u32 val);
+	u32			(*ndo_get_per_queue_tx_usecs)(struct net_device *dev,
+							      int index);
 };
 
 /**
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index f88a62a..48016b8 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1239,12 +1239,50 @@ static struct netdev_queue_attribute xps_cpus_attribute =
     __ATTR(xps_cpus, S_IRUGO | S_IWUSR, show_xps_map, store_xps_map);
 #endif /* CONFIG_XPS */
 
+static ssize_t tx_usecs_show(struct netdev_queue *queue,
+			     struct netdev_queue_attribute *attr,
+			     char *buf)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_tx;
+	u32 val;
+
+	if (dev->netdev_ops->ndo_get_per_queue_tx_usecs) {
+		val = dev->netdev_ops->ndo_get_per_queue_tx_usecs(dev, index);
+		return sprintf(buf, "%u\n", val);
+	}
+
+	return sprintf(buf, "N/A\n");
+}
+
+static ssize_t tx_usecs_store(struct netdev_queue *queue,
+			      struct netdev_queue_attribute *attr,
+			      const char *buf, size_t len)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_tx;
+	u32 val, ret;
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret < 0)
+		return -EINVAL;
+
+	if (dev->netdev_ops->ndo_set_per_queue_tx_usecs)
+		dev->netdev_ops->ndo_set_per_queue_tx_usecs(dev, index, val);
+
+	return len;
+}
+
+static struct netdev_queue_attribute tx_usecs_attribute =
+    __ATTR(tx_usecs, S_IRUGO | S_IWUSR, tx_usecs_show, tx_usecs_store);
+
 static struct attribute *netdev_queue_default_attrs[] = {
 	&queue_trans_timeout.attr,
 #ifdef CONFIG_XPS
 	&xps_cpus_attribute.attr,
 	&queue_tx_maxrate.attr,
 #endif
+	&tx_usecs_attribute.attr,
 	NULL
 };
 
-- 
1.7.11.7

[RFC 2/4] i40e: handle per queue tx_usecs setting

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Handle ndo_get_per_queue_tx_usecs and ndo_set_per_queue_tx_usecs
options for i40e driver specifically.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 4b7d874..d4310ae 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8673,6 +8673,43 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				       nlflags, 0, 0, filter_mask, NULL);
 }
 
+/**
+ * i40e_ndo_get_per_queue_tx_usecs - Get per queue coalesce parameter tx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ *
+ * Return tx_usecs for specific channel
+ **/
+u32 i40e_ndo_get_per_queue_tx_usecs(struct net_device *dev, int index)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+	u32 val;
+
+	val = rd32(hw, I40E_PFINT_ITRN(1, index));
+
+	return (val * 2);
+}
+
+/**
+ * i40e_ndo_set_per_queue_tx_usecs - Set per queue coalesce parameter tx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ * @val: tx_usecs value
+ **/
+void i40e_ndo_set_per_queue_tx_usecs(struct net_device *dev, int index, u32 val)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+
+	wr32(hw, I40E_PFINT_ITRN(1, index), val / 2);
+	i40e_flush(hw);
+}
+
 #define I40E_MAX_TUNNEL_HDR_LEN 80
 /**
  * i40e_features_check - Validate encapsulated packet conforms to limits
@@ -8729,6 +8766,8 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_features_check	= i40e_features_check,
 	.ndo_bridge_getlink	= i40e_ndo_bridge_getlink,
 	.ndo_bridge_setlink	= i40e_ndo_bridge_setlink,
+	.ndo_get_per_queue_tx_usecs	= i40e_ndo_get_per_queue_tx_usecs,
+	.ndo_set_per_queue_tx_usecs	= i40e_ndo_set_per_queue_tx_usecs,
 };
 
 /**
-- 
1.7.11.7

[Intel-wired-lan] [RFC 2/4] i40e: handle per queue tx_usecs setting

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Handle ndo_get_per_queue_tx_usecs and ndo_set_per_queue_tx_usecs
options for i40e driver specifically.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 4b7d874..d4310ae 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8673,6 +8673,43 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				       nlflags, 0, 0, filter_mask, NULL);
 }
 
+/**
+ * i40e_ndo_get_per_queue_tx_usecs - Get per queue coalesce parameter tx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ *
+ * Return tx_usecs for specific channel
+ **/
+u32 i40e_ndo_get_per_queue_tx_usecs(struct net_device *dev, int index)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+	u32 val;
+
+	val = rd32(hw, I40E_PFINT_ITRN(1, index));
+
+	return (val * 2);
+}
+
+/**
+ * i40e_ndo_set_per_queue_tx_usecs - Set per queue coalesce parameter tx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ * @val: tx_usecs value
+ **/
+void i40e_ndo_set_per_queue_tx_usecs(struct net_device *dev, int index, u32 val)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+
+	wr32(hw, I40E_PFINT_ITRN(1, index), val / 2);
+	i40e_flush(hw);
+}
+
 #define I40E_MAX_TUNNEL_HDR_LEN 80
 /**
  * i40e_features_check - Validate encapsulated packet conforms to limits
@@ -8729,6 +8766,8 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_features_check	= i40e_features_check,
 	.ndo_bridge_getlink	= i40e_ndo_bridge_getlink,
 	.ndo_bridge_setlink	= i40e_ndo_bridge_setlink,
+	.ndo_get_per_queue_tx_usecs	= i40e_ndo_get_per_queue_tx_usecs,
+	.ndo_set_per_queue_tx_usecs	= i40e_ndo_set_per_queue_tx_usecs,
 };
 
 /**
-- 
1.7.11.7

[RFC 3/4] net: support per queue rx_usecs in sysfs

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Network devices usually have many queues. Each queue has its own
rx_usecs options. Currently, we can only set all the queues with same
value by ethtool. This patch expose the rx_usecs in sysfs. So the user
can set/get per queue coalesce parameter rx_usecs by sysfs.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 Documentation/networking/scaling.txt |  4 ++++
 include/linux/netdevice.h            |  8 ++++++++
 net/core/net-sysfs.c                 | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
index 636192d..218429c 100644
--- a/Documentation/networking/scaling.txt
+++ b/Documentation/networking/scaling.txt
@@ -440,6 +440,10 @@ TX queue absolute delay timer can be set to a microseconds value with
 
 /sys/class/net/<dev>/queues/tx-<n>/tx_usecs
 
+RX queue absolute delay timer can be set to a microseconds value with
+
+/sys/class/net/<dev>/queues/rx-<n>/rx_usecs
+
 For the device which doesn't support per queue interrupt moderation,
 it shows "N/A".
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9db5c57..45a0054 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1063,6 +1063,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  * 				      int index, u32 val);
  * void (*ndo_get_per_queue_tx_usecs)(struct net_device *dev, int index);
  * 	This function is used to set/get per queue coalesce parameter tx_usecs.
+ * void (*ndo_set_per_queue_rx_usecs)(struct net_device *dev,
+ * 				      int index, u32 val);
+ * void (*ndo_get_per_queue_rx_usecs)(struct net_device *dev, int index);
+ * 	This function is used to set/get per queue coalesce parameter rx_usecs.
  *
  */
 struct net_device_ops {
@@ -1244,6 +1248,10 @@ struct net_device_ops {
 							      int index, u32 val);
 	u32			(*ndo_get_per_queue_tx_usecs)(struct net_device *dev,
 							      int index);
+	void			(*ndo_set_per_queue_rx_usecs)(struct net_device *dev,
+							      int index, u32 val);
+	u32			(*ndo_get_per_queue_rx_usecs)(struct net_device *dev,
+							      int index);
 };
 
 /**
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 48016b8..ab08b2b 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -831,11 +831,47 @@ static struct rx_queue_attribute rps_dev_flow_table_cnt_attribute =
 	    show_rps_dev_flow_table_cnt, store_rps_dev_flow_table_cnt);
 #endif /* CONFIG_RPS */
 
+static ssize_t rx_usecs_show(struct netdev_rx_queue *queue,
+			     struct rx_queue_attribute *attribute, char *buf)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_rx;
+	u32 val;
+
+	if (dev->netdev_ops->ndo_get_per_queue_rx_usecs) {
+		val = dev->netdev_ops->ndo_get_per_queue_rx_usecs(dev, index);
+		return sprintf(buf, "%u\n", val);
+	}
+	return sprintf(buf, "N/A\n");
+}
+
+static ssize_t rx_usecs_store(struct netdev_rx_queue *queue,
+			      struct rx_queue_attribute *attribute,
+			      const char *buf, size_t len)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_rx;
+	u32 val, ret;
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret < 0)
+		return -EINVAL;
+
+	if (dev->netdev_ops->ndo_set_per_queue_rx_usecs)
+		dev->netdev_ops->ndo_set_per_queue_rx_usecs(dev, index, val);
+
+	return len;
+}
+
+static struct rx_queue_attribute rx_usecs_attribute =
+	__ATTR(rx_usecs, S_IRUGO | S_IWUSR, rx_usecs_show, rx_usecs_store);
+
 static struct attribute *rx_queue_default_attrs[] = {
 #ifdef CONFIG_RPS
 	&rps_cpus_attribute.attr,
 	&rps_dev_flow_table_cnt_attribute.attr,
 #endif
+	&rx_usecs_attribute.attr,
 	NULL
 };
 
-- 
1.7.11.7

[Intel-wired-lan] [RFC 3/4] net: support per queue rx_usecs in sysfs

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Network devices usually have many queues. Each queue has its own
rx_usecs options. Currently, we can only set all the queues with same
value by ethtool. This patch expose the rx_usecs in sysfs. So the user
can set/get per queue coalesce parameter rx_usecs by sysfs.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 Documentation/networking/scaling.txt |  4 ++++
 include/linux/netdevice.h            |  8 ++++++++
 net/core/net-sysfs.c                 | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
index 636192d..218429c 100644
--- a/Documentation/networking/scaling.txt
+++ b/Documentation/networking/scaling.txt
@@ -440,6 +440,10 @@ TX queue absolute delay timer can be set to a microseconds value with
 
 /sys/class/net/<dev>/queues/tx-<n>/tx_usecs
 
+RX queue absolute delay timer can be set to a microseconds value with
+
+/sys/class/net/<dev>/queues/rx-<n>/rx_usecs
+
 For the device which doesn't support per queue interrupt moderation,
 it shows "N/A".
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9db5c57..45a0054 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1063,6 +1063,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  * 				      int index, u32 val);
  * void (*ndo_get_per_queue_tx_usecs)(struct net_device *dev, int index);
  * 	This function is used to set/get per queue coalesce parameter tx_usecs.
+ * void (*ndo_set_per_queue_rx_usecs)(struct net_device *dev,
+ * 				      int index, u32 val);
+ * void (*ndo_get_per_queue_rx_usecs)(struct net_device *dev, int index);
+ * 	This function is used to set/get per queue coalesce parameter rx_usecs.
  *
  */
 struct net_device_ops {
@@ -1244,6 +1248,10 @@ struct net_device_ops {
 							      int index, u32 val);
 	u32			(*ndo_get_per_queue_tx_usecs)(struct net_device *dev,
 							      int index);
+	void			(*ndo_set_per_queue_rx_usecs)(struct net_device *dev,
+							      int index, u32 val);
+	u32			(*ndo_get_per_queue_rx_usecs)(struct net_device *dev,
+							      int index);
 };
 
 /**
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 48016b8..ab08b2b 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -831,11 +831,47 @@ static struct rx_queue_attribute rps_dev_flow_table_cnt_attribute =
 	    show_rps_dev_flow_table_cnt, store_rps_dev_flow_table_cnt);
 #endif /* CONFIG_RPS */
 
+static ssize_t rx_usecs_show(struct netdev_rx_queue *queue,
+			     struct rx_queue_attribute *attribute, char *buf)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_rx;
+	u32 val;
+
+	if (dev->netdev_ops->ndo_get_per_queue_rx_usecs) {
+		val = dev->netdev_ops->ndo_get_per_queue_rx_usecs(dev, index);
+		return sprintf(buf, "%u\n", val);
+	}
+	return sprintf(buf, "N/A\n");
+}
+
+static ssize_t rx_usecs_store(struct netdev_rx_queue *queue,
+			      struct rx_queue_attribute *attribute,
+			      const char *buf, size_t len)
+{
+	struct net_device *dev = queue->dev;
+	int index = queue - dev->_rx;
+	u32 val, ret;
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret < 0)
+		return -EINVAL;
+
+	if (dev->netdev_ops->ndo_set_per_queue_rx_usecs)
+		dev->netdev_ops->ndo_set_per_queue_rx_usecs(dev, index, val);
+
+	return len;
+}
+
+static struct rx_queue_attribute rx_usecs_attribute =
+	__ATTR(rx_usecs, S_IRUGO | S_IWUSR, rx_usecs_show, rx_usecs_store);
+
 static struct attribute *rx_queue_default_attrs[] = {
 #ifdef CONFIG_RPS
 	&rps_cpus_attribute.attr,
 	&rps_dev_flow_table_cnt_attribute.attr,
 #endif
+	&rx_usecs_attribute.attr,
 	NULL
 };
 
-- 
1.7.11.7

[RFC 4/4] i40e: handle per queue rx_usecs setting

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Handle ndo_get_per_queue_rx_usecs and ndo_set_per_queue_rx_usecs
options for i40e driver specifically.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d4310ae..c0291bb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8710,6 +8710,43 @@ void i40e_ndo_set_per_queue_tx_usecs(struct net_device *dev, int index, u32 val)
 	i40e_flush(hw);
 }
 
+/**
+ * i40e_ndo_get_per_queue_rx_usecs - Get per queue coalesce parameter rx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ *
+ * Return rx_usecs for specific queue
+ **/
+u32 i40e_ndo_get_per_queue_rx_usecs(struct net_device *dev, int index)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+	u32 val;
+
+	val = rd32(hw, I40E_PFINT_ITRN(0, index));
+
+	return (val * 2);
+}
+
+/**
+ * i40e_ndo_set_per_queue_rx_usecs - Set per queue coalesce parameter rx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ * @val: rx_usecs value
+ **/
+void i40e_ndo_set_per_queue_rx_usecs(struct net_device *dev, int index, u32 val)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+
+	wr32(hw, I40E_PFINT_ITRN(0, index), val / 2);
+	i40e_flush(hw);
+}
+
 #define I40E_MAX_TUNNEL_HDR_LEN 80
 /**
  * i40e_features_check - Validate encapsulated packet conforms to limits
@@ -8768,6 +8805,8 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_bridge_setlink	= i40e_ndo_bridge_setlink,
 	.ndo_get_per_queue_tx_usecs	= i40e_ndo_get_per_queue_tx_usecs,
 	.ndo_set_per_queue_tx_usecs	= i40e_ndo_set_per_queue_tx_usecs,
+	.ndo_get_per_queue_rx_usecs	= i40e_ndo_get_per_queue_rx_usecs,
+	.ndo_set_per_queue_rx_usecs	= i40e_ndo_set_per_queue_rx_usecs,
 };
 
 /**
-- 
1.7.11.7

[Intel-wired-lan] [RFC 4/4] i40e: handle per queue rx_usecs setting

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Handle ndo_get_per_queue_rx_usecs and ndo_set_per_queue_rx_usecs
options for i40e driver specifically.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d4310ae..c0291bb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8710,6 +8710,43 @@ void i40e_ndo_set_per_queue_tx_usecs(struct net_device *dev, int index, u32 val)
 	i40e_flush(hw);
 }
 
+/**
+ * i40e_ndo_get_per_queue_rx_usecs - Get per queue coalesce parameter rx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ *
+ * Return rx_usecs for specific queue
+ **/
+u32 i40e_ndo_get_per_queue_rx_usecs(struct net_device *dev, int index)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+	u32 val;
+
+	val = rd32(hw, I40E_PFINT_ITRN(0, index));
+
+	return (val * 2);
+}
+
+/**
+ * i40e_ndo_set_per_queue_rx_usecs - Set per queue coalesce parameter rx_usecs
+ * @dev: the netdev being configured
+ * @index: queue index
+ * @val: rx_usecs value
+ **/
+void i40e_ndo_set_per_queue_rx_usecs(struct net_device *dev, int index, u32 val)
+{
+	struct i40e_netdev_priv *np = netdev_priv(dev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	struct i40e_hw *hw = &pf->hw;
+
+	wr32(hw, I40E_PFINT_ITRN(0, index), val / 2);
+	i40e_flush(hw);
+}
+
 #define I40E_MAX_TUNNEL_HDR_LEN 80
 /**
  * i40e_features_check - Validate encapsulated packet conforms to limits
@@ -8768,6 +8805,8 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_bridge_setlink	= i40e_ndo_bridge_setlink,
 	.ndo_get_per_queue_tx_usecs	= i40e_ndo_get_per_queue_tx_usecs,
 	.ndo_set_per_queue_tx_usecs	= i40e_ndo_set_per_queue_tx_usecs,
+	.ndo_get_per_queue_rx_usecs	= i40e_ndo_get_per_queue_rx_usecs,
+	.ndo_set_per_queue_rx_usecs	= i40e_ndo_set_per_queue_rx_usecs,
 };
 
 /**
-- 
1.7.11.7

Re: [RFC 4/4] i40e: handle per queue rx_usecs setting

[Stephen Hemminger]

On Tue,  1 Dec 2015 08:01:32 +0000
kan.liang@intel.com wrote:

> +u32 i40e_ndo_get_per_queue_rx_usecs(struct net_device *dev, int index)
> +{
> +	struct i40e_netdev_priv *np = netdev_priv(dev);
> +	struct i40e_vsi *vsi = np->vsi;
> +	struct i40e_pf *pf = vsi->back;
> +	struct i40e_hw *hw = &pf->hw;
> +	u32 val;
> +
> +	val = rd32(hw, I40E_PFINT_ITRN(0, index));
> +
> +	return (val * 2);

Please don't put useless paren's around return value.
That is a BSD style that is not practiced in Linux.

[Intel-wired-lan] [RFC 4/4] i40e: handle per queue rx_usecs setting

[Stephen Hemminger]

On Tue,  1 Dec 2015 08:01:32 +0000
kan.liang at intel.com wrote:

> +u32 i40e_ndo_get_per_queue_rx_usecs(struct net_device *dev, int index)
> +{
> +	struct i40e_netdev_priv *np = netdev_priv(dev);
> +	struct i40e_vsi *vsi = np->vsi;
> +	struct i40e_pf *pf = vsi->back;
> +	struct i40e_hw *hw = &pf->hw;
> +	u32 val;
> +
> +	val = rd32(hw, I40E_PFINT_ITRN(0, index));
> +
> +	return (val * 2);

Please don't put useless paren's around return value.
That is a BSD style that is not practiced in Linux.

Re: [RFC 1/4] net: support per queue tx_usecs in sysfs

[Florian Fainelli]

On 01/12/15 00:01, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> Network devices usually have many queues. Each queue has its own
> tx_usecs options. Currently, we can only set all the queues with same
> value by ethtool. This patch expose the tx_usecs in sysfs. So the user
> can set/get per queue coalesce parameter tx_usecs by sysfs.

The new interface you propose makes things inconsistent, since we have
two separate configuration paths (sysfs and ethtool), and it would seem
better to have per-queue awareness in ethtool, since there is a whole
bunch of other parameters that could be configured on a per-queue basis.

Have you tried to extend existing ethtool interfaces to cover the need
for multiple queues?

> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
>  Documentation/networking/scaling.txt | 12 ++++++++++++
>  include/linux/netdevice.h            |  8 ++++++++
>  net/core/net-sysfs.c                 | 38 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+)
> 
> diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
> index 59f4db2..636192d 100644
> --- a/Documentation/networking/scaling.txt
> +++ b/Documentation/networking/scaling.txt
> @@ -431,6 +431,18 @@ a max-rate attribute is supported, by setting a Mbps value to
>  
>  A value of zero means disabled, and this is the default.
>  
> +Per Queue interrupt moderation:
> +=============================
> +
> +The interrupt moderation mechanism, which implemented by HW, employs
> +a series of timers to limit the number of interrupts it generates.
> +TX queue absolute delay timer can be set to a microseconds value with
> +
> +/sys/class/net/<dev>/queues/tx-<n>/tx_usecs
> +
> +For the device which doesn't support per queue interrupt moderation,
> +it shows "N/A".
> +
>  Further Information
>  ===================
>  RPS and RFS were introduced in kernel 2.6.35. XPS was incorporated into
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 7d2d1d7..9db5c57 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1059,6 +1059,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>   *	This function is used to get egress tunnel information for given skb.
>   *	This is useful for retrieving outer tunnel header parameters while
>   *	sampling packet.
> + * void (*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
> + * 				      int index, u32 val);
> + * void (*ndo_get_per_queue_tx_usecs)(struct net_device *dev, int index);
> + * 	This function is used to set/get per queue coalesce parameter tx_usecs.
>   *
>   */
>  struct net_device_ops {
> @@ -1236,6 +1240,10 @@ struct net_device_ops {
>  							 bool proto_down);
>  	int			(*ndo_fill_metadata_dst)(struct net_device *dev,
>  						       struct sk_buff *skb);
> +	void			(*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
> +							      int index, u32 val);
> +	u32			(*ndo_get_per_queue_tx_usecs)(struct net_device *dev,
> +							      int index);
>  };
>  
>  /**
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index f88a62a..48016b8 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -1239,12 +1239,50 @@ static struct netdev_queue_attribute xps_cpus_attribute =
>      __ATTR(xps_cpus, S_IRUGO | S_IWUSR, show_xps_map, store_xps_map);
>  #endif /* CONFIG_XPS */
>  
> +static ssize_t tx_usecs_show(struct netdev_queue *queue,
> +			     struct netdev_queue_attribute *attr,
> +			     char *buf)
> +{
> +	struct net_device *dev = queue->dev;
> +	int index = queue - dev->_tx;
> +	u32 val;
> +
> +	if (dev->netdev_ops->ndo_get_per_queue_tx_usecs) {
> +		val = dev->netdev_ops->ndo_get_per_queue_tx_usecs(dev, index);
> +		return sprintf(buf, "%u\n", val);
> +	}
> +
> +	return sprintf(buf, "N/A\n");
> +}
> +
> +static ssize_t tx_usecs_store(struct netdev_queue *queue,
> +			      struct netdev_queue_attribute *attr,
> +			      const char *buf, size_t len)
> +{
> +	struct net_device *dev = queue->dev;
> +	int index = queue - dev->_tx;
> +	u32 val, ret;
> +
> +	ret = kstrtouint(buf, 0, &val);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	if (dev->netdev_ops->ndo_set_per_queue_tx_usecs)
> +		dev->netdev_ops->ndo_set_per_queue_tx_usecs(dev, index, val);
> +
> +	return len;
> +}
> +
> +static struct netdev_queue_attribute tx_usecs_attribute =
> +    __ATTR(tx_usecs, S_IRUGO | S_IWUSR, tx_usecs_show, tx_usecs_store);
> +
>  static struct attribute *netdev_queue_default_attrs[] = {
>  	&queue_trans_timeout.attr,
>  #ifdef CONFIG_XPS
>  	&xps_cpus_attribute.attr,
>  	&queue_tx_maxrate.attr,
>  #endif
> +	&tx_usecs_attribute.attr,
>  	NULL
>  };
>  
> 


-- 
Florian

[Intel-wired-lan] [RFC 1/4] net: support per queue tx_usecs in sysfs

[Florian Fainelli]

On 01/12/15 00:01, kan.liang at intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> Network devices usually have many queues. Each queue has its own
> tx_usecs options. Currently, we can only set all the queues with same
> value by ethtool. This patch expose the tx_usecs in sysfs. So the user
> can set/get per queue coalesce parameter tx_usecs by sysfs.

The new interface you propose makes things inconsistent, since we have
two separate configuration paths (sysfs and ethtool), and it would seem
better to have per-queue awareness in ethtool, since there is a whole
bunch of other parameters that could be configured on a per-queue basis.

Have you tried to extend existing ethtool interfaces to cover the need
for multiple queues?

> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
>  Documentation/networking/scaling.txt | 12 ++++++++++++
>  include/linux/netdevice.h            |  8 ++++++++
>  net/core/net-sysfs.c                 | 38 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+)
> 
> diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
> index 59f4db2..636192d 100644
> --- a/Documentation/networking/scaling.txt
> +++ b/Documentation/networking/scaling.txt
> @@ -431,6 +431,18 @@ a max-rate attribute is supported, by setting a Mbps value to
>  
>  A value of zero means disabled, and this is the default.
>  
> +Per Queue interrupt moderation:
> +=============================
> +
> +The interrupt moderation mechanism, which implemented by HW, employs
> +a series of timers to limit the number of interrupts it generates.
> +TX queue absolute delay timer can be set to a microseconds value with
> +
> +/sys/class/net/<dev>/queues/tx-<n>/tx_usecs
> +
> +For the device which doesn't support per queue interrupt moderation,
> +it shows "N/A".
> +
>  Further Information
>  ===================
>  RPS and RFS were introduced in kernel 2.6.35. XPS was incorporated into
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 7d2d1d7..9db5c57 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1059,6 +1059,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>   *	This function is used to get egress tunnel information for given skb.
>   *	This is useful for retrieving outer tunnel header parameters while
>   *	sampling packet.
> + * void (*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
> + * 				      int index, u32 val);
> + * void (*ndo_get_per_queue_tx_usecs)(struct net_device *dev, int index);
> + * 	This function is used to set/get per queue coalesce parameter tx_usecs.
>   *
>   */
>  struct net_device_ops {
> @@ -1236,6 +1240,10 @@ struct net_device_ops {
>  							 bool proto_down);
>  	int			(*ndo_fill_metadata_dst)(struct net_device *dev,
>  						       struct sk_buff *skb);
> +	void			(*ndo_set_per_queue_tx_usecs)(struct net_device *dev,
> +							      int index, u32 val);
> +	u32			(*ndo_get_per_queue_tx_usecs)(struct net_device *dev,
> +							      int index);
>  };
>  
>  /**
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index f88a62a..48016b8 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -1239,12 +1239,50 @@ static struct netdev_queue_attribute xps_cpus_attribute =
>      __ATTR(xps_cpus, S_IRUGO | S_IWUSR, show_xps_map, store_xps_map);
>  #endif /* CONFIG_XPS */
>  
> +static ssize_t tx_usecs_show(struct netdev_queue *queue,
> +			     struct netdev_queue_attribute *attr,
> +			     char *buf)
> +{
> +	struct net_device *dev = queue->dev;
> +	int index = queue - dev->_tx;
> +	u32 val;
> +
> +	if (dev->netdev_ops->ndo_get_per_queue_tx_usecs) {
> +		val = dev->netdev_ops->ndo_get_per_queue_tx_usecs(dev, index);
> +		return sprintf(buf, "%u\n", val);
> +	}
> +
> +	return sprintf(buf, "N/A\n");
> +}
> +
> +static ssize_t tx_usecs_store(struct netdev_queue *queue,
> +			      struct netdev_queue_attribute *attr,
> +			      const char *buf, size_t len)
> +{
> +	struct net_device *dev = queue->dev;
> +	int index = queue - dev->_tx;
> +	u32 val, ret;
> +
> +	ret = kstrtouint(buf, 0, &val);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	if (dev->netdev_ops->ndo_set_per_queue_tx_usecs)
> +		dev->netdev_ops->ndo_set_per_queue_tx_usecs(dev, index, val);
> +
> +	return len;
> +}
> +
> +static struct netdev_queue_attribute tx_usecs_attribute =
> +    __ATTR(tx_usecs, S_IRUGO | S_IWUSR, tx_usecs_show, tx_usecs_store);
> +
>  static struct attribute *netdev_queue_default_attrs[] = {
>  	&queue_trans_timeout.attr,
>  #ifdef CONFIG_XPS
>  	&xps_cpus_attribute.attr,
>  	&queue_tx_maxrate.attr,
>  #endif
> +	&tx_usecs_attribute.attr,
>  	NULL
>  };
>  
> 


-- 
Florian

Re: [RFC 1/4] net: support per queue tx_usecs in sysfs

[Jesse Brandeburg]

On Tue, 1 Dec 2015 14:13:34 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 01/12/15 00:01, kan.liang@intel.com wrote:
> > From: Kan Liang <kan.liang@intel.com>
> > 
> > Network devices usually have many queues. Each queue has its own
> > tx_usecs options. Currently, we can only set all the queues with same
> > value by ethtool. This patch expose the tx_usecs in sysfs. So the user
> > can set/get per queue coalesce parameter tx_usecs by sysfs.
> 
> The new interface you propose makes things inconsistent, since we have
> two separate configuration paths (sysfs and ethtool), and it would seem
> better to have per-queue awareness in ethtool, since there is a whole
> bunch of other parameters that could be configured on a per-queue basis.
> 
> Have you tried to extend existing ethtool interfaces to cover the need
> for multiple queues?

While I agree that ethtool provides a similar functionality, ethtool
was designed (particularly the ethtool -C/c commands) around one queue
NICs.  We can't change the output or functionality of the user
interface without breaking a bunch of user's scripts and stuff.

With this effort, Kan is laying groundwork for making further kernel
changes, and having the kernel call back in to drivers via ethtool
mechanisms that were designed before multiple queue adapters.

We can also next migrate the legacy ethtool interfaces to use these
new .ndo_ops should we wish.

These patches were provided with the intent of getting some feedback
about going down this path of making a *consistent* user interface that
is driver agnostic in sysfs, and supports multiple queue adapters.

[Intel-wired-lan] [RFC 1/4] net: support per queue tx_usecs in sysfs

[Jesse Brandeburg]

On Tue, 1 Dec 2015 14:13:34 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 01/12/15 00:01, kan.liang at intel.com wrote:
> > From: Kan Liang <kan.liang@intel.com>
> > 
> > Network devices usually have many queues. Each queue has its own
> > tx_usecs options. Currently, we can only set all the queues with same
> > value by ethtool. This patch expose the tx_usecs in sysfs. So the user
> > can set/get per queue coalesce parameter tx_usecs by sysfs.
> 
> The new interface you propose makes things inconsistent, since we have
> two separate configuration paths (sysfs and ethtool), and it would seem
> better to have per-queue awareness in ethtool, since there is a whole
> bunch of other parameters that could be configured on a per-queue basis.
> 
> Have you tried to extend existing ethtool interfaces to cover the need
> for multiple queues?

While I agree that ethtool provides a similar functionality, ethtool
was designed (particularly the ethtool -C/c commands) around one queue
NICs.  We can't change the output or functionality of the user
interface without breaking a bunch of user's scripts and stuff.

With this effort, Kan is laying groundwork for making further kernel
changes, and having the kernel call back in to drivers via ethtool
mechanisms that were designed before multiple queue adapters.

We can also next migrate the legacy ethtool interfaces to use these
new .ndo_ops should we wish.

These patches were provided with the intent of getting some feedback
about going down this path of making a *consistent* user interface that
is driver agnostic in sysfs, and supports multiple queue adapters.

Re: [RFC 1/4] net: support per queue tx_usecs in sysfs

[Alexander Duyck]

On Tue, Dec 1, 2015 at 3:44 PM, Jesse Brandeburg
<jesse.brandeburg@intel.com> wrote:
> On Tue, 1 Dec 2015 14:13:34 -0800
> Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>> On 01/12/15 00:01, kan.liang@intel.com wrote:
>> > From: Kan Liang <kan.liang@intel.com>
>> >
>> > Network devices usually have many queues. Each queue has its own
>> > tx_usecs options. Currently, we can only set all the queues with same
>> > value by ethtool. This patch expose the tx_usecs in sysfs. So the user
>> > can set/get per queue coalesce parameter tx_usecs by sysfs.
>>
>> The new interface you propose makes things inconsistent, since we have
>> two separate configuration paths (sysfs and ethtool), and it would seem
>> better to have per-queue awareness in ethtool, since there is a whole
>> bunch of other parameters that could be configured on a per-queue basis.
>>
>> Have you tried to extend existing ethtool interfaces to cover the need
>> for multiple queues?
>
> While I agree that ethtool provides a similar functionality, ethtool
> was designed (particularly the ethtool -C/c commands) around one queue
> NICs.  We can't change the output or functionality of the user
> interface without breaking a bunch of user's scripts and stuff.

Actually it seems like it should be fairly easy to extend the existing
interface.  If for example you were to add a couple new keywords such
as rx-queue or tx-queue what you could do is make it so that you would
then specifically overwrite or access the values of a given Tx queue
or Rx queue instead of doing it generically for the entire device.

> With this effort, Kan is laying groundwork for making further kernel
> changes, and having the kernel call back in to drivers via ethtool
> mechanisms that were designed before multiple queue adapters.
>
> We can also next migrate the legacy ethtool interfaces to use these
> new .ndo_ops should we wish.

Maybe that is the path this should start taking now.  Another thing to
keep in mind is that not all drivers make use of the rx-usecs value
the way the Intel drivers do.  As such we need to be able to still
support all the various interrupt moderation types that are supported
by any NICs that might make use of this feature.

> These patches were provided with the intent of getting some feedback
> about going down this path of making a *consistent* user interface that
> is driver agnostic in sysfs, and supports multiple queue adapters.

If you are truly going for something that is driver agnostic you need
to start looking at other drivers.  For example I don't see how this
current implementation deals with the tx/rx_frames values provided in
the mlx4 drivers for their interrupt moderation.  Also it seems like
the assumption here is that you are going to want to run all of the
queues statically without allowing dynamic interrupt moderation.  I
would think that this might be something that could be managed per
queue as well.

[Intel-wired-lan] [RFC 1/4] net: support per queue tx_usecs in sysfs

[Alexander Duyck]

On Tue, Dec 1, 2015 at 3:44 PM, Jesse Brandeburg
<jesse.brandeburg@intel.com> wrote:
> On Tue, 1 Dec 2015 14:13:34 -0800
> Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>> On 01/12/15 00:01, kan.liang at intel.com wrote:
>> > From: Kan Liang <kan.liang@intel.com>
>> >
>> > Network devices usually have many queues. Each queue has its own
>> > tx_usecs options. Currently, we can only set all the queues with same
>> > value by ethtool. This patch expose the tx_usecs in sysfs. So the user
>> > can set/get per queue coalesce parameter tx_usecs by sysfs.
>>
>> The new interface you propose makes things inconsistent, since we have
>> two separate configuration paths (sysfs and ethtool), and it would seem
>> better to have per-queue awareness in ethtool, since there is a whole
>> bunch of other parameters that could be configured on a per-queue basis.
>>
>> Have you tried to extend existing ethtool interfaces to cover the need
>> for multiple queues?
>
> While I agree that ethtool provides a similar functionality, ethtool
> was designed (particularly the ethtool -C/c commands) around one queue
> NICs.  We can't change the output or functionality of the user
> interface without breaking a bunch of user's scripts and stuff.

Actually it seems like it should be fairly easy to extend the existing
interface.  If for example you were to add a couple new keywords such
as rx-queue or tx-queue what you could do is make it so that you would
then specifically overwrite or access the values of a given Tx queue
or Rx queue instead of doing it generically for the entire device.

> With this effort, Kan is laying groundwork for making further kernel
> changes, and having the kernel call back in to drivers via ethtool
> mechanisms that were designed before multiple queue adapters.
>
> We can also next migrate the legacy ethtool interfaces to use these
> new .ndo_ops should we wish.

Maybe that is the path this should start taking now.  Another thing to
keep in mind is that not all drivers make use of the rx-usecs value
the way the Intel drivers do.  As such we need to be able to still
support all the various interrupt moderation types that are supported
by any NICs that might make use of this feature.

> These patches were provided with the intent of getting some feedback
> about going down this path of making a *consistent* user interface that
> is driver agnostic in sysfs, and supports multiple queue adapters.

If you are truly going for something that is driver agnostic you need
to start looking at other drivers.  For example I don't see how this
current implementation deals with the tx/rx_frames values provided in
the mlx4 drivers for their interrupt moderation.  Also it seems like
the assumption here is that you are going to want to run all of the
queues statically without allowing dynamic interrupt moderation.  I
would think that this might be something that could be managed per
queue as well.

Re: [RFC 1/4] net: support per queue tx_usecs in sysfs

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 01 Dec 2015 14:13:34 -0800

> The new interface you propose makes things inconsistent, since we have
> two separate configuration paths (sysfs and ethtool), and it would seem
> better to have per-queue awareness in ethtool, since there is a whole
> bunch of other parameters that could be configured on a per-queue basis.

Agreed, we have to extend ethtool to support this, in order to provide
a consistent interface.

We could even do this by encapsulating one ethtool command within
another, the outer ethtool command says "apply the inner op to
queue(s) N".

[Intel-wired-lan] [RFC 1/4] net: support per queue tx_usecs in sysfs

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 01 Dec 2015 14:13:34 -0800

> The new interface you propose makes things inconsistent, since we have
> two separate configuration paths (sysfs and ethtool), and it would seem
> better to have per-queue awareness in ethtool, since there is a whole
> bunch of other parameters that could be configured on a per-queue basis.

Agreed, we have to extend ethtool to support this, in order to provide
a consistent interface.

We could even do this by encapsulating one ethtool command within
another, the outer ethtool command says "apply the inner op to
queue(s) N".

Re: [RFC 1/4] net: support per queue tx_usecs in sysfs

[David Miller]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date: Tue, 1 Dec 2015 15:44:54 -0800

> While I agree that ethtool provides a similar functionality, ethtool
> was designed (particularly the ethtool -C/c commands) around one queue
> NICs.  We can't change the output or functionality of the user
> interface without breaking a bunch of user's scripts and stuff.

See my suggestion, we can encapsulate existing ethtool commands inside
of a new command that specifies operations that are to be applied to
specific queues.

Do not use sysfs for this.

[Intel-wired-lan] [RFC 1/4] net: support per queue tx_usecs in sysfs

[David Miller]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date: Tue, 1 Dec 2015 15:44:54 -0800

> While I agree that ethtool provides a similar functionality, ethtool
> was designed (particularly the ethtool -C/c commands) around one queue
> NICs.  We can't change the output or functionality of the user
> interface without breaking a bunch of user's scripts and stuff.

See my suggestion, we can encapsulate existing ethtool commands inside
of a new command that specifies operations that are to be applied to
specific queues.

Do not use sysfs for this.