Re: [PATCH] hv_netvsc: Add per-cpu ethtool stats for netvsc

2018-06-07 Thread David Miller
From: Yidong Ren 
Date: Wed,  6 Jun 2018 15:27:00 -0700

> From: Yidong Ren 
> 
> This patch implements following ethtool stats fields for netvsc:
> cpu_tx/rx_packets/bytes
> cpu_vf_tx/rx_packets/bytes
> 
> Corresponding per-cpu counters exist in current code. Exposing these
> counters will help troubleshooting performance issues.
> 
> Signed-off-by: Yidong Ren 

net-next is closed, please resubmit this new feature when it opens again,
also:

> + // fetch percpu stats of vf

Please do not use c++ comments.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] hv_netvsc: Add per-cpu ethtool stats for netvsc

2018-06-06 Thread Stephen Hemminger
On Wed,  6 Jun 2018 15:27:00 -0700
Yidong Ren  wrote:

> From: Yidong Ren 
> 
> This patch implements following ethtool stats fields for netvsc:
> cpu_tx/rx_packets/bytes
> cpu_vf_tx/rx_packets/bytes
> 
> Corresponding per-cpu counters exist in current code. Exposing these
> counters will help troubleshooting performance issues.
> 
> Signed-off-by: Yidong Ren 

This patch would be targeted for net-next (davem's tree);
but net-next is currently closed until 4.19-rc1 is done.

> ---
>  drivers/net/hyperv/hyperv_net.h |  11 
>  drivers/net/hyperv/netvsc_drv.c | 104 +++-
>  2 files changed, 113 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 960f06141472..f8c798bf9418 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -710,6 +710,17 @@ struct netvsc_ethtool_stats {
>   unsigned long wake_queue;
>  };
>  
> +struct netvsc_ethtool_pcpu_stats {
> + u64 rx_packets;
> + u64 rx_bytes;
> + u64 tx_packets;
> + u64 tx_bytes;
> + u64 vf_rx_packets;
> + u64 vf_rx_bytes;
> + u64 vf_tx_packets;
> + u64 vf_tx_bytes;
> +};
> +
>  struct netvsc_vf_pcpu_stats {
>   u64 rx_packets;
>   u64 rx_bytes;
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index da07ccdf84bf..c43e64606c1a 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1104,6 +1104,66 @@ static void netvsc_get_vf_stats(struct net_device *net,
>   }
>  }
>  
> +static void netvsc_get_pcpu_stats(struct net_device *net,
> +   struct netvsc_ethtool_pcpu_stats
> + __percpu *pcpu_tot)
> +{
> + struct net_device_context *ndev_ctx = netdev_priv(net);
> + struct netvsc_device *nvdev = rcu_dereference_rtnl(ndev_ctx->nvdev);
> + int i;
> +
> + // fetch percpu stats of vf

If you ran checkpatch you would see that Linux always uses C style
comments, and not C++ style //

> + for_each_possible_cpu(i) {
> + const struct netvsc_vf_pcpu_stats *stats =
> + per_cpu_ptr(ndev_ctx->vf_stats, i);
> + struct netvsc_ethtool_pcpu_stats *this_tot =
> + per_cpu_ptr(pcpu_tot, i);
> + unsigned int start;
> +
> + do {
> + start = u64_stats_fetch_begin_irq(>syncp);
> + this_tot->vf_rx_packets = stats->rx_packets;
> + this_tot->vf_tx_packets = stats->tx_packets;
> + this_tot->vf_rx_bytes = stats->rx_bytes;
> + this_tot->vf_tx_bytes = stats->tx_bytes;
> + } while (u64_stats_fetch_retry_irq(>syncp, start));
> + this_tot->rx_packets = this_tot->vf_rx_packets;
> + this_tot->tx_packets = this_tot->vf_tx_packets;
> + this_tot->rx_bytes   = this_tot->vf_rx_bytes;
> + this_tot->tx_bytes   = this_tot->vf_tx_bytes;
> + }
> +
> + // fetch percpu stats of netvsc
> + for (i = 0; i < nvdev->num_chn; i++) {
> + const struct netvsc_channel *nvchan = >chan_table[i];
> + const struct netvsc_stats *stats;
> + struct netvsc_ethtool_pcpu_stats *this_tot =
> + per_cpu_ptr(pcpu_tot, nvchan->channel->target_cpu);
> + u64 packets, bytes;
> + unsigned int start;
> +
> + stats = >tx_stats;
> + do {
> + start = u64_stats_fetch_begin_irq(>syncp);
> + packets = stats->packets;
> + bytes = stats->bytes;
> + } while (u64_stats_fetch_retry_irq(>syncp, start));
> +
> + this_tot->tx_bytes  += bytes;
> + this_tot->tx_packets+= packets;
> +
> + stats = >rx_stats;
> + do {
> + start = u64_stats_fetch_begin_irq(>syncp);
> + packets = stats->packets;
> + bytes = stats->bytes;
> + } while (u64_stats_fetch_retry_irq(>syncp, start));
> +
> + this_tot->rx_bytes  += bytes;
> + this_tot->rx_packets+= packets;
> + }
> +}
> +
>  static void netvsc_get_stats64(struct net_device *net,
>  struct rtnl_link_stats64 *t)
>  {
> @@ -1201,6 +1261,23 @@ static const struct {
>   { "rx_no_memory", offsetof(struct netvsc_ethtool_stats, rx_no_memory) },
>   { "stop_queue", offsetof(struct netvsc_ethtool_stats, stop_queue) },
>   { "wake_queue", offsetof(struct netvsc_ethtool_stats, wake_queue) },
> +}, pcpu_stats[] = {
> + { "cpu%u_rx_packets",
> + offsetof(struct netvsc_ethtool_pcpu_stats, rx_packets) },
> + { "cpu%u_rx_bytes",
> + offsetof(struct netvsc_ethtool_pcpu_stats, rx_bytes) },
> + { 

RE: [PATCH] hv_netvsc: add per-cpu ethtool stats for netvsc

2018-06-06 Thread Yidong Ren
Thank you, Greg. I'm sorry for didn't read the guidelines on kernel newbies. 
Please ignore this patch. 

Thanks,
Yidong

-Original Message-
From: Greg KH  
Sent: Wednesday, June 6, 2018 1:34 AM
To: Yidong Ren 
Cc: driverdev-devel@linuxdriverproject.org; Haiyang Zhang 
; Stephen Hemminger ; Madhan 
Sivakumar 
Subject: Re: [PATCH] hv_netvsc: add per-cpu ethtool stats for netvsc

On Tue, Jun 05, 2018 at 08:14:06PM +, Yidong Ren wrote:
> This patch implements following ethtool stats fields for netvsc:
> cpu_rx_packets
> cpu_tx_packets
> cpu_rx_bytes
> cpu_tx_bytes
> cpu_vf_rx_packets
> cpu_vf_tx_packets
> cpu_vf_rx_bytes
> cpu_vf_tx_bytes
> ---

No signed-off-by line?  Always use scripts/checkpatch.pl on your patches os you 
do not get grumpy kernel maintainers telling you to run checkpatch.pl on your 
patches...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] hv_netvsc: add per-cpu ethtool stats for netvsc

2018-06-06 Thread Greg KH
On Tue, Jun 05, 2018 at 08:14:06PM +, Yidong Ren wrote:
> This patch implements following ethtool stats fields for netvsc:
> cpu_rx_packets
> cpu_tx_packets
> cpu_rx_bytes
> cpu_tx_bytes
> cpu_vf_rx_packets
> cpu_vf_tx_packets
> cpu_vf_rx_bytes
> cpu_vf_tx_bytes
> ---

No signed-off-by line?  Always use scripts/checkpatch.pl on your patches
os you do not get grumpy kernel maintainers telling you to run
checkpatch.pl on your patches...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel