[PATCH] net: ethtool support to configure number of RX rings

o Ethtool hook to configure number of RX rings.
o Implementation is scalable to support configuring TX rings.
o There exist ETHTOOL_GRXRINGS command for getting number of RX rings,
  but it is tigtly coupled with RX flow hash configuration.
o Patches for qlcnic and netxen_nic driver supporting this features
  will follow soon.

Signed-off-by: Amit Kumar Salecha <redacted>
---
 include/linux/ethtool.h |   20 ++++++++++++++++++++
 net/core/ethtool.c      |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index aac3e2e..de0decd 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h

@@ -226,6 +226,21 @@ struct ethtool_ringparam {
 	__u32	tx_pending;
 };
 
+/* for configuring number of RX ring */
+struct ethtool_ring {
+	__u32	cmd;	/* ETHTOOL_{G,S}RING */
+
+	/* Read only attributes.  These indicate the maximum number
+	 * of RX rings the driver will allow the user to set.
+	 */
+	__u32	rx_max;
+
+	/* Values changeable by the user.  The valid values are
+	 * in the range 1 to the "*_max" counterpart above.
+	 */
+	__u32	rx_pending;
+};
+
 /* for configuring link flow control parameters */
 struct ethtool_pauseparam {
 	__u32	cmd;	/* ETHTOOL_{G,S}PAUSEPARAM */

@@ -764,6 +779,9 @@ struct ethtool_ops {
 				  struct ethtool_rxfh_indir *);
 	int	(*set_rxfh_indir)(struct net_device *,
 				  const struct ethtool_rxfh_indir *);
+	void	(*get_rings)(struct net_device *, struct ethtool_ring *);
+	int	(*set_rings)(struct net_device *, struct ethtool_ring *);
+
 };
 #endif /* __KERNEL__ */
 

@@ -832,6 +850,8 @@ struct ethtool_ops {
 
 #define ETHTOOL_GFEATURES	0x0000003a /* Get device offload settings */
 #define ETHTOOL_SFEATURES	0x0000003b /* Change device offload settings */
+#define ETHTOOL_GRINGS		0x0000003c /* Get no of rings */
+#define ETHTOOL_SRINGS		0x0000003d /* Set no of rings */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index c1a71bb..2d96b4d 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c

@@ -1426,6 +1426,33 @@ static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr)
 	return dev->ethtool_ops->set_ringparam(dev, &ringparam);
 }
 
+static int ethtool_get_rings(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_ring ring_val = { .cmd = ETHTOOL_GRINGS };
+
+	if (!dev->ethtool_ops->get_rings)
+		return -EOPNOTSUPP;
+
+	dev->ethtool_ops->get_rings(dev, &ring_val);
+
+	if (copy_to_user(useraddr, &ring_val, sizeof(ring_val)))
+		return -EFAULT;
+	return 0;
+}
+
+static int ethtool_set_rings(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_ring ring_val;
+
+	if (!dev->ethtool_ops->set_rings)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&ring_val, useraddr, sizeof(ring_val)))
+		return -EFAULT;
+
+	return dev->ethtool_ops->set_rings(dev, &ring_val);
+}
+
 static int ethtool_get_pauseparam(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_pauseparam pauseparam = { ETHTOOL_GPAUSEPARAM };

@@ -1935,6 +1962,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SGRO:
 		rc = ethtool_set_one_feature(dev, useraddr, ethcmd);
 		break;
+	case ETHTOOL_GRINGS:
+		rc = ethtool_get_rings(dev, useraddr);
+		break;
+	case ETHTOOL_SRINGS:
+		rc = ethtool_set_rings(dev, useraddr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}

-- 
1.7.3.2

Current implementation can't scale to support TX rings. Because once structure is defined, that can't be changed.
As tools will have dependency on it.
If configuring tx rings need to be supported, I need to define Tx parameters in struct ethtool_ring now.
Currently qlcnic and netxen_nic driver doesn't have requirement to configure number of tx rings.
Should I define tx parameters in struct ethtool_ring ?

-Amit

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

On Thu, 2011-03-31 at 00:55 -0500, Amit Salecha wrote:

quoted

o Ethtool hook to configure number of RX rings.
o Implementation is scalable to support configuring TX rings.
o There exist ETHTOOL_GRXRINGS command for getting number of RX rings,
  but it is tigtly coupled with RX flow hash configuration.
o Patches for qlcnic and netxen_nic driver supporting this features
  will follow soon.

Signed-off-by: Amit Kumar Salecha <redacted>
---
 include/linux/ethtool.h |   20 ++++++++++++++++++++
 net/core/ethtool.c      |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index aac3e2e..de0decd 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h

@@ -226,6 +226,21 @@ struct ethtool_ringparam {
      __u32   tx_pending;
 };

+/* for configuring number of RX ring */
+struct ethtool_ring {
+     __u32   cmd;    /* ETHTOOL_{G,S}RING */
+
+     /* Read only attributes.  These indicate the maximum number
+      * of RX rings the driver will allow the user to set.
+      */
+     __u32   rx_max;
+
+     /* Values changeable by the user.  The valid values are
+      * in the range 1 to the "*_max" counterpart above.
+      */
+     __u32   rx_pending;
+};
+

Current implementation can't scale to support TX rings. Because once
structure is defined, that can't be changed.
As tools will have dependency on it.
If configuring tx rings need to be supported, I need to define Tx
parameters in struct ethtool_ring now.
Currently qlcnic and netxen_nic driver doesn't have requirement to
configure number of tx rings.
Should I define tx parameters in struct ethtool_ring ?

You could take this patch: http://patchwork.ozlabs.org/patch/65202/
but drop the affinity control from it.

Your implementation of set_channels() would then refuse all changes to
the numbers of TX/other/combined channels.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

You could take this patch: http://patchwork.ozlabs.org/patch/65202/
but drop the affinity control from it.

Your implementation of set_channels() would then refuse all changes to
the numbers of TX/other/combined channels.

Ben.

Thanks Ben for sharing this.
David,
        Please discard this patch, I will resubmit patch based on Ben patch.

-Amit

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

From: Amit Salecha <redacted>
Date: Fri, 1 Apr 2011 04:16:09 -0500

        Please discard this patch, I will resubmit patch based on Ben patch.

Ok.