Re: [PATCH net-next v6 3/8] virtio_net: support device stats

[Jason Wang <jasowang@redhat.com>]

On Tue, Apr 23, 2024 at 7:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
>
> make virtio-net support getting the stats from the device by ethtool -S
> <eth0>.
>
> NIC statistics:
>      rx0_packets: 582951
>      rx0_bytes: 155307077
>      rx0_drops: 0
>      rx0_xdp_packets: 0
>      rx0_xdp_tx: 0
>      rx0_xdp_redirects: 0
>      rx0_xdp_drops: 0
>      rx0_kicks: 17007
>      rx0_hw_packets: 2179409
>      rx0_hw_bytes: 510015040
>      rx0_hw_notifications: 0
>      rx0_hw_interrupts: 0
>      rx0_hw_needs_csum: 2179409
>      rx0_hw_ratelimit_bytes: 0
>      tx0_packets: 15361
>      tx0_bytes: 1918970
>      tx0_xdp_tx: 0
>      tx0_xdp_tx_drops: 0
>      tx0_kicks: 15361
>      tx0_timeouts: 0
>      tx0_hw_packets: 32272
>      tx0_hw_bytes: 4311698
>      tx0_hw_notifications: 0
>      tx0_hw_interrupts: 0
>      tx0_hw_ratelimit_bytes: 0
>
> The follow stats are hidden, there are exported by the queue stat API
> in the subsequent comment.
>
>     VIRTNET_STATS_DESC_RX(basic, drops)
>     VIRTNET_STATS_DESC_RX(basic, drop_overruns),
>     VIRTNET_STATS_DESC_TX(basic, drops),
>     VIRTNET_STATS_DESC_TX(basic, drop_malformed),
>     VIRTNET_STATS_DESC_RX(csum, csum_valid),
>     VIRTNET_STATS_DESC_RX(csum, csum_none),
>     VIRTNET_STATS_DESC_RX(csum, csum_bad),
>     VIRTNET_STATS_DESC_TX(csum, needs_csum),
>     VIRTNET_STATS_DESC_TX(csum, csum_none),
>     VIRTNET_STATS_DESC_RX(gso, gso_packets),
>     VIRTNET_STATS_DESC_RX(gso, gso_bytes),
>     VIRTNET_STATS_DESC_RX(gso, gso_packets_coalesced),
>     VIRTNET_STATS_DESC_RX(gso, gso_bytes_coalesced),
>     VIRTNET_STATS_DESC_TX(gso, gso_packets),
>     VIRTNET_STATS_DESC_TX(gso, gso_bytes),
>     VIRTNET_STATS_DESC_TX(gso, gso_segments),
>     VIRTNET_STATS_DESC_TX(gso, gso_segments_bytes),
>     VIRTNET_STATS_DESC_RX(speed, ratelimit_packets),
>     VIRTNET_STATS_DESC_TX(speed, ratelimit_packets),
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  drivers/net/virtio_net.c | 476 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 472 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index bd90f9d3d9b7..acae0c310688 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -128,6 +128,57 @@ static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = {
>  #define VIRTNET_SQ_STATS_LEN   ARRAY_SIZE(virtnet_sq_stats_desc)
>  #define VIRTNET_RQ_STATS_LEN   ARRAY_SIZE(virtnet_rq_stats_desc)
>
> +#define VIRTNET_STATS_DESC_CQ(name) \
> +       {#name, offsetof(struct virtio_net_stats_cvq, name)}
> +
> +#define VIRTNET_STATS_DESC_RX(class, name) \
> +       {#name, offsetof(struct virtio_net_stats_rx_ ## class, rx_ ## name)}
> +
> +#define VIRTNET_STATS_DESC_TX(class, name) \
> +       {#name, offsetof(struct virtio_net_stats_tx_ ## class, tx_ ## name)}
> +
> +static const struct virtnet_stat_desc virtnet_stats_cvq_desc[] = {
> +       VIRTNET_STATS_DESC_CQ(command_num),
> +       VIRTNET_STATS_DESC_CQ(ok_num),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_rx_basic_desc[] = {
> +       VIRTNET_STATS_DESC_RX(basic, packets),
> +       VIRTNET_STATS_DESC_RX(basic, bytes),
> +
> +       VIRTNET_STATS_DESC_RX(basic, notifications),
> +       VIRTNET_STATS_DESC_RX(basic, interrupts),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_tx_basic_desc[] = {
> +       VIRTNET_STATS_DESC_TX(basic, packets),
> +       VIRTNET_STATS_DESC_TX(basic, bytes),
> +
> +       VIRTNET_STATS_DESC_TX(basic, notifications),
> +       VIRTNET_STATS_DESC_TX(basic, interrupts),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_rx_csum_desc[] = {
> +       VIRTNET_STATS_DESC_RX(csum, needs_csum),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_tx_gso_desc[] = {
> +       VIRTNET_STATS_DESC_TX(gso, gso_packets_noseg),
> +       VIRTNET_STATS_DESC_TX(gso, gso_bytes_noseg),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_rx_speed_desc[] = {
> +       VIRTNET_STATS_DESC_RX(speed, ratelimit_bytes),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc[] = {
> +       VIRTNET_STATS_DESC_TX(speed, ratelimit_bytes),
> +};
> +
> +#define VIRTNET_Q_TYPE_RX 0
> +#define VIRTNET_Q_TYPE_TX 1
> +#define VIRTNET_Q_TYPE_CQ 2
> +
>  struct virtnet_interrupt_coalesce {
>         u32 max_packets;
>         u32 max_usecs;
> @@ -244,6 +295,7 @@ struct control_buf {
>         struct virtio_net_ctrl_coal_tx coal_tx;
>         struct virtio_net_ctrl_coal_rx coal_rx;
>         struct virtio_net_ctrl_coal_vq coal_vq;
> +       struct virtio_net_stats_capabilities stats_cap;
>  };
>
>  struct virtnet_info {
> @@ -329,6 +381,8 @@ struct virtnet_info {
>
>         /* failover when STANDBY feature enabled */
>         struct failover *failover;
> +
> +       u64 device_stats_cap;
>  };
>
>  struct padded_vnet_hdr {
> @@ -389,6 +443,17 @@ static int rxq2vq(int rxq)
>         return rxq * 2;
>  }
>
> +static int vq_type(struct virtnet_info *vi, int qid)
> +{
> +       if (qid == vi->max_queue_pairs * 2)
> +               return VIRTNET_Q_TYPE_CQ;
> +
> +       if (qid % 2)
> +               return VIRTNET_Q_TYPE_TX;
> +
> +       return VIRTNET_Q_TYPE_RX;
> +}
> +
>  static inline struct virtio_net_common_hdr *
>  skb_vnet_common_hdr(struct sk_buff *skb)
>  {
> @@ -3268,6 +3333,369 @@ static int virtnet_set_channels(struct net_device *dev,
>         return err;
>  }
>
> +static void virtnet_stats_sprintf(u8 **p, const char *fmt, const char *noq_fmt,
> +                                 int num, int qid, const struct virtnet_stat_desc *desc)
> +{
> +       int i;
> +
> +       if (qid < 0) {
> +               for (i = 0; i < num; ++i)
> +                       ethtool_sprintf(p, noq_fmt, desc[i].desc);
> +       } else {
> +               for (i = 0; i < num; ++i)
> +                       ethtool_sprintf(p, fmt, qid, desc[i].desc);
> +       }
> +}
> +
> +static void virtnet_get_hw_stats_string(struct virtnet_info *vi, int type, int qid, u8 **data)
> +{
> +       const struct virtnet_stat_desc *desc;
> +       const char *fmt, *noq_fmt;
> +       u8 *p = *data;
> +       u32 num = 0;
> +
> +       if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
> +               return;
> +
> +       if (type == VIRTNET_Q_TYPE_CQ) {
> +               noq_fmt = "cq_hw_%s";
> +
> +               if (VIRTIO_NET_STATS_TYPE_CVQ & vi->device_stats_cap) {

Nit: I think we'd better to have a consistent style:

If we do

type == VIRTNET_Q_TYPE_CQ

then we'd better use

vi->device_stats_cap & VIRTIO_NET_STATS_TYPE_CVQ

Other than this,

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks