Patchwork [RFC,net-2.6] net/ethtool: add multiple queue support to {get,set}_ringparams

login
register
mail settings
Submitter Ajit Khaparde
Date March 14, 2010, 1:43 a.m.
Message ID <20100314014335.GA17208@serverengines.com>
Download mbox | patch
Permalink /patch/47723/
State Rejected
Delegated to: David Miller
Headers show

Comments

Ajit Khaparde - March 14, 2010, 1:43 a.m.
With network devices and hence device drivers supporting
multiple Tx and Rx rings, currently there is no way for
ethtool to specify which Tx/Rx ring the user wants to get/set.
This patch enhances the {get,set}_ringparams by allowing
the user to specify the Tx/Rx ring id of interest.
Please review.

Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
---
 include/linux/ethtool.h |    1 +
 net/core/ethtool.c      |    5 ++++-
 2 files changed, 5 insertions(+), 1 deletions(-)
David Miller - March 14, 2010, 2:11 a.m.
From: Ajit Khaparde <ajitkhaparde@gmail.com>
Date: Sun, 14 Mar 2010 07:13:45 +0530

> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index b33f316..de6a90a 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -206,6 +206,7 @@ struct ethtool_coalesce {
>  /* for configuring RX/TX ring parameters */
>  struct ethtool_ringparam {
>  	__u32	cmd;	/* ETHTOOL_{G,S}RINGPARAM */
> +	__u32	ring_id;
>  
>  	/* Read only attributes.  These indicate the maximum number
>  	 * of pending RX/TX ring entries the driver will allow the

For the millionth time, you cannot change these datastructures
like this without breaking all existing userspace applications
out there.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - March 14, 2010, 4:21 a.m.
On Sun, 2010-03-14 at 07:13 +0530, Ajit Khaparde wrote:
> With network devices and hence device drivers supporting
> multiple Tx and Rx rings, currently there is no way for
> ethtool to specify which Tx/Rx ring the user wants to get/set.
> This patch enhances the {get,set}_ringparams by allowing
> the user to specify the Tx/Rx ring id of interest.
> Please review.
[...]

So long as queue selection is done using a hash of the flow parameters,
there is no sense in setting different sizes for different queues.  If
drivers use additional queues that are not selected in this way and
which may require different sizes, that information also needs to be
exposed through ethtool (and perhaps to the networking core).

Ben.

Patch

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index b33f316..de6a90a 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -206,6 +206,7 @@  struct ethtool_coalesce {
 /* for configuring RX/TX ring parameters */
 struct ethtool_ringparam {
 	__u32	cmd;	/* ETHTOOL_{G,S}RINGPARAM */
+	__u32	ring_id;
 
 	/* Read only attributes.  These indicate the maximum number
 	 * of pending RX/TX ring entries the driver will allow the
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index f4cb6b6..81e2e62 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -881,11 +881,14 @@  static noinline_for_stack int ethtool_set_coalesce(struct net_device *dev, void
 
 static int ethtool_get_ringparam(struct net_device *dev, void __user *useraddr)
 {
-	struct ethtool_ringparam ringparam = { .cmd = ETHTOOL_GRINGPARAM };
+	struct ethtool_ringparam ringparam;
 
 	if (!dev->ethtool_ops->get_ringparam)
 		return -EOPNOTSUPP;
 
+	if (copy_from_user(&ringparam, useraddr, sizeof(ringparam)))
+		return -EFAULT;
+
 	dev->ethtool_ops->get_ringparam(dev, &ringparam);
 
 	if (copy_to_user(useraddr, &ringparam, sizeof(ringparam)))