On Wed, Jun 06, 2012 at 03:52:17PM +0800, Jason Wang wrote: > Satistics counters is useful for debugging and performance optimization, so > this > patch lets virtio_net driver collect following and export them to userspace > through "ethtool -S": > > - number of packets sent/received > - number of bytes sent/received > - number of callbacks for tx/rx > - number of kick for tx/rx > - number of bytes/packets queued for tx > > As virtnet_stats were per-cpu, so both per-cpu and gloabl satistics were > collected like: > > NIC statistics: > tx_bytes[0]: 1731209929 > tx_packets[0]: 60685 > tx_kicks[0]: 63 > tx_callbacks[0]: 73 > tx_queued_bytes[0]: 1935749360 > tx_queued_packets[0]: 80652 > rx_bytes[0]: 2695648 > rx_packets[0]: 40767 > rx_kicks[0]: 1 > rx_callbacks[0]: 2077 > tx_bytes[1]: 9105588697 > tx_packets[1]: 344150 > tx_kicks[1]: 162 > tx_callbacks[1]: 905 > tx_queued_bytes[1]: 8901049412 > tx_queued_packets[1]: 324184 > rx_bytes[1]: 23679828 > rx_packets[1]: 358770 > rx_kicks[1]: 6 > rx_callbacks[1]: 17717 > tx_bytes: 10836798626 > tx_packets: 404835 > tx_kicks: 225 > tx_callbacks: 978 > tx_queued_bytes: 10836798772 > tx_queued_packets: 404836 > rx_bytes: 26375476 > rx_packets: 399537 > rx_kicks: 7 > rx_callbacks: 19794 > > TODO: > > - more statistics > - calculate the pending bytes/pkts >
Do we need that? pending is (queued - packets), no? > Signed-off-by: Jason Wang <[email protected]> > > --- > Changes from v1: > > - style & typo fixs > - convert the statistics fields to array > - use unlikely() > --- > drivers/net/virtio_net.c | 115 > +++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 113 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 6e4aa6f..909a0a7 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -44,8 +44,14 @@ module_param(gso, bool, 0444); > enum virtnet_stats_type { > VIRTNET_TX_BYTES, > VIRTNET_TX_PACKETS, > + VIRTNET_TX_KICKS, > + VIRTNET_TX_CBS, > + VIRTNET_TX_Q_BYTES, > + VIRTNET_TX_Q_PACKETS, > VIRTNET_RX_BYTES, > VIRTNET_RX_PACKETS, > + VIRTNET_RX_KICKS, > + VIRTNET_RX_CBS, > VIRTNET_NUM_STATS, > }; > > @@ -54,6 +60,21 @@ struct virtnet_stats { > u64 data[VIRTNET_NUM_STATS]; > }; > > +static struct { static const? > + char string[ETH_GSTRING_LEN]; > +} virtnet_stats_str_attr[] = { > + { "tx_bytes" }, > + { "tx_packets" }, > + { "tx_kicks" }, > + { "tx_callbacks" }, > + { "tx_queued_bytes" }, > + { "tx_queued_packets" }, > + { "rx_bytes" }, > + { "rx_packets" }, > + { "rx_kicks" }, > + { "rx_callbacks" }, > +}; > + > struct virtnet_info { > struct virtio_device *vdev; > struct virtqueue *rvq, *svq, *cvq; > @@ -146,6 +167,11 @@ static struct page *get_a_page(struct virtnet_info *vi, > gfp_t gfp_mask) > static void skb_xmit_done(struct virtqueue *svq) > { > struct virtnet_info *vi = svq->vdev->priv; > + struct virtnet_stats *stats = this_cpu_ptr(vi->stats); > + > + u64_stats_update_begin(&stats->syncp); > + stats->data[VIRTNET_TX_CBS]++; > + u64_stats_update_end(&stats->syncp); > > /* Suppress further interrupts. */ > virtqueue_disable_cb(svq); > @@ -465,6 +491,7 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t > gfp) > { > int err; > bool oom; > + struct virtnet_stats *stats = this_cpu_ptr(vi->stats); > > do { > if (vi->mergeable_rx_bufs) > @@ -481,13 +508,24 @@ static bool try_fill_recv(struct virtnet_info *vi, > gfp_t gfp) > } while (err > 0); > if (unlikely(vi->num > vi->max)) > vi->max = vi->num; > - virtqueue_kick(vi->rvq); > + if (virtqueue_kick_prepare(vi->rvq)) { if (unlikely()) also move stats here where they are actually used? > + virtqueue_notify(vi->rvq); > + u64_stats_update_begin(&stats->syncp); > + stats->data[VIRTNET_RX_KICKS]++; > + u64_stats_update_end(&stats->syncp); > + } > return !oom; > } > > static void skb_recv_done(struct virtqueue *rvq) > { > struct virtnet_info *vi = rvq->vdev->priv; > + struct virtnet_stats *stats = this_cpu_ptr(vi->stats); > + > + u64_stats_update_begin(&stats->syncp); > + stats->data[VIRTNET_RX_CBS]++; > + u64_stats_update_end(&stats->syncp); > + > /* Schedule NAPI, Suppress further interrupts if successful. */ > if (napi_schedule_prep(&vi->napi)) { > virtqueue_disable_cb(rvq); > @@ -630,7 +668,9 @@ static int xmit_skb(struct virtnet_info *vi, struct > sk_buff *skb) > static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev) > { > struct virtnet_info *vi = netdev_priv(dev); > + struct virtnet_stats *stats = this_cpu_ptr(vi->stats); > int capacity; > + bool kick; > > /* Free up any pending old buffers before queueing new ones. */ > free_old_xmit_skbs(vi); > @@ -655,7 +695,17 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, > struct net_device *dev) > kfree_skb(skb); > return NETDEV_TX_OK; > } > - virtqueue_kick(vi->svq); > + > + kick = virtqueue_kick_prepare(vi->svq); > + if (unlikely(kick)) > + virtqueue_notify(vi->svq); > + > + u64_stats_update_begin(&stats->syncp); > + if (unlikely(kick)) > + stats->data[VIRTNET_TX_KICKS]++; > + stats->data[VIRTNET_TX_Q_BYTES] += skb->len; > + stats->data[VIRTNET_TX_Q_PACKETS]++; > + u64_stats_update_end(&stats->syncp); > > /* Don't wait up for transmitted skbs to be freed. */ > skb_orphan(skb); > @@ -943,10 +993,71 @@ static void virtnet_get_drvinfo(struct net_device *dev, > > } > > +static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 > *buf) > +{ > + int i, cpu; > + switch (stringset) { > + case ETH_SS_STATS: > + for_each_possible_cpu(cpu) > + for (i = 0; i < VIRTNET_NUM_STATS; i++) { > + sprintf(buf, "%s[%u]", > + virtnet_stats_str_attr[i].string, cpu); > + buf += ETH_GSTRING_LEN; I would do ret = snprintf(buf, ETH_GSTRING_LEN, ...) BUG_ON(ret >= ETH_GSTRING_LEN); here to make it more robust. > + } > + for (i = 0; i < VIRTNET_NUM_STATS; i++) { > + memcpy(buf, virtnet_stats_str_attr[i].string, > + ETH_GSTRING_LEN); > + buf += ETH_GSTRING_LEN; > + } So why not just memcpy the whole array there? memcpy(buf, virtnet_stats_str_attr, sizeof virtnet_stats_str_attr); > + break; > + } > +} > + > +static int virtnet_get_sset_count(struct net_device *dev, int sset) > +{ > + switch (sset) { > + case ETH_SS_STATS: also add BUILD_BUG_ON(VIRTNET_NUM_STATS != (sizeof virtnet_stats_str_attr) / ETH_GSTRING_LEN); > + return VIRTNET_NUM_STATS * (num_possible_cpus() + 1); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static void virtnet_get_ethtool_stats(struct net_device *dev, > + struct ethtool_stats *stats, u64 *buf) > +{ > + struct virtnet_info *vi = netdev_priv(dev); > + int cpu, i; > + unsigned int start; > + struct virtnet_stats sample, total; > + > + memset(&total, 0, sizeof(total)); sizeof total when operand is a variable, to distinguish from when it is a type. > + > + for_each_possible_cpu(cpu) { > + struct virtnet_stats *s = per_cpu_ptr(vi->stats, cpu); > + do { > + start = u64_stats_fetch_begin(&s->syncp); > + memcpy(&sample.data, &s->data, > + sizeof(u64) * VIRTNET_NUM_STATS); > + } while (u64_stats_fetch_retry(&s->syncp, start)); > + > + for (i = 0; i < VIRTNET_NUM_STATS; i++) { > + *buf = sample.data[i]; > + total.data[i] += sample.data[i]; > + buf++; > + } > + } > + > + memcpy(buf, &total.data, sizeof(u64) * VIRTNET_NUM_STATS); > +} > + > static const struct ethtool_ops virtnet_ethtool_ops = { > .get_drvinfo = virtnet_get_drvinfo, > .get_link = ethtool_op_get_link, > .get_ringparam = virtnet_get_ringparam, > + .get_ethtool_stats = virtnet_get_ethtool_stats, > + .get_strings = virtnet_get_strings, > + .get_sset_count = virtnet_get_sset_count, > }; > > #define MIN_MTU 68 _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
