On 04/17/2018 06:49 PM, greearb@xxxxxxxxxxxxxxx wrote:
> From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>
> This is similar to ETHTOOL_GSTATS, but it allows you to specify
> flags. These flags can be used by the driver to decrease the
> amount of stats refreshed. In particular, this helps with ath10k
> since getting the firmware stats can be slow.
You can configure the statistics refresh rate through the ethtool
coalescing parameter stats_block_coalesce_usecs, not sure if this is
exactly what you had in mind, but if it works, then you might as well
want to use it.
>
> Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
> ---
> include/linux/ethtool.h | 12 ++++++++++++
> include/uapi/linux/ethtool.h | 10 ++++++++++
> net/core/ethtool.c | 40 +++++++++++++++++++++++++++++++++++-----
> 3 files changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index ebe4181..a4aa11f 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -243,6 +243,15 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
> * @get_ethtool_stats: Return extended statistics about the device.
> * This is only useful if the device maintains statistics not
> * included in &struct rtnl_link_stats64.
> + * @get_ethtool_stats2: Return extended statistics about the device.
> + * This is only useful if the device maintains statistics not
> + * included in &struct rtnl_link_stats64.
> + * Takes a flags argument: 0 means all (same as get_ethtool_stats),
> + * 0x1 (ETHTOOL_GS2_SKIP_FW) means skip firmware stats.
> + * Other flags are reserved for now.
> + * Same number of stats will be returned, but some of them might
> + * not be as accurate/refreshed. This is to allow not querying
> + * firmware or other expensive-to-read stats, for instance.
> * @begin: Function to be called before any other operation. Returns a
> * negative error code or zero.
> * @complete: Function to be called after any other operation except
> @@ -355,6 +364,9 @@ struct ethtool_ops {
> int (*set_phys_id)(struct net_device *, enum ethtool_phys_id_state);
> void (*get_ethtool_stats)(struct net_device *,
> struct ethtool_stats *, u64 *);
> + void (*get_ethtool_stats2)(struct net_device *dev,
> + struct ethtool_stats *gstats, u64 *data,
> + u32 flags);
> int (*begin)(struct net_device *);
> void (*complete)(struct net_device *);
> u32 (*get_priv_flags)(struct net_device *);
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 4ca65b5..1c74f3e 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1396,11 +1396,21 @@ enum ethtool_fec_config_bits {
> #define ETHTOOL_PHY_STUNABLE 0x0000004f /* Set PHY tunable configuration */
> #define ETHTOOL_GFECPARAM 0x00000050 /* Get FEC settings */
> #define ETHTOOL_SFECPARAM 0x00000051 /* Set FEC settings */
> +#define ETHTOOL_GSTATS2 0x00000052 /* get NIC-specific statistics
> + * with ability to specify flags.
> + * See ETHTOOL_GS2* below.
> + */
>
> /* compatibility with older code */
> #define SPARC_ETH_GSET ETHTOOL_GSET
> #define SPARC_ETH_SSET ETHTOOL_SSET
>
> +/* GSTATS2 flags */
> +#define ETHTOOL_GS2_SKIP_NONE (0) /* default is to update all stats */
> +#define ETHTOOL_GS2_SKIP_FW (1<<0) /* Skip reading stats that probe firmware,
> + * and thus are slow/expensive.
> + */
> +
> /* Link mode bit indices */
> enum ethtool_link_mode_bit_indices {
> ETHTOOL_LINK_MODE_10baseT_Half_BIT = 0,
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 03416e6..6ec3413 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1952,16 +1952,14 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
> return rc;
> }
>
> -static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
> +static int _ethtool_get_stats(struct net_device *dev, void __user *useraddr,
> + u32 flags)
> {
> struct ethtool_stats stats;
> const struct ethtool_ops *ops = dev->ethtool_ops;
> u64 *data;
> int ret, n_stats;
>
> - if (!ops->get_ethtool_stats || !ops->get_sset_count)
> - return -EOPNOTSUPP;
> -
> n_stats = ops->get_sset_count(dev, ETH_SS_STATS);
> if (n_stats < 0)
> return n_stats;
> @@ -1976,7 +1974,10 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
> if (n_stats && !data)
> return -ENOMEM;
>
> - ops->get_ethtool_stats(dev, &stats, data);
> + if (flags != ETHTOOL_GS2_SKIP_NONE)
> + ops->get_ethtool_stats2(dev, &stats, data, flags);
> + else
> + ops->get_ethtool_stats(dev, &stats, data);
>
> ret = -EFAULT;
> if (copy_to_user(useraddr, &stats, sizeof(stats)))
> @@ -1991,6 +1992,31 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
> return ret;
> }
>
> +static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
> +{
> + const struct ethtool_ops *ops = dev->ethtool_ops;
> +
> + if (!ops->get_ethtool_stats || !ops->get_sset_count)
> + return -EOPNOTSUPP;
> + return _ethtool_get_stats(dev, useraddr, ETHTOOL_GS2_SKIP_NONE);
> +}
> +
> +static int ethtool_get_stats2(struct net_device *dev, void __user *useraddr)
> +{
> + struct ethtool_stats stats;
> + const struct ethtool_ops *ops = dev->ethtool_ops;
> + u32 flags = 0;
> +
> + if (!ops->get_ethtool_stats2 || !ops->get_sset_count)
> + return -EOPNOTSUPP;
> +
> + if (copy_from_user(&stats, useraddr, sizeof(stats)))
> + return -EFAULT;
> +
> + flags = stats.n_stats;
> + return _ethtool_get_stats(dev, useraddr, flags);
> +}
> +
> static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
> {
> struct ethtool_stats stats;
> @@ -2632,6 +2658,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> case ETHTOOL_GSSET_INFO:
> case ETHTOOL_GSTRINGS:
> case ETHTOOL_GSTATS:
> + case ETHTOOL_GSTATS2:
> case ETHTOOL_GPHYSTATS:
> case ETHTOOL_GTSO:
> case ETHTOOL_GPERMADDR:
> @@ -2742,6 +2769,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> case ETHTOOL_GSTATS:
> rc = ethtool_get_stats(dev, useraddr);
> break;
> + case ETHTOOL_GSTATS2:
> + rc = ethtool_get_stats2(dev, useraddr);
> + break;
> case ETHTOOL_GPERMADDR:
> rc = ethtool_get_perm_addr(dev, useraddr);
> break;
>
--
Florian
