diff mbox

[1/3] ethtool: Support ETHTOOL_GSTATS2 command.

Message ID 1524016176-3881-1-git-send-email-greearb@candelatech.com (mailing list archive)
State Not Applicable
Delegated to: Johannes Berg
Headers show

Commit Message

Ben Greear April 18, 2018, 1:49 a.m. UTC
From: Ben Greear <greearb@candelatech.com>

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.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 include/linux/ethtool.h      | 12 ++++++++++++
 include/uapi/linux/ethtool.h | 10 ++++++++++
 net/core/ethtool.c           | 40 +++++++++++++++++++++++++++++++++++-----
 3 files changed, 57 insertions(+), 5 deletions(-)

Comments

Florian Fainelli April 18, 2018, 9:07 p.m. UTC | #1
On 04/17/2018 06:49 PM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> 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@candelatech.com>
> ---
>  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;
>
Johannes Berg April 18, 2018, 9:26 p.m. UTC | #2
On Tue, 2018-04-17 at 18:49 -0700, greearb@candelatech.com wrote:
> 
> + * @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.

It'd be pretty hard to know which flags are firmware stats?

Anyway, there's no way I'm going to take this patch, so you need to
float it on netdev first (best CC us here) and get it applied there
before we can do anything on the wifi side.

johannes
Ben Greear April 18, 2018, 9:51 p.m. UTC | #3
On 04/18/2018 02:26 PM, Johannes Berg wrote:
> On Tue, 2018-04-17 at 18:49 -0700, greearb@candelatech.com wrote:
>>
>> + * @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.
>
> It'd be pretty hard to know which flags are firmware stats?

Yes, it is, but ethtool stats are difficult to understand in a generic
manner anyway, so someone using them is already likely aware of low-level
details of the driver(s) they are using.

In my case, I have lots of virtual stations (or APs), and I want stats
for them as well as for the 'radio', so I would probe the first vdev with
flags of 'skip-none' to get all stats, including radio (firmware) stats.

And then the rest I would just probe the non-firmware stats.

To be honest, I was slightly amused that anyone expressed interest in
this patch originally, but maybe other people have similar use case
and/or drivers with slow-to-acquire stats.

> Anyway, there's no way I'm going to take this patch, so you need to
> float it on netdev first (best CC us here) and get it applied there
> before we can do anything on the wifi side.

I posted the patches to netdev, ath10k and linux-wireless.  If I had only
posted them individually to different lists I figure I'd be hearing about how
the netdev patch is useless because it has no driver support, etc.

Thanks,
Ben
Johannes Berg April 19, 2018, 6:38 a.m. UTC | #4
On Wed, 2018-04-18 at 14:51 -0700, Ben Greear wrote:

> > It'd be pretty hard to know which flags are firmware stats?
> 
> Yes, it is, but ethtool stats are difficult to understand in a generic
> manner anyway, so someone using them is already likely aware of low-level
> details of the driver(s) they are using.

Right. Come to think of it though,

> + * @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.

"skip" vs. "don't refresh" is a bit ambiguous - I'd argue better to
either really skip and not return the non-refreshed ones (also helps
with the identifying), or rename the flag.

Also, wrt. the rest of the patch, I'd argue that it'd be worthwhile to
write the spatch and just add the flags argument to "get_ethtool_stats"
instead of adding a separate method - internally to the kernel it's not
that hard to change.

> I posted the patches to netdev, ath10k and linux-wireless.  If I had only
> posted them individually to different lists I figure I'd be hearing about how
> the netdev patch is useless because it has no driver support, etc.

Sure. I missed netdev, perhaps because it was in To, or more likely
because I was too sleepy. Sorry for the noise.

johannes
Ben Greear April 19, 2018, 3:25 p.m. UTC | #5
On 04/18/2018 11:38 PM, Johannes Berg wrote:
> On Wed, 2018-04-18 at 14:51 -0700, Ben Greear wrote:
>
>>> It'd be pretty hard to know which flags are firmware stats?
>>
>> Yes, it is, but ethtool stats are difficult to understand in a generic
>> manner anyway, so someone using them is already likely aware of low-level
>> details of the driver(s) they are using.
>
> Right. Come to think of it though,
>
>> + * @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.
>
> "skip" vs. "don't refresh" is a bit ambiguous - I'd argue better to
> either really skip and not return the non-refreshed ones (also helps
> with the identifying), or rename the flag.

In order to efficiently parse lots of stats over and over again, I probe
the stat names once on startup, map them to the variable I am trying to use
(since different drivers may have different names for the same basic stat),
and then I store the stat index.

On subsequent stat reads, I just grab stats and go right to the index to
store the stat.

If the stats indexes change, that will complicate my logic quite a bit.

Maybe the flag could be called:  ETHTOOL_GS2_NO_REFRESH_FW ?

>
> Also, wrt. the rest of the patch, I'd argue that it'd be worthwhile to
> write the spatch and just add the flags argument to "get_ethtool_stats"
> instead of adding a separate method - internally to the kernel it's not
> that hard to change.

Maybe this could be in followup patches?  It's going to touch a lot of files,
and might be hell to get merged all at once, and I've never used spatch, so
just maybe someone else will volunteer that part :)

Thanks,
Ben
Johannes Berg April 19, 2018, 3:26 p.m. UTC | #6
On Thu, 2018-04-19 at 08:25 -0700, Ben Greear wrote:
> 
> In order to efficiently parse lots of stats over and over again, I probe
> the stat names once on startup, map them to the variable I am trying to use
> (since different drivers may have different names for the same basic stat),
> and then I store the stat index.
> 
> On subsequent stat reads, I just grab stats and go right to the index to
> store the stat.
> 
> If the stats indexes change, that will complicate my logic quite a bit.

That's a good point.

> Maybe the flag could be called:  ETHTOOL_GS2_NO_REFRESH_FW ?

Sounds more to the point to me, yeah.

> > 
> > Also, wrt. the rest of the patch, I'd argue that it'd be worthwhile to
> > write the spatch and just add the flags argument to "get_ethtool_stats"
> > instead of adding a separate method - internally to the kernel it's not
> > that hard to change.
> 
> Maybe this could be in followup patches?  It's going to touch a lot of files,
> and might be hell to get merged all at once, and I've never used spatch, so
> just maybe someone else will volunteer that part :)

I guess you'll have to ask davem. :)

johannes
David Miller April 22, 2018, 6:54 p.m. UTC | #7
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 19 Apr 2018 17:26:57 +0200

> On Thu, 2018-04-19 at 08:25 -0700, Ben Greear wrote:
>> 
>> Maybe this could be in followup patches?  It's going to touch a lot of files,
>> and might be hell to get merged all at once, and I've never used spatch, so
>> just maybe someone else will volunteer that part :)
> 
> I guess you'll have to ask davem. :)

Well, first of all, I really don't like this.

The first reason is that every time I see interface foo become foo2,
foo3 is never far behind it.

If foo was not extensible enough such that we needed foo2, we beter
design the new thing with explicitly better extensibility in mind.

Furthermore, what you want here is a specific filter.  Someone else
will want to filter on another criteria, and the next person will
want yet another.

This needs to be properly generalized.

And frankly if we had moved to ethtool netlink/devlink by now, we
could just add a netlink attribute for filtering and not even be
having this conversation.
Roopa Prabhu April 22, 2018, 9:15 p.m. UTC | #8
On Sun, Apr 22, 2018 at 11:54 AM, David Miller <davem@davemloft.net> wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Thu, 19 Apr 2018 17:26:57 +0200
>
>> On Thu, 2018-04-19 at 08:25 -0700, Ben Greear wrote:
>>>
>>> Maybe this could be in followup patches?  It's going to touch a lot of files,
>>> and might be hell to get merged all at once, and I've never used spatch, so
>>> just maybe someone else will volunteer that part :)
>>
>> I guess you'll have to ask davem. :)
>
> Well, first of all, I really don't like this.
>
> The first reason is that every time I see interface foo become foo2,
> foo3 is never far behind it.
>
> If foo was not extensible enough such that we needed foo2, we beter
> design the new thing with explicitly better extensibility in mind.
>
> Furthermore, what you want here is a specific filter.  Someone else
> will want to filter on another criteria, and the next person will
> want yet another.
>
> This needs to be properly generalized.
>
> And frankly if we had moved to ethtool netlink/devlink by now, we
> could just add a netlink attribute for filtering and not even be
> having this conversation.


+1.

Also, the RTM_GETSTATS api was added to improve stats query efficiency
(with filters).
 we should look at it  to see if this fits there. Keeping all stats
queries in one place will help.
Ben Greear April 23, 2018, 3:38 p.m. UTC | #9
On 04/22/2018 11:54 AM, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Thu, 19 Apr 2018 17:26:57 +0200
>
>> On Thu, 2018-04-19 at 08:25 -0700, Ben Greear wrote:
>>>
>>> Maybe this could be in followup patches?  It's going to touch a lot of files,
>>> and might be hell to get merged all at once, and I've never used spatch, so
>>> just maybe someone else will volunteer that part :)
>>
>> I guess you'll have to ask davem. :)
>
> Well, first of all, I really don't like this.
>
> The first reason is that every time I see interface foo become foo2,
> foo3 is never far behind it.
>
> If foo was not extensible enough such that we needed foo2, we beter
> design the new thing with explicitly better extensibility in mind.
>
> Furthermore, what you want here is a specific filter.  Someone else
> will want to filter on another criteria, and the next person will
> want yet another.
>
> This needs to be properly generalized.
>
> And frankly if we had moved to ethtool netlink/devlink by now, we
> could just add a netlink attribute for filtering and not even be
> having this conversation.

Well, since there are un-defined flags, it would be simple enough to
extend the API further in the future (flag (1<<31) could mean expect
more input members, etc.  And, adding up to 30 more flags to filter on different
things won't change the API and should be backwards compatible.

But, if you don't want it, that is OK by me, I agree it is a fairly
obscure feature.  It would have saved me time if you had said you didn't
want it at the first RFC patch though...

Thanks,
Ben
Ben Greear April 23, 2018, 3:41 p.m. UTC | #10
On 04/22/2018 02:15 PM, Roopa Prabhu wrote:
> On Sun, Apr 22, 2018 at 11:54 AM, David Miller <davem@davemloft.net> wrote:
>> From: Johannes Berg <johannes@sipsolutions.net>
>> Date: Thu, 19 Apr 2018 17:26:57 +0200
>>
>>> On Thu, 2018-04-19 at 08:25 -0700, Ben Greear wrote:
>>>>
>>>> Maybe this could be in followup patches?  It's going to touch a lot of files,
>>>> and might be hell to get merged all at once, and I've never used spatch, so
>>>> just maybe someone else will volunteer that part :)
>>>
>>> I guess you'll have to ask davem. :)
>>
>> Well, first of all, I really don't like this.
>>
>> The first reason is that every time I see interface foo become foo2,
>> foo3 is never far behind it.
>>
>> If foo was not extensible enough such that we needed foo2, we beter
>> design the new thing with explicitly better extensibility in mind.
>>
>> Furthermore, what you want here is a specific filter.  Someone else
>> will want to filter on another criteria, and the next person will
>> want yet another.
>>
>> This needs to be properly generalized.
>>
>> And frankly if we had moved to ethtool netlink/devlink by now, we
>> could just add a netlink attribute for filtering and not even be
>> having this conversation.
>
>
> +1.
>
> Also, the RTM_GETSTATS api was added to improve stats query efficiency
> (with filters).
>  we should look at it  to see if this fits there. Keeping all stats
> queries in one place will help.

I like the ethtool API, so I'll be sticking with that for now.

Thanks,
Ben
diff mbox

Patch

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;