diff options
| author | Jeff Garzik | 2010-03-04 08:21:53 +0000 |
|---|---|---|
| committer | David S. Miller | 2010-03-05 14:00:17 -0800 |
| commit | d17792ebdf90289c9fd1bce888076d3d60ecd53b (patch) | |
| tree | 5b649c673b00d159c1a571387fead761cef570fe | |
| parent | 723b2f57ad83ee7087acf9a95e8e289414b1f521 (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.h | 7 | ||||
| -rw-r--r-- | net/core/ethtool.c | 7 |
2 files changed, 10 insertions, 4 deletions
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index f6f961fefbe5..b33f316bb92e 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 70075c47ada8..33d2ded50f84 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; |