aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff Garzik <jeff@garzik.org>2010-03-04 08:21:53 +0000
committerDavid S. Miller <davem@davemloft.net>2010-03-05 14:00:17 -0800
commitd17792ebdf90289c9fd1bce888076d3d60ecd53b (patch)
tree5b649c673b00d159c1a571387fead761cef570fe
parent723b2f57ad83ee7087acf9a95e8e289414b1f521 (diff)
ethtool: Add direct access to ops->get_sset_count
On 03/04/2010 09:26 AM, Ben Hutchings wrote: > On Thu, 2010-03-04 at 00:51 -0800, Jeff Kirsher wrote: >> From: Jeff Garzik<jgarzik@redhat.com> >> >> This patch is an alternative approach for accessing string >> counts, vs. the drvinfo indirect approach. This way the drvinfo >> space doesn't run out, and we don't break ABI later. > [...] >> --- a/net/core/ethtool.c >> +++ b/net/core/ethtool.c >> @@ -214,6 +214,10 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use >> info.cmd = ETHTOOL_GDRVINFO; >> ops->get_drvinfo(dev,&info); >> >> + /* >> + * this method of obtaining string set info is deprecated; >> + * consider using ETHTOOL_GSSET_INFO instead >> + */ > > This comment belongs on the interface (ethtool.h) not the > implementation. Debatable -- the current comment is located at the callsite of ops->get_sset_count(), which is where an implementor might think to add a new call. Not all the numeric fields in ethtool_drvinfo are obtained from ->get_sset_count(). Hence the "some" in the attached patch to include/linux/ethtool.h, addressing your comment. > [...] >> +static noinline int ethtool_get_sset_info(struct net_device *dev, >> + void __user *useraddr) >> +{ > [...] >> + /* calculate size of return buffer */ >> + for (i = 0; i< 64; i++) >> + if (sset_mask& (1ULL<< i)) >> + n_bits++; > [...] > > We have a function for this: > > n_bits = hweight64(sset_mask); Agreed. I've attached a follow-up patch, which should enable my/Jeff's kernel patch to be applied, followed by this one. Signed-off-by: Jeff Garzik <jgarzik@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--include/linux/ethtool.h7
-rw-r--r--net/core/ethtool.c7
2 files changed, 10 insertions, 4 deletions
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f6f961fefbe..b33f316bb92 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -61,6 +61,13 @@ struct ethtool_drvinfo {
/* For PCI devices, use pci_name(pci_dev). */
char reserved1[32];
char reserved2[12];
+ /*
+ * Some struct members below are filled in
+ * using ops->get_sset_count(). Obtaining
+ * this info from ethtool_drvinfo is now
+ * deprecated; Use ETHTOOL_GSSET_INFO
+ * instead.
+ */
__u32 n_priv_flags; /* number of flags valid in ETHTOOL_GPFLAGS */
__u32 n_stats; /* number of u64's from ETHTOOL_GSTATS */
__u32 testinfo_len;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 70075c47ada..33d2ded50f8 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -17,6 +17,7 @@
#include <linux/errno.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/bitops.h>
#include <asm/uaccess.h>
/*
@@ -216,7 +217,7 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
/*
* this method of obtaining string set info is deprecated;
- * consider using ETHTOOL_GSSET_INFO instead
+ * Use ETHTOOL_GSSET_INFO instead.
*/
if (ops->get_sset_count) {
int rc;
@@ -265,9 +266,7 @@ static noinline int ethtool_get_sset_info(struct net_device *dev,
return 0;
/* calculate size of return buffer */
- for (i = 0; i < 64; i++)
- if (sset_mask & (1ULL << i))
- n_bits++;
+ n_bits = hweight64(sset_mask);
memset(&info, 0, sizeof(info));
info.cmd = ETHTOOL_GSSET_INFO;