[PATCH net-next v1 0/6] virtio-net: support device stats

[PATCH net-next v1 0/6] virtio-net: support device stats

[Xuan Zhuo]

As the spec:

https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82

The virtio net supports to get device stats.

Please review.

Thanks.

v1:
    1. fix some definitions of the marco and the struct


Xuan Zhuo (6):
  virtio_net: introduce device stats feature and structures
  virtio_net: virtnet_send_command supports command-specific-result
  virtio_net: support device stats
  virtio_net: stats map include driver stats
  virtio_net: add the total stats field
  virtio_net: rename stat tx_timeout to timeout

 drivers/net/virtio_net.c        | 533 ++++++++++++++++++++++++++++----
 include/uapi/linux/virtio_net.h | 137 ++++++++
 2 files changed, 610 insertions(+), 60 deletions(-)

--
2.32.0.3.g01195cf9f

[PATCH net-next v1 1/6] virtio_net: introduce device stats feature and structures

[Xuan Zhuo]

The virtio-net device stats spec:

https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82

This commit introduces the relative feature and structures.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/uapi/linux/virtio_net.h | 137 ++++++++++++++++++++++++++++++++
 1 file changed, 137 insertions(+)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index cc65ef0f3c3e..8fca4d1b7635 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -56,6 +56,7 @@
 #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
 					 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
+#define VIRTIO_NET_F_DEVICE_STATS 50	/* Device can provide device-level statistics. */
 #define VIRTIO_NET_F_VQ_NOTF_COAL 52	/* Device supports virtqueue notification coalescing */
 #define VIRTIO_NET_F_NOTF_COAL	53	/* Device supports notifications coalescing */
 #define VIRTIO_NET_F_GUEST_USO4	54	/* Guest can handle USOv4 in. */
@@ -406,4 +407,140 @@ struct  virtio_net_ctrl_coal_vq {
 	struct virtio_net_ctrl_coal coal;
 };
 
+/*
+ * Device Statistics
+ */
+#define VIRTIO_NET_CTRL_STATS         8
+#define VIRTIO_NET_CTRL_STATS_QUERY   0
+#define VIRTIO_NET_CTRL_STATS_GET     1
+
+struct virtio_net_stats_capabilities {
+
+#define VIRTIO_NET_STATS_TYPE_CVQ       (1ULL << 32)
+
+#define VIRTIO_NET_STATS_TYPE_RX_BASIC  (1ULL << 0)
+#define VIRTIO_NET_STATS_TYPE_RX_CSUM   (1ULL << 1)
+#define VIRTIO_NET_STATS_TYPE_RX_GSO    (1ULL << 2)
+#define VIRTIO_NET_STATS_TYPE_RX_SPEED  (1ULL << 3)
+
+#define VIRTIO_NET_STATS_TYPE_TX_BASIC  (1ULL << 16)
+#define VIRTIO_NET_STATS_TYPE_TX_CSUM   (1ULL << 17)
+#define VIRTIO_NET_STATS_TYPE_TX_GSO    (1ULL << 18)
+#define VIRTIO_NET_STATS_TYPE_TX_SPEED  (1ULL << 19)
+
+	__le64 supported_stats_types[1];
+};
+
+struct virtio_net_ctrl_queue_stats {
+	struct {
+		__le16 vq_index;
+		__le16 reserved[3];
+		__le64 types_bitmap[1];
+	} stats[1];
+};
+
+struct virtio_net_stats_reply_hdr {
+#define VIRTIO_NET_STATS_TYPE_REPLY_CVQ       32
+
+#define VIRTIO_NET_STATS_TYPE_REPLY_RX_BASIC  0
+#define VIRTIO_NET_STATS_TYPE_REPLY_RX_CSUM   1
+#define VIRTIO_NET_STATS_TYPE_REPLY_RX_GSO    2
+#define VIRTIO_NET_STATS_TYPE_REPLY_RX_SPEED  3
+
+#define VIRTIO_NET_STATS_TYPE_REPLY_TX_BASIC  16
+#define VIRTIO_NET_STATS_TYPE_REPLY_TX_CSUM   17
+#define VIRTIO_NET_STATS_TYPE_REPLY_TX_GSO    18
+#define VIRTIO_NET_STATS_TYPE_REPLY_TX_SPEED  19
+	__u8 type;
+	__u8 reserved;
+	__le16 vq_index;
+	__le16 reserved1;
+	__le16 size;
+};
+
+struct virtio_net_stats_cvq {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 command_num;
+	__le64 ok_num;
+};
+
+struct virtio_net_stats_rx_basic {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 rx_notifications;
+
+	__le64 rx_packets;
+	__le64 rx_bytes;
+
+	__le64 rx_interrupts;
+
+	__le64 rx_drops;
+	__le64 rx_drop_overruns;
+};
+
+struct virtio_net_stats_tx_basic {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 tx_notifications;
+
+	__le64 tx_packets;
+	__le64 tx_bytes;
+
+	__le64 tx_interrupts;
+
+	__le64 tx_drops;
+	__le64 tx_drop_malformed;
+};
+
+struct virtio_net_stats_rx_csum {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 rx_csum_valid;
+	__le64 rx_needs_csum;
+	__le64 rx_csum_none;
+	__le64 rx_csum_bad;
+};
+
+struct virtio_net_stats_tx_csum {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 tx_csum_none;
+	__le64 tx_needs_csum;
+};
+
+struct virtio_net_stats_rx_gso {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 rx_gso_packets;
+	__le64 rx_gso_bytes;
+	__le64 rx_gso_packets_coalesced;
+	__le64 rx_gso_bytes_coalesced;
+};
+
+struct virtio_net_stats_tx_gso {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 tx_gso_packets;
+	__le64 tx_gso_bytes;
+	__le64 tx_gso_segments;
+	__le64 tx_gso_segments_bytes;
+	__le64 tx_gso_packets_noseg;
+	__le64 tx_gso_bytes_noseg;
+};
+
+struct virtio_net_stats_rx_speed {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 rx_packets_allowance_exceeded;
+	__le64 rx_bytes_allowance_exceeded;
+};
+
+struct virtio_net_stats_tx_speed {
+	struct virtio_net_stats_reply_hdr hdr;
+
+	__le64 tx_packets_allowance_exceeded;
+	__le64 tx_bytes_allowance_exceeded;
+};
+
 #endif /* _UAPI_LINUX_VIRTIO_NET_H */
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net-next v1 1/6] virtio_net: introduce device stats feature and structures

[Zhu Yanjun]

在 2023/12/26 15:30, Xuan Zhuo 写道:
> The virtio-net device stats spec:
>
> https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
>
> This commit introduces the relative feature and structures.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   include/uapi/linux/virtio_net.h | 137 ++++++++++++++++++++++++++++++++
>   1 file changed, 137 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index cc65ef0f3c3e..8fca4d1b7635 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -56,6 +56,7 @@
>   #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
>   					 * Steering */
>   #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> +#define VIRTIO_NET_F_DEVICE_STATS 50	/* Device can provide device-level statistics. */
>   #define VIRTIO_NET_F_VQ_NOTF_COAL 52	/* Device supports virtqueue notification coalescing */
>   #define VIRTIO_NET_F_NOTF_COAL	53	/* Device supports notifications coalescing */
>   #define VIRTIO_NET_F_GUEST_USO4	54	/* Guest can handle USOv4 in. */
> @@ -406,4 +407,140 @@ struct  virtio_net_ctrl_coal_vq {
>   	struct virtio_net_ctrl_coal coal;
>   };
>   
> +/*
> + * Device Statistics
> + */
> +#define VIRTIO_NET_CTRL_STATS         8
> +#define VIRTIO_NET_CTRL_STATS_QUERY   0
> +#define VIRTIO_NET_CTRL_STATS_GET     1
> +
> +struct virtio_net_stats_capabilities {
> +
> +#define VIRTIO_NET_STATS_TYPE_CVQ       (1ULL << 32)
> +
> +#define VIRTIO_NET_STATS_TYPE_RX_BASIC  (1ULL << 0)
> +#define VIRTIO_NET_STATS_TYPE_RX_CSUM   (1ULL << 1)
> +#define VIRTIO_NET_STATS_TYPE_RX_GSO    (1ULL << 2)
> +#define VIRTIO_NET_STATS_TYPE_RX_SPEED  (1ULL << 3)
> +
> +#define VIRTIO_NET_STATS_TYPE_TX_BASIC  (1ULL << 16)
> +#define VIRTIO_NET_STATS_TYPE_TX_CSUM   (1ULL << 17)
> +#define VIRTIO_NET_STATS_TYPE_TX_GSO    (1ULL << 18)
> +#define VIRTIO_NET_STATS_TYPE_TX_SPEED  (1ULL << 19)
> +
> +	__le64 supported_stats_types[1];
> +};
> +
> +struct virtio_net_ctrl_queue_stats {
> +	struct {
> +		__le16 vq_index;
> +		__le16 reserved[3];
> +		__le64 types_bitmap[1];
> +	} stats[1];
> +};
> +
> +struct virtio_net_stats_reply_hdr {
> +#define VIRTIO_NET_STATS_TYPE_REPLY_CVQ       32
> +
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_BASIC  0
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_CSUM   1
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_GSO    2
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_SPEED  3
> +
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_BASIC  16
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_CSUM   17
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_GSO    18
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_SPEED  19
> +	__u8 type;
> +	__u8 reserved;

Thanks a lot. I have made tests. The mentioned errors are fixed in this 
patch series.

Zhu Yanjun

> +	__le16 vq_index;
> +	__le16 reserved1;
> +	__le16 size;
> +};
> +
> +struct virtio_net_stats_cvq {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 command_num;
> +	__le64 ok_num;
> +};
> +
> +struct virtio_net_stats_rx_basic {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_notifications;
> +
> +	__le64 rx_packets;
> +	__le64 rx_bytes;
> +
> +	__le64 rx_interrupts;
> +
> +	__le64 rx_drops;
> +	__le64 rx_drop_overruns;
> +};
> +
> +struct virtio_net_stats_tx_basic {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_notifications;
> +
> +	__le64 tx_packets;
> +	__le64 tx_bytes;
> +
> +	__le64 tx_interrupts;
> +
> +	__le64 tx_drops;
> +	__le64 tx_drop_malformed;
> +};
> +
> +struct virtio_net_stats_rx_csum {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_csum_valid;
> +	__le64 rx_needs_csum;
> +	__le64 rx_csum_none;
> +	__le64 rx_csum_bad;
> +};
> +
> +struct virtio_net_stats_tx_csum {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_csum_none;
> +	__le64 tx_needs_csum;
> +};
> +
> +struct virtio_net_stats_rx_gso {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_gso_packets;
> +	__le64 rx_gso_bytes;
> +	__le64 rx_gso_packets_coalesced;
> +	__le64 rx_gso_bytes_coalesced;
> +};
> +
> +struct virtio_net_stats_tx_gso {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_gso_packets;
> +	__le64 tx_gso_bytes;
> +	__le64 tx_gso_segments;
> +	__le64 tx_gso_segments_bytes;
> +	__le64 tx_gso_packets_noseg;
> +	__le64 tx_gso_bytes_noseg;
> +};
> +
> +struct virtio_net_stats_rx_speed {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_packets_allowance_exceeded;
> +	__le64 rx_bytes_allowance_exceeded;
> +};
> +
> +struct virtio_net_stats_tx_speed {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_packets_allowance_exceeded;
> +	__le64 tx_bytes_allowance_exceeded;
> +};
> +
>   #endif /* _UAPI_LINUX_VIRTIO_NET_H */

Re: [PATCH net-next v1 1/6] virtio_net: introduce device stats feature and structures

[Michael S. Tsirkin]

On Tue, Dec 26, 2023 at 03:30:58PM +0800, Xuan Zhuo wrote:
> The virtio-net device stats spec:
> 
> https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> 
> This commit introduces the relative feature and structures.
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  include/uapi/linux/virtio_net.h | 137 ++++++++++++++++++++++++++++++++
>  1 file changed, 137 insertions(+)
> 
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index cc65ef0f3c3e..8fca4d1b7635 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -56,6 +56,7 @@
>  #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
>  					 * Steering */
>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> +#define VIRTIO_NET_F_DEVICE_STATS 50	/* Device can provide device-level statistics. */
>  #define VIRTIO_NET_F_VQ_NOTF_COAL 52	/* Device supports virtqueue notification coalescing */
>  #define VIRTIO_NET_F_NOTF_COAL	53	/* Device supports notifications coalescing */
>  #define VIRTIO_NET_F_GUEST_USO4	54	/* Guest can handle USOv4 in. */
> @@ -406,4 +407,140 @@ struct  virtio_net_ctrl_coal_vq {
>  	struct virtio_net_ctrl_coal coal;
>  };
>  
> +/*
> + * Device Statistics
> + */
> +#define VIRTIO_NET_CTRL_STATS         8
> +#define VIRTIO_NET_CTRL_STATS_QUERY   0
> +#define VIRTIO_NET_CTRL_STATS_GET     1
> +
> +struct virtio_net_stats_capabilities {
> +
> +#define VIRTIO_NET_STATS_TYPE_CVQ       (1ULL << 32)
> +
> +#define VIRTIO_NET_STATS_TYPE_RX_BASIC  (1ULL << 0)
> +#define VIRTIO_NET_STATS_TYPE_RX_CSUM   (1ULL << 1)
> +#define VIRTIO_NET_STATS_TYPE_RX_GSO    (1ULL << 2)
> +#define VIRTIO_NET_STATS_TYPE_RX_SPEED  (1ULL << 3)
> +
> +#define VIRTIO_NET_STATS_TYPE_TX_BASIC  (1ULL << 16)
> +#define VIRTIO_NET_STATS_TYPE_TX_CSUM   (1ULL << 17)
> +#define VIRTIO_NET_STATS_TYPE_TX_GSO    (1ULL << 18)
> +#define VIRTIO_NET_STATS_TYPE_TX_SPEED  (1ULL << 19)
> +
> +	__le64 supported_stats_types[1];
> +};
> +
> +struct virtio_net_ctrl_queue_stats {
> +	struct {
> +		__le16 vq_index;
> +		__le16 reserved[3];
> +		__le64 types_bitmap[1];
> +	} stats[1];
> +};
> +
> +struct virtio_net_stats_reply_hdr {
> +#define VIRTIO_NET_STATS_TYPE_REPLY_CVQ       32
> +
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_BASIC  0
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_CSUM   1
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_GSO    2
> +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_SPEED  3
> +
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_BASIC  16
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_CSUM   17
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_GSO    18
> +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_SPEED  19
> +	__u8 type;
> +	__u8 reserved;
> +	__le16 vq_index;
> +	__le16 reserved1;
> +	__le16 size;
> +};
> +
> +struct virtio_net_stats_cvq {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 command_num;
> +	__le64 ok_num;
> +};
> +
> +struct virtio_net_stats_rx_basic {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_notifications;
> +
> +	__le64 rx_packets;
> +	__le64 rx_bytes;
> +
> +	__le64 rx_interrupts;
> +
> +	__le64 rx_drops;
> +	__le64 rx_drop_overruns;
> +};
> +
> +struct virtio_net_stats_tx_basic {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_notifications;
> +
> +	__le64 tx_packets;
> +	__le64 tx_bytes;
> +
> +	__le64 tx_interrupts;
> +
> +	__le64 tx_drops;
> +	__le64 tx_drop_malformed;
> +};
> +
> +struct virtio_net_stats_rx_csum {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_csum_valid;
> +	__le64 rx_needs_csum;
> +	__le64 rx_csum_none;
> +	__le64 rx_csum_bad;
> +};
> +
> +struct virtio_net_stats_tx_csum {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_csum_none;
> +	__le64 tx_needs_csum;
> +};
> +
> +struct virtio_net_stats_rx_gso {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_gso_packets;
> +	__le64 rx_gso_bytes;
> +	__le64 rx_gso_packets_coalesced;
> +	__le64 rx_gso_bytes_coalesced;
> +};
> +
> +struct virtio_net_stats_tx_gso {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_gso_packets;
> +	__le64 tx_gso_bytes;
> +	__le64 tx_gso_segments;
> +	__le64 tx_gso_segments_bytes;
> +	__le64 tx_gso_packets_noseg;
> +	__le64 tx_gso_bytes_noseg;
> +};
> +
> +struct virtio_net_stats_rx_speed {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 rx_packets_allowance_exceeded;
> +	__le64 rx_bytes_allowance_exceeded;
> +};
> +
> +struct virtio_net_stats_tx_speed {
> +	struct virtio_net_stats_reply_hdr hdr;
> +
> +	__le64 tx_packets_allowance_exceeded;
> +	__le64 tx_bytes_allowance_exceeded;
> +};
> +

A ton of duplication here. E.g.  virtio_net_stats_rx_speed and
virtio_net_stats_tx_speed are exactly the same.





>  #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> -- 
> 2.32.0.3.g01195cf9f

Re: [PATCH net-next v1 1/6] virtio_net: introduce device stats feature and structures

[Xuan Zhuo]

On Wed, 27 Dec 2023 16:05:25 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 26, 2023 at 03:30:58PM +0800, Xuan Zhuo wrote:
> > The virtio-net device stats spec:
> >
> > https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> >
> > This commit introduces the relative feature and structures.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  include/uapi/linux/virtio_net.h | 137 ++++++++++++++++++++++++++++++++
> >  1 file changed, 137 insertions(+)
> >
> > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > index cc65ef0f3c3e..8fca4d1b7635 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -56,6 +56,7 @@
> >  #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
> >  					 * Steering */
> >  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> > +#define VIRTIO_NET_F_DEVICE_STATS 50	/* Device can provide device-level statistics. */
> >  #define VIRTIO_NET_F_VQ_NOTF_COAL 52	/* Device supports virtqueue notification coalescing */
> >  #define VIRTIO_NET_F_NOTF_COAL	53	/* Device supports notifications coalescing */
> >  #define VIRTIO_NET_F_GUEST_USO4	54	/* Guest can handle USOv4 in. */
> > @@ -406,4 +407,140 @@ struct  virtio_net_ctrl_coal_vq {
> >  	struct virtio_net_ctrl_coal coal;
> >  };
> >
> > +/*
> > + * Device Statistics
> > + */
> > +#define VIRTIO_NET_CTRL_STATS         8
> > +#define VIRTIO_NET_CTRL_STATS_QUERY   0
> > +#define VIRTIO_NET_CTRL_STATS_GET     1
> > +
> > +struct virtio_net_stats_capabilities {
> > +
> > +#define VIRTIO_NET_STATS_TYPE_CVQ       (1ULL << 32)
> > +
> > +#define VIRTIO_NET_STATS_TYPE_RX_BASIC  (1ULL << 0)
> > +#define VIRTIO_NET_STATS_TYPE_RX_CSUM   (1ULL << 1)
> > +#define VIRTIO_NET_STATS_TYPE_RX_GSO    (1ULL << 2)
> > +#define VIRTIO_NET_STATS_TYPE_RX_SPEED  (1ULL << 3)
> > +
> > +#define VIRTIO_NET_STATS_TYPE_TX_BASIC  (1ULL << 16)
> > +#define VIRTIO_NET_STATS_TYPE_TX_CSUM   (1ULL << 17)
> > +#define VIRTIO_NET_STATS_TYPE_TX_GSO    (1ULL << 18)
> > +#define VIRTIO_NET_STATS_TYPE_TX_SPEED  (1ULL << 19)
> > +
> > +	__le64 supported_stats_types[1];
> > +};
> > +
> > +struct virtio_net_ctrl_queue_stats {
> > +	struct {
> > +		__le16 vq_index;
> > +		__le16 reserved[3];
> > +		__le64 types_bitmap[1];
> > +	} stats[1];
> > +};
> > +
> > +struct virtio_net_stats_reply_hdr {
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_CVQ       32
> > +
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_BASIC  0
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_CSUM   1
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_GSO    2
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_RX_SPEED  3
> > +
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_BASIC  16
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_CSUM   17
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_GSO    18
> > +#define VIRTIO_NET_STATS_TYPE_REPLY_TX_SPEED  19
> > +	__u8 type;
> > +	__u8 reserved;
> > +	__le16 vq_index;
> > +	__le16 reserved1;
> > +	__le16 size;
> > +};
> > +
> > +struct virtio_net_stats_cvq {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 command_num;
> > +	__le64 ok_num;
> > +};
> > +
> > +struct virtio_net_stats_rx_basic {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 rx_notifications;
> > +
> > +	__le64 rx_packets;
> > +	__le64 rx_bytes;
> > +
> > +	__le64 rx_interrupts;
> > +
> > +	__le64 rx_drops;
> > +	__le64 rx_drop_overruns;
> > +};
> > +
> > +struct virtio_net_stats_tx_basic {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 tx_notifications;
> > +
> > +	__le64 tx_packets;
> > +	__le64 tx_bytes;
> > +
> > +	__le64 tx_interrupts;
> > +
> > +	__le64 tx_drops;
> > +	__le64 tx_drop_malformed;
> > +};
> > +
> > +struct virtio_net_stats_rx_csum {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 rx_csum_valid;
> > +	__le64 rx_needs_csum;
> > +	__le64 rx_csum_none;
> > +	__le64 rx_csum_bad;
> > +};
> > +
> > +struct virtio_net_stats_tx_csum {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 tx_csum_none;
> > +	__le64 tx_needs_csum;
> > +};
> > +
> > +struct virtio_net_stats_rx_gso {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 rx_gso_packets;
> > +	__le64 rx_gso_bytes;
> > +	__le64 rx_gso_packets_coalesced;
> > +	__le64 rx_gso_bytes_coalesced;
> > +};
> > +
> > +struct virtio_net_stats_tx_gso {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 tx_gso_packets;
> > +	__le64 tx_gso_bytes;
> > +	__le64 tx_gso_segments;
> > +	__le64 tx_gso_segments_bytes;
> > +	__le64 tx_gso_packets_noseg;
> > +	__le64 tx_gso_bytes_noseg;
> > +};
> > +
> > +struct virtio_net_stats_rx_speed {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 rx_packets_allowance_exceeded;
> > +	__le64 rx_bytes_allowance_exceeded;
> > +};
> > +
> > +struct virtio_net_stats_tx_speed {
> > +	struct virtio_net_stats_reply_hdr hdr;
> > +
> > +	__le64 tx_packets_allowance_exceeded;
> > +	__le64 tx_bytes_allowance_exceeded;
> > +};
> > +
>
> A ton of duplication here. E.g.  virtio_net_stats_rx_speed and
> virtio_net_stats_tx_speed are exactly the same.
>

YES.

But I don't think it matters.

Thanks.


>
>
>
>
> >  #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> > --
> > 2.32.0.3.g01195cf9f
>

[PATCH net-next v1 2/6] virtio_net: virtnet_send_command supports command-specific-result

[Xuan Zhuo]

As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82

The virtnet cvq supports to get result from the device.
This commit implement this.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 47 +++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index d16f592c2061..31b9ead6260d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2451,10 +2451,11 @@ static int virtnet_tx_resize(struct virtnet_info *vi,
  * never fail unless improperly formatted.
  */
 static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
-				 struct scatterlist *out)
+				 struct scatterlist *out,
+				 struct scatterlist *in)
 {
-	struct scatterlist *sgs[4], hdr, stat;
-	unsigned out_num = 0, tmp;
+	struct scatterlist *sgs[5], hdr, stat;
+	u32 out_num = 0, tmp, in_num = 0;
 	int ret;
 
 	/* Caller should know better */
@@ -2472,10 +2473,13 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
 
 	/* Add return status. */
 	sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
-	sgs[out_num] = &stat;
+	sgs[out_num + in_num++] = &stat;
 
-	BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
-	ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
+	if (in)
+		sgs[out_num + in_num++] = in;
+
+	BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
+	ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC);
 	if (ret < 0) {
 		dev_warn(&vi->vdev->dev,
 			 "Failed to add sgs for command vq: %d\n.", ret);
@@ -2517,7 +2521,8 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
 		sg_init_one(&sg, addr->sa_data, dev->addr_len);
 		if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
-					  VIRTIO_NET_CTRL_MAC_ADDR_SET, &sg)) {
+					  VIRTIO_NET_CTRL_MAC_ADDR_SET,
+					  &sg, NULL)) {
 			dev_warn(&vdev->dev,
 				 "Failed to set mac address by vq command.\n");
 			ret = -EINVAL;
@@ -2586,7 +2591,7 @@ static void virtnet_ack_link_announce(struct virtnet_info *vi)
 {
 	rtnl_lock();
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_ANNOUNCE,
-				  VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL))
+				  VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL, NULL))
 		dev_warn(&vi->dev->dev, "Failed to ack link announce.\n");
 	rtnl_unlock();
 }
@@ -2603,7 +2608,7 @@ static int _virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
 	sg_init_one(&sg, &vi->ctrl->mq, sizeof(vi->ctrl->mq));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
-				  VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg)) {
+				  VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg, NULL)) {
 		dev_warn(&dev->dev, "Fail to set num of queue pairs to %d\n",
 			 queue_pairs);
 		return -EINVAL;
@@ -2664,14 +2669,14 @@ static void virtnet_set_rx_mode(struct net_device *dev)
 	sg_init_one(sg, &vi->ctrl->promisc, sizeof(vi->ctrl->promisc));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
-				  VIRTIO_NET_CTRL_RX_PROMISC, sg))
+				  VIRTIO_NET_CTRL_RX_PROMISC, sg, NULL))
 		dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
 			 vi->ctrl->promisc ? "en" : "dis");
 
 	sg_init_one(sg, &vi->ctrl->allmulti, sizeof(vi->ctrl->allmulti));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
-				  VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
+				  VIRTIO_NET_CTRL_RX_ALLMULTI, sg, NULL))
 		dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
 			 vi->ctrl->allmulti ? "en" : "dis");
 
@@ -2707,7 +2712,7 @@ static void virtnet_set_rx_mode(struct net_device *dev)
 		   sizeof(mac_data->entries) + (mc_count * ETH_ALEN));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
-				  VIRTIO_NET_CTRL_MAC_TABLE_SET, sg))
+				  VIRTIO_NET_CTRL_MAC_TABLE_SET, sg, NULL))
 		dev_warn(&dev->dev, "Failed to set MAC filter table.\n");
 
 	kfree(buf);
@@ -2723,7 +2728,7 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
 	sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
-				  VIRTIO_NET_CTRL_VLAN_ADD, &sg))
+				  VIRTIO_NET_CTRL_VLAN_ADD, &sg, NULL))
 		dev_warn(&dev->dev, "Failed to add VLAN ID %d.\n", vid);
 	return 0;
 }
@@ -2738,7 +2743,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
 	sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
-				  VIRTIO_NET_CTRL_VLAN_DEL, &sg))
+				  VIRTIO_NET_CTRL_VLAN_DEL, &sg, NULL))
 		dev_warn(&dev->dev, "Failed to kill VLAN ID %d.\n", vid);
 	return 0;
 }
@@ -2956,7 +2961,7 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
 				  vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
-				  : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
+				  : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs, NULL)) {
 		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
 		return false;
 	}
@@ -3274,7 +3279,7 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
 				  VIRTIO_NET_CTRL_NOTF_COAL_TX_SET,
-				  &sgs_tx))
+				  &sgs_tx, NULL))
 		return -EINVAL;
 
 	/* Save parameters */
@@ -3291,7 +3296,7 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
 				  VIRTIO_NET_CTRL_NOTF_COAL_RX_SET,
-				  &sgs_rx))
+				  &sgs_rx, NULL))
 		return -EINVAL;
 
 	/* Save parameters */
@@ -3317,7 +3322,7 @@ static int virtnet_send_ctrl_coal_vq_cmd(struct virtnet_info *vi,
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
 				  VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET,
-				  &sgs))
+				  &sgs, NULL))
 		return -EINVAL;
 
 	return 0;
@@ -3687,7 +3692,8 @@ static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
 	sg_init_one(&sg, &vi->ctrl->offloads, sizeof(vi->ctrl->offloads));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
-				  VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, &sg)) {
+				  VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET,
+				  &sg, NULL)) {
 		dev_warn(&vi->dev->dev, "Fail to set guest offload.\n");
 		return -EINVAL;
 	}
@@ -4619,7 +4625,8 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 		sg_init_one(&sg, dev->dev_addr, dev->addr_len);
 		if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
-					  VIRTIO_NET_CTRL_MAC_ADDR_SET, &sg)) {
+					  VIRTIO_NET_CTRL_MAC_ADDR_SET,
+					  &sg, NULL)) {
 			pr_debug("virtio_net: setting MAC address failed\n");
 			rtnl_unlock();
 			err = -EINVAL;
-- 
2.32.0.3.g01195cf9f

[PATCH net-next v1 3/6] virtio_net: support device stats

[Xuan Zhuo]

As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82

Virtio-net supports to get the stats from the device by ethtool -S <eth0>.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 354 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 350 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 31b9ead6260d..1f4d9605552f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -127,6 +127,113 @@ 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(qtype, class, name) \
+	{#name, offsetof(struct virtio_net_stats_ ## qtype ## _ ## class, qtype ## _ ## name)}
+
+static const struct virtnet_stat_desc virtnet_stats_cvq_desc[] = {
+	{"command_num", offsetof(struct virtio_net_stats_cvq, command_num)},
+	{"ok_num", offsetof(struct virtio_net_stats_cvq, 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),
+
+	VIRTNET_STATS_DESC(rx, basic, drops),
+	VIRTNET_STATS_DESC(rx, basic, drop_overruns),
+};
+
+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),
+
+	VIRTNET_STATS_DESC(tx, basic, drops),
+	VIRTNET_STATS_DESC(tx, basic, drop_malformed),
+};
+
+static const struct virtnet_stat_desc virtnet_stats_rx_csum_desc[] = {
+	VIRTNET_STATS_DESC(rx, csum, csum_valid),
+	VIRTNET_STATS_DESC(rx, csum, needs_csum),
+
+	VIRTNET_STATS_DESC(rx, csum, csum_none),
+	VIRTNET_STATS_DESC(rx, csum, csum_bad),
+};
+
+static const struct virtnet_stat_desc virtnet_stats_tx_csum_desc[] = {
+	VIRTNET_STATS_DESC(tx, csum, needs_csum),
+	VIRTNET_STATS_DESC(tx, csum, csum_none),
+};
+
+static const struct virtnet_stat_desc virtnet_stats_rx_gso_desc[] = {
+	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),
+};
+
+static const struct virtnet_stat_desc virtnet_stats_tx_gso_desc[] = {
+	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(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, packets_allowance_exceeded),
+	VIRTNET_STATS_DESC(rx, speed, bytes_allowance_exceeded),
+};
+
+static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc[] = {
+	VIRTNET_STATS_DESC(tx, speed, packets_allowance_exceeded),
+	VIRTNET_STATS_DESC(tx, speed, packets_allowance_exceeded),
+};
+
+struct virtnet_stats_map {
+	u64 flag;
+	u32 len;
+	u32 num;
+
+#define VIRTNET_STATS_Q_TYPE_RX 0
+#define VIRTNET_STATS_Q_TYPE_TX 1
+#define VIRTNET_STATS_Q_TYPE_CQ 2
+	u32 queue_type;
+
+	u8 type;
+	const struct virtnet_stat_desc *desc;
+};
+
+#define VIRTNET_DEVICE_STATS_MAP_ITEM(TYPE, type, queue_type)	\
+	{							\
+		VIRTIO_NET_STATS_TYPE_##TYPE,			\
+		sizeof(struct virtio_net_stats_ ## type),	\
+		ARRAY_SIZE(virtnet_stats_ ## type ##_desc),	\
+		VIRTNET_STATS_Q_TYPE_##queue_type,		\
+		VIRTIO_NET_STATS_TYPE_REPLY_##TYPE,		\
+		&virtnet_stats_##type##_desc[0]			\
+	}
+
+static struct virtnet_stats_map virtio_net_stats_map[] = {
+	VIRTNET_DEVICE_STATS_MAP_ITEM(CVQ, cvq, CQ),
+
+	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_BASIC, rx_basic, RX),
+	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_CSUM,  rx_csum,  RX),
+	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_GSO,   rx_gso,   RX),
+	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_SPEED, rx_speed, RX),
+
+	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_BASIC, tx_basic, TX),
+	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_CSUM,  tx_csum,  TX),
+	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_GSO,   tx_gso,   TX),
+	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_SPEED, tx_speed, TX),
+};
+
 struct virtnet_interrupt_coalesce {
 	u32 max_packets;
 	u32 max_usecs;
@@ -232,6 +339,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 {
@@ -314,6 +422,8 @@ struct virtnet_info {
 
 	/* failover when STANDBY feature enabled */
 	struct failover *failover;
+
+	u64 device_stats_cap;
 };
 
 struct padded_vnet_hdr {
@@ -3157,6 +3267,204 @@ static int virtnet_set_channels(struct net_device *dev,
 	return err;
 }
 
+static void virtnet_get_hw_stats_string(struct virtnet_info *vi, int type, int qid, u8 **data)
+{
+	struct virtnet_stats_map *m;
+	int i, j;
+	u8 *p = *data;
+
+	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
+		m = &virtio_net_stats_map[i];
+
+		if (m->queue_type != type)
+			continue;
+
+		if (!(vi->device_stats_cap & m->flag))
+			continue;
+
+		for (j = 0; j < m->num; ++j) {
+			if (type == VIRTNET_STATS_Q_TYPE_RX)
+				ethtool_sprintf(&p, "rx_queue_hw_%u_%s", qid, m->desc[j].desc);
+
+			else if (type == VIRTNET_STATS_Q_TYPE_TX)
+				ethtool_sprintf(&p, "tx_queue_hw_%u_%s", qid, m->desc[j].desc);
+
+			else if (type == VIRTNET_STATS_Q_TYPE_CQ)
+				ethtool_sprintf(&p, "cq_hw_%s", m->desc[j].desc);
+		}
+	}
+
+	*data = p;
+}
+
+struct virtnet_stats_ctx {
+	u32 num_cq;
+	u32 num_rx;
+	u32 num_tx;
+
+	u64 bitmap_cq;
+	u64 bitmap_rx;
+	u64 bitmap_tx;
+
+	u32 size_cq;
+	u32 size_rx;
+	u32 size_tx;
+
+	u64 *data;
+};
+
+static void virtnet_stats_ctx_init(struct virtnet_info *vi,
+				   struct virtnet_stats_ctx *ctx,
+				   u64 *data)
+{
+	struct virtnet_stats_map *m;
+	int i;
+
+	ctx->data = data;
+
+	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
+		m = &virtio_net_stats_map[i];
+
+		if (vi->device_stats_cap & m->flag) {
+			if (m->queue_type == VIRTNET_STATS_Q_TYPE_CQ) {
+				ctx->bitmap_cq |= m->flag;
+				ctx->num_cq += m->num;
+				ctx->size_cq += m->len;
+			}
+
+			if (m->queue_type == VIRTNET_STATS_Q_TYPE_RX) {
+				ctx->bitmap_rx |= m->flag;
+				ctx->num_rx += m->num;
+				ctx->size_rx += m->len;
+			}
+
+			if (m->queue_type == VIRTNET_STATS_Q_TYPE_TX) {
+				ctx->bitmap_tx |= m->flag;
+				ctx->num_tx += m->num;
+				ctx->size_tx += m->len;
+			}
+		}
+	}
+}
+
+static int virtnet_get_hw_stats(struct virtnet_info *vi,
+				struct virtnet_stats_ctx *ctx)
+{
+	struct virtio_net_ctrl_queue_stats *req;
+	struct virtio_net_stats_reply_hdr *hdr;
+	struct scatterlist sgs_in, sgs_out;
+	u32 num_rx, num_tx, num_cq, offset;
+	int qnum, i, j,  qid, res_size;
+	struct virtnet_stats_map *m;
+	void *reply, *p;
+	u64 bitmap;
+	int ok;
+	u64 *v;
+
+	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
+		return 0;
+
+	qnum = 0;
+	if (ctx->bitmap_cq)
+		qnum += 1;
+
+	if (ctx->bitmap_rx)
+		qnum += vi->curr_queue_pairs;
+
+	if (ctx->bitmap_tx)
+		qnum += vi->curr_queue_pairs;
+
+	req = kcalloc(qnum, sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	res_size = (ctx->size_rx + ctx->size_tx) * vi->curr_queue_pairs + ctx->size_cq;
+	reply = kmalloc(res_size, GFP_KERNEL);
+	if (!reply) {
+		kfree(req);
+		return -ENOMEM;
+	}
+
+	j = 0;
+	for (i = 0; i < vi->curr_queue_pairs; ++i) {
+		if (ctx->bitmap_rx) {
+			req->stats[j].vq_index = cpu_to_le16(i * 2);
+			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_rx);
+			++j;
+		}
+
+		if (ctx->bitmap_tx) {
+			req->stats[j].vq_index = cpu_to_le16(i * 2 + 1);
+			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_tx);
+			++j;
+		}
+	}
+
+	if (ctx->size_cq) {
+		req->stats[j].vq_index = cpu_to_le16(vi->max_queue_pairs * 2);
+		req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_cq);
+		++j;
+	}
+
+	sg_init_one(&sgs_out, req, sizeof(*req) * j);
+	sg_init_one(&sgs_in, reply, res_size);
+
+	ok = virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
+				  VIRTIO_NET_CTRL_STATS_GET,
+				  &sgs_out, &sgs_in);
+	kfree(req);
+
+	if (!ok) {
+		kfree(reply);
+		return ok;
+	}
+
+	num_rx = VIRTNET_RQ_STATS_LEN + ctx->num_rx;
+	num_tx = VIRTNET_SQ_STATS_LEN + ctx->num_tx;
+	num_cq = ctx->num_tx;
+
+	for (p = reply; p - reply < res_size; p += virtio16_to_cpu(vi->vdev, hdr->size)) {
+		hdr = p;
+
+		qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
+
+		if (qid == vi->max_queue_pairs * 2) {
+			offset = 0;
+			bitmap = ctx->bitmap_cq;
+		} else if (qid % 2) {
+			offset = num_cq + num_rx * vi->curr_queue_pairs + num_tx * (qid / 2);
+			offset += VIRTNET_SQ_STATS_LEN;
+			bitmap = ctx->bitmap_tx;
+		} else {
+			offset = num_cq + num_rx * (qid / 2) + VIRTNET_RQ_STATS_LEN;
+			bitmap = ctx->bitmap_rx;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
+			m = &virtio_net_stats_map[i];
+
+			if (m->flag & bitmap)
+				offset += m->num;
+
+			if (hdr->type != m->type)
+				continue;
+
+			for (j = 0; j < m->num; ++j) {
+				v = p + m->desc[j].offset;
+				ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
+			}
+
+			break;
+		}
+	}
+
+	kfree(reply);
+	return 0;
+}
+
 static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
@@ -3165,16 +3473,22 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 
 	switch (stringset) {
 	case ETH_SS_STATS:
+		virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
+
 		for (i = 0; i < vi->curr_queue_pairs; i++) {
 			for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++)
 				ethtool_sprintf(&p, "rx_queue_%u_%s", i,
 						virtnet_rq_stats_desc[j].desc);
+
+			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, i, &p);
 		}
 
 		for (i = 0; i < vi->curr_queue_pairs; i++) {
 			for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++)
 				ethtool_sprintf(&p, "tx_queue_%u_%s", i,
 						virtnet_sq_stats_desc[j].desc);
+
+			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, i, &p);
 		}
 		break;
 	}
@@ -3183,11 +3497,35 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 static int virtnet_get_sset_count(struct net_device *dev, int sset)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
+	struct virtnet_stats_ctx ctx = {0};
+	u32 pair_count;
 
 	switch (sset) {
 	case ETH_SS_STATS:
-		return vi->curr_queue_pairs * (VIRTNET_RQ_STATS_LEN +
-					       VIRTNET_SQ_STATS_LEN);
+		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS) &&
+		    !vi->device_stats_cap) {
+			struct scatterlist sg;
+
+			sg_init_one(&sg, &vi->ctrl->stats_cap, sizeof(vi->ctrl->stats_cap));
+
+			if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
+						  VIRTIO_NET_CTRL_STATS_QUERY,
+						  NULL, &sg)) {
+				dev_warn(&dev->dev, "Fail to get stats capability\n");
+			} else {
+				__le64 v;
+
+				v = vi->ctrl->stats_cap.supported_stats_types[0];
+				vi->device_stats_cap = virtio64_to_cpu(vi->vdev, v);
+			}
+		}
+
+		virtnet_stats_ctx_init(vi, &ctx, NULL);
+
+		pair_count = VIRTNET_RQ_STATS_LEN + VIRTNET_SQ_STATS_LEN;
+		pair_count += ctx.num_rx + ctx.num_tx;
+
+		return ctx.num_cq + vi->curr_queue_pairs * pair_count;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -3197,11 +3535,17 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
 				      struct ethtool_stats *stats, u64 *data)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
-	unsigned int idx = 0, start, i, j;
+	struct virtnet_stats_ctx ctx = {0};
+	unsigned int idx, start, i, j;
 	const u8 *stats_base;
 	const u64_stats_t *p;
 	size_t offset;
 
+	virtnet_stats_ctx_init(vi, &ctx, data);
+	virtnet_get_hw_stats(vi, &ctx);
+
+	idx = ctx.num_cq;
+
 	for (i = 0; i < vi->curr_queue_pairs; i++) {
 		struct receive_queue *rq = &vi->rq[i];
 
@@ -3215,6 +3559,7 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
 			}
 		} while (u64_stats_fetch_retry(&rq->stats.syncp, start));
 		idx += VIRTNET_RQ_STATS_LEN;
+		idx += ctx.num_rx;
 	}
 
 	for (i = 0; i < vi->curr_queue_pairs; i++) {
@@ -3230,6 +3575,7 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
 			}
 		} while (u64_stats_fetch_retry(&sq->stats.syncp, start));
 		idx += VIRTNET_SQ_STATS_LEN;
+		idx += ctx.num_tx;
 	}
 }
 
@@ -4760,7 +5106,7 @@ static struct virtio_device_id id_table[] = {
 	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
 	VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT, VIRTIO_NET_F_NOTF_COAL, \
 	VIRTIO_NET_F_VQ_NOTF_COAL, \
-	VIRTIO_NET_F_GUEST_HDRLEN
+	VIRTIO_NET_F_GUEST_HDRLEN, VIRTIO_NET_F_DEVICE_STATS
 
 static unsigned int features[] = {
 	VIRTNET_FEATURES,
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net-next v1 3/6] virtio_net: support device stats

[Michael S. Tsirkin]

On Tue, Dec 26, 2023 at 03:31:00PM +0800, Xuan Zhuo wrote:
> As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> 
> Virtio-net supports to get the stats from the device by ethtool -S <eth0>.

you mean "make Virtio-net support getting..."

> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  drivers/net/virtio_net.c | 354 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 350 insertions(+), 4 deletions(-)


> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 31b9ead6260d..1f4d9605552f 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -127,6 +127,113 @@ 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(qtype, class, name) \
> +	{#name, offsetof(struct virtio_net_stats_ ## qtype ## _ ## class, qtype ## _ ## name)}
> +
> +static const struct virtnet_stat_desc virtnet_stats_cvq_desc[] = {
> +	{"command_num", offsetof(struct virtio_net_stats_cvq, command_num)},
> +	{"ok_num", offsetof(struct virtio_net_stats_cvq, ok_num)}
> +};

Use named initializers please this is confusing enough.

> +
> +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),
> +
> +	VIRTNET_STATS_DESC(rx, basic, drops),
> +	VIRTNET_STATS_DESC(rx, basic, drop_overruns),
> +};
> +
> +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),
> +
> +	VIRTNET_STATS_DESC(tx, basic, drops),
> +	VIRTNET_STATS_DESC(tx, basic, drop_malformed),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_rx_csum_desc[] = {
> +	VIRTNET_STATS_DESC(rx, csum, csum_valid),
> +	VIRTNET_STATS_DESC(rx, csum, needs_csum),
> +
> +	VIRTNET_STATS_DESC(rx, csum, csum_none),
> +	VIRTNET_STATS_DESC(rx, csum, csum_bad),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_tx_csum_desc[] = {
> +	VIRTNET_STATS_DESC(tx, csum, needs_csum),
> +	VIRTNET_STATS_DESC(tx, csum, csum_none),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_rx_gso_desc[] = {
> +	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),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_tx_gso_desc[] = {
> +	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(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, packets_allowance_exceeded),
> +	VIRTNET_STATS_DESC(rx, speed, bytes_allowance_exceeded),
> +};
> +
> +static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc[] = {
> +	VIRTNET_STATS_DESC(tx, speed, packets_allowance_exceeded),
> +	VIRTNET_STATS_DESC(tx, speed, packets_allowance_exceeded),
> +};
> +
> +struct virtnet_stats_map {

This seems to be somehow trying to do a table driven design?
All this effort does not seem to have resulted in less or
cleaner code.

Keep it simple: for each structure add a function decoding from
UAPI format to ethtool format. No offsetof games nothing.




> +	u64 flag;

is it a single flag rly?

> +	u32 len;

len here means "bytes"?

> +	u32 num;
> +
> +#define VIRTNET_STATS_Q_TYPE_RX 0
> +#define VIRTNET_STATS_Q_TYPE_TX 1
> +#define VIRTNET_STATS_Q_TYPE_CQ 2
> +	u32 queue_type;
> +
> +	u8 type;
> +	const struct virtnet_stat_desc *desc;
> +};
> +
> +#define VIRTNET_DEVICE_STATS_MAP_ITEM(TYPE, type, queue_type)	\
> +	{							\
> +		VIRTIO_NET_STATS_TYPE_##TYPE,			\
> +		sizeof(struct virtio_net_stats_ ## type),	\
> +		ARRAY_SIZE(virtnet_stats_ ## type ##_desc),	\
> +		VIRTNET_STATS_Q_TYPE_##queue_type,		\
> +		VIRTIO_NET_STATS_TYPE_REPLY_##TYPE,		\
> +		&virtnet_stats_##type##_desc[0]			\
> +	}
> +
> +static struct virtnet_stats_map virtio_net_stats_map[] = {
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(CVQ, cvq, CQ),
> +
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_BASIC, rx_basic, RX),
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_CSUM,  rx_csum,  RX),
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_GSO,   rx_gso,   RX),
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_SPEED, rx_speed, RX),
> +
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_BASIC, tx_basic, TX),
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_CSUM,  tx_csum,  TX),
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_GSO,   tx_gso,   TX),
> +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_SPEED, tx_speed, TX),
> +};
> +
>  struct virtnet_interrupt_coalesce {
>  	u32 max_packets;
>  	u32 max_usecs;
> @@ -232,6 +339,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 {
> @@ -314,6 +422,8 @@ struct virtnet_info {
>  
>  	/* failover when STANDBY feature enabled */
>  	struct failover *failover;
> +
> +	u64 device_stats_cap;
>  };
>  
>  struct padded_vnet_hdr {
> @@ -3157,6 +3267,204 @@ static int virtnet_set_channels(struct net_device *dev,
>  	return err;
>  }
>  
> +static void virtnet_get_hw_stats_string(struct virtnet_info *vi, int type, int qid, u8 **data)
> +{
> +	struct virtnet_stats_map *m;
> +	int i, j;
> +	u8 *p = *data;
> +
> +	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
> +		m = &virtio_net_stats_map[i];
> +
> +		if (m->queue_type != type)
> +			continue;
> +
> +		if (!(vi->device_stats_cap & m->flag))
> +			continue;
> +
> +		for (j = 0; j < m->num; ++j) {
> +			if (type == VIRTNET_STATS_Q_TYPE_RX)
> +				ethtool_sprintf(&p, "rx_queue_hw_%u_%s", qid, m->desc[j].desc);
> +
> +			else if (type == VIRTNET_STATS_Q_TYPE_TX)
> +				ethtool_sprintf(&p, "tx_queue_hw_%u_%s", qid, m->desc[j].desc);
> +
> +			else if (type == VIRTNET_STATS_Q_TYPE_CQ)
> +				ethtool_sprintf(&p, "cq_hw_%s", m->desc[j].desc);

what is this XX_queue_hw_ cuteness? I don't see other drivers
doing this.

> +		}
> +	}
> +
> +	*data = p;
> +}
> +
> +struct virtnet_stats_ctx {

what is this struct? I don't understand the need for this one either.
If you want a union of UAPI types just do that.


> +	u32 num_cq;
> +	u32 num_rx;
> +	u32 num_tx;
> +
> +	u64 bitmap_cq;
> +	u64 bitmap_rx;
> +	u64 bitmap_tx;

Can't this be an array indexed by type?

> +
> +	u32 size_cq;
> +	u32 size_rx;
> +	u32 size_tx;

Using "size" to mean "bytes" is weird.

> +
> +	u64 *data;
> +};
> +
> +static void virtnet_stats_ctx_init(struct virtnet_info *vi,
> +				   struct virtnet_stats_ctx *ctx,
> +				   u64 *data)
> +{
> +	struct virtnet_stats_map *m;
> +	int i;
> +
> +	ctx->data = data;
> +
> +	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
> +		m = &virtio_net_stats_map[i];
> +
> +		if (vi->device_stats_cap & m->flag) {
> +			if (m->queue_type == VIRTNET_STATS_Q_TYPE_CQ) {
> +				ctx->bitmap_cq |= m->flag;
> +				ctx->num_cq += m->num;
> +				ctx->size_cq += m->len;
> +			}
> +
> +			if (m->queue_type == VIRTNET_STATS_Q_TYPE_RX) {
> +				ctx->bitmap_rx |= m->flag;
> +				ctx->num_rx += m->num;
> +				ctx->size_rx += m->len;
> +			}
> +
> +			if (m->queue_type == VIRTNET_STATS_Q_TYPE_TX) {
> +				ctx->bitmap_tx |= m->flag;
> +				ctx->num_tx += m->num;
> +				ctx->size_tx += m->len;
> +			}
> +		}
> +	}
> +}
> +
> +static int virtnet_get_hw_stats(struct virtnet_info *vi,
> +				struct virtnet_stats_ctx *ctx)
> +{
> +	struct virtio_net_ctrl_queue_stats *req;
> +	struct virtio_net_stats_reply_hdr *hdr;
> +	struct scatterlist sgs_in, sgs_out;
> +	u32 num_rx, num_tx, num_cq, offset;
> +	int qnum, i, j,  qid, res_size;
> +	struct virtnet_stats_map *m;
> +	void *reply, *p;
> +	u64 bitmap;
> +	int ok;
> +	u64 *v;
> +
> +	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
> +		return 0;
> +
> +	qnum = 0;
> +	if (ctx->bitmap_cq)
> +		qnum += 1;
> +
> +	if (ctx->bitmap_rx)
> +		qnum += vi->curr_queue_pairs;
> +
> +	if (ctx->bitmap_tx)
> +		qnum += vi->curr_queue_pairs;
> +
> +	req = kcalloc(qnum, sizeof(*req), GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	res_size = (ctx->size_rx + ctx->size_tx) * vi->curr_queue_pairs + ctx->size_cq;
> +	reply = kmalloc(res_size, GFP_KERNEL);
> +	if (!reply) {
> +		kfree(req);
> +		return -ENOMEM;
> +	}
> +
> +	j = 0;
> +	for (i = 0; i < vi->curr_queue_pairs; ++i) {
> +		if (ctx->bitmap_rx) {
> +			req->stats[j].vq_index = cpu_to_le16(i * 2);
> +			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_rx);
> +			++j;
> +		}
> +
> +		if (ctx->bitmap_tx) {
> +			req->stats[j].vq_index = cpu_to_le16(i * 2 + 1);
> +			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_tx);
> +			++j;
> +		}
> +	}
> +
> +	if (ctx->size_cq) {
> +		req->stats[j].vq_index = cpu_to_le16(vi->max_queue_pairs * 2);
> +		req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_cq);
> +		++j;
> +	}
> +
> +	sg_init_one(&sgs_out, req, sizeof(*req) * j);
> +	sg_init_one(&sgs_in, reply, res_size);
> +
> +	ok = virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
> +				  VIRTIO_NET_CTRL_STATS_GET,
> +				  &sgs_out, &sgs_in);
> +	kfree(req);
> +
> +	if (!ok) {
> +		kfree(reply);
> +		return ok;
> +	}
> +
> +	num_rx = VIRTNET_RQ_STATS_LEN + ctx->num_rx;
> +	num_tx = VIRTNET_SQ_STATS_LEN + ctx->num_tx;
> +	num_cq = ctx->num_tx;
> +
> +	for (p = reply; p - reply < res_size; p += virtio16_to_cpu(vi->vdev, hdr->size)) {
> +		hdr = p;
> +
> +		qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
> +
> +		if (qid == vi->max_queue_pairs * 2) {
> +			offset = 0;
> +			bitmap = ctx->bitmap_cq;
> +		} else if (qid % 2) {
> +			offset = num_cq + num_rx * vi->curr_queue_pairs + num_tx * (qid / 2);
> +			offset += VIRTNET_SQ_STATS_LEN;
> +			bitmap = ctx->bitmap_tx;
> +		} else {
> +			offset = num_cq + num_rx * (qid / 2) + VIRTNET_RQ_STATS_LEN;
> +			bitmap = ctx->bitmap_rx;
> +		}
> +
> +		for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
> +			m = &virtio_net_stats_map[i];
> +
> +			if (m->flag & bitmap)
> +				offset += m->num;
> +
> +			if (hdr->type != m->type)
> +				continue;
> +
> +			for (j = 0; j < m->num; ++j) {
> +				v = p + m->desc[j].offset;
> +				ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
> +			}
> +
> +			break;
> +		}
> +	}
> +
> +	kfree(reply);
> +	return 0;
> +}
> +
>  static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
> @@ -3165,16 +3473,22 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
>  
>  	switch (stringset) {
>  	case ETH_SS_STATS:
> +		virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
> +
>  		for (i = 0; i < vi->curr_queue_pairs; i++) {
>  			for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++)
>  				ethtool_sprintf(&p, "rx_queue_%u_%s", i,
>  						virtnet_rq_stats_desc[j].desc);
> +
> +			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, i, &p);
>  		}
>  
>  		for (i = 0; i < vi->curr_queue_pairs; i++) {
>  			for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++)
>  				ethtool_sprintf(&p, "tx_queue_%u_%s", i,
>  						virtnet_sq_stats_desc[j].desc);
> +
> +			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, i, &p);
>  		}
>  		break;
>  	}
> @@ -3183,11 +3497,35 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
>  static int virtnet_get_sset_count(struct net_device *dev, int sset)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
> +	struct virtnet_stats_ctx ctx = {0};
> +	u32 pair_count;
>  
>  	switch (sset) {
>  	case ETH_SS_STATS:
> -		return vi->curr_queue_pairs * (VIRTNET_RQ_STATS_LEN +
> -					       VIRTNET_SQ_STATS_LEN);
> +		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS) &&
> +		    !vi->device_stats_cap) {
> +			struct scatterlist sg;
> +
> +			sg_init_one(&sg, &vi->ctrl->stats_cap, sizeof(vi->ctrl->stats_cap));
> +
> +			if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
> +						  VIRTIO_NET_CTRL_STATS_QUERY,
> +						  NULL, &sg)) {
> +				dev_warn(&dev->dev, "Fail to get stats capability\n");
> +			} else {
> +				__le64 v;
> +
> +				v = vi->ctrl->stats_cap.supported_stats_types[0];
> +				vi->device_stats_cap = virtio64_to_cpu(vi->vdev, v);
> +			}
> +		}
> +
> +		virtnet_stats_ctx_init(vi, &ctx, NULL);
> +
> +		pair_count = VIRTNET_RQ_STATS_LEN + VIRTNET_SQ_STATS_LEN;
> +		pair_count += ctx.num_rx + ctx.num_tx;
> +
> +		return ctx.num_cq + vi->curr_queue_pairs * pair_count;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -3197,11 +3535,17 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
>  				      struct ethtool_stats *stats, u64 *data)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
> -	unsigned int idx = 0, start, i, j;
> +	struct virtnet_stats_ctx ctx = {0};
> +	unsigned int idx, start, i, j;
>  	const u8 *stats_base;
>  	const u64_stats_t *p;
>  	size_t offset;
>  
> +	virtnet_stats_ctx_init(vi, &ctx, data);
> +	virtnet_get_hw_stats(vi, &ctx);
> +
> +	idx = ctx.num_cq;
> +
>  	for (i = 0; i < vi->curr_queue_pairs; i++) {
>  		struct receive_queue *rq = &vi->rq[i];
>  
> @@ -3215,6 +3559,7 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
>  			}
>  		} while (u64_stats_fetch_retry(&rq->stats.syncp, start));
>  		idx += VIRTNET_RQ_STATS_LEN;
> +		idx += ctx.num_rx;
>  	}
>  
>  	for (i = 0; i < vi->curr_queue_pairs; i++) {
> @@ -3230,6 +3575,7 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
>  			}
>  		} while (u64_stats_fetch_retry(&sq->stats.syncp, start));
>  		idx += VIRTNET_SQ_STATS_LEN;
> +		idx += ctx.num_tx;
>  	}
>  }
>  
> @@ -4760,7 +5106,7 @@ static struct virtio_device_id id_table[] = {
>  	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
>  	VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT, VIRTIO_NET_F_NOTF_COAL, \
>  	VIRTIO_NET_F_VQ_NOTF_COAL, \
> -	VIRTIO_NET_F_GUEST_HDRLEN
> +	VIRTIO_NET_F_GUEST_HDRLEN, VIRTIO_NET_F_DEVICE_STATS
>  
>  static unsigned int features[] = {
>  	VIRTNET_FEATURES,
> -- 
> 2.32.0.3.g01195cf9f

Re: [PATCH net-next v1 3/6] virtio_net: support device stats

[Xuan Zhuo]

On Wed, 27 Dec 2023 16:08:47 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 26, 2023 at 03:31:00PM +0800, Xuan Zhuo wrote:
> > As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> >
> > Virtio-net supports to get the stats from the device by ethtool -S <eth0>.
>
> you mean "make Virtio-net support getting..."

Not native speaker, thanks, I will fix in next version.


>
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  drivers/net/virtio_net.c | 354 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 350 insertions(+), 4 deletions(-)
>
>
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 31b9ead6260d..1f4d9605552f 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -127,6 +127,113 @@ 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(qtype, class, name) \
> > +	{#name, offsetof(struct virtio_net_stats_ ## qtype ## _ ## class, qtype ## _ ## name)}
> > +
> > +static const struct virtnet_stat_desc virtnet_stats_cvq_desc[] = {
> > +	{"command_num", offsetof(struct virtio_net_stats_cvq, command_num)},
> > +	{"ok_num", offsetof(struct virtio_net_stats_cvq, ok_num)}
> > +};
>
> Use named initializers please this is confusing enough.

Do you mean that I donot use the marco VIRTNET_STATS_DESC?


>
> > +
> > +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),
> > +
> > +	VIRTNET_STATS_DESC(rx, basic, drops),
> > +	VIRTNET_STATS_DESC(rx, basic, drop_overruns),
> > +};
> > +
> > +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),
> > +
> > +	VIRTNET_STATS_DESC(tx, basic, drops),
> > +	VIRTNET_STATS_DESC(tx, basic, drop_malformed),
> > +};
> > +
> > +static const struct virtnet_stat_desc virtnet_stats_rx_csum_desc[] = {
> > +	VIRTNET_STATS_DESC(rx, csum, csum_valid),
> > +	VIRTNET_STATS_DESC(rx, csum, needs_csum),
> > +
> > +	VIRTNET_STATS_DESC(rx, csum, csum_none),
> > +	VIRTNET_STATS_DESC(rx, csum, csum_bad),
> > +};
> > +
> > +static const struct virtnet_stat_desc virtnet_stats_tx_csum_desc[] = {
> > +	VIRTNET_STATS_DESC(tx, csum, needs_csum),
> > +	VIRTNET_STATS_DESC(tx, csum, csum_none),
> > +};
> > +
> > +static const struct virtnet_stat_desc virtnet_stats_rx_gso_desc[] = {
> > +	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),
> > +};
> > +
> > +static const struct virtnet_stat_desc virtnet_stats_tx_gso_desc[] = {
> > +	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(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, packets_allowance_exceeded),
> > +	VIRTNET_STATS_DESC(rx, speed, bytes_allowance_exceeded),
> > +};
> > +
> > +static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc[] = {
> > +	VIRTNET_STATS_DESC(tx, speed, packets_allowance_exceeded),
> > +	VIRTNET_STATS_DESC(tx, speed, packets_allowance_exceeded),
> > +};
> > +
> > +struct virtnet_stats_map {
>
> This seems to be somehow trying to do a table driven design?
> All this effort does not seem to have resulted in less or
> cleaner code.

I think this works.

If we has not a table of all hw stats.
How can we parse the stats from the device?
A huge if-else?


>
> Keep it simple: for each structure add a function decoding from
> UAPI format to ethtool format. No offsetof games nothing.

That will make a mass of small same functions.


>
>
>
>
> > +	u64 flag;
>
> is it a single flag rly?
>
> > +	u32 len;
>
> len here means "bytes"?
>
> > +	u32 num;
> > +
> > +#define VIRTNET_STATS_Q_TYPE_RX 0
> > +#define VIRTNET_STATS_Q_TYPE_TX 1
> > +#define VIRTNET_STATS_Q_TYPE_CQ 2
> > +	u32 queue_type;
> > +
> > +	u8 type;
> > +	const struct virtnet_stat_desc *desc;
> > +};
> > +
> > +#define VIRTNET_DEVICE_STATS_MAP_ITEM(TYPE, type, queue_type)	\
> > +	{							\
> > +		VIRTIO_NET_STATS_TYPE_##TYPE,			\
> > +		sizeof(struct virtio_net_stats_ ## type),	\
> > +		ARRAY_SIZE(virtnet_stats_ ## type ##_desc),	\
> > +		VIRTNET_STATS_Q_TYPE_##queue_type,		\
> > +		VIRTIO_NET_STATS_TYPE_REPLY_##TYPE,		\
> > +		&virtnet_stats_##type##_desc[0]			\
> > +	}
> > +
> > +static struct virtnet_stats_map virtio_net_stats_map[] = {
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(CVQ, cvq, CQ),
> > +
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_BASIC, rx_basic, RX),
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_CSUM,  rx_csum,  RX),
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_GSO,   rx_gso,   RX),
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_SPEED, rx_speed, RX),
> > +
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_BASIC, tx_basic, TX),
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_CSUM,  tx_csum,  TX),
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_GSO,   tx_gso,   TX),
> > +	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_SPEED, tx_speed, TX),
> > +};
> > +
> >  struct virtnet_interrupt_coalesce {
> >  	u32 max_packets;
> >  	u32 max_usecs;
> > @@ -232,6 +339,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 {
> > @@ -314,6 +422,8 @@ struct virtnet_info {
> >
> >  	/* failover when STANDBY feature enabled */
> >  	struct failover *failover;
> > +
> > +	u64 device_stats_cap;
> >  };
> >
> >  struct padded_vnet_hdr {
> > @@ -3157,6 +3267,204 @@ static int virtnet_set_channels(struct net_device *dev,
> >  	return err;
> >  }
> >
> > +static void virtnet_get_hw_stats_string(struct virtnet_info *vi, int type, int qid, u8 **data)
> > +{
> > +	struct virtnet_stats_map *m;
> > +	int i, j;
> > +	u8 *p = *data;
> > +
> > +	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
> > +		return;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
> > +		m = &virtio_net_stats_map[i];
> > +
> > +		if (m->queue_type != type)
> > +			continue;
> > +
> > +		if (!(vi->device_stats_cap & m->flag))
> > +			continue;
> > +
> > +		for (j = 0; j < m->num; ++j) {
> > +			if (type == VIRTNET_STATS_Q_TYPE_RX)
> > +				ethtool_sprintf(&p, "rx_queue_hw_%u_%s", qid, m->desc[j].desc);
> > +
> > +			else if (type == VIRTNET_STATS_Q_TYPE_TX)
> > +				ethtool_sprintf(&p, "tx_queue_hw_%u_%s", qid, m->desc[j].desc);
> > +
> > +			else if (type == VIRTNET_STATS_Q_TYPE_CQ)
> > +				ethtool_sprintf(&p, "cq_hw_%s", m->desc[j].desc);
>
> what is this XX_queue_hw_ cuteness? I don't see other drivers
> doing this.

We has "bytes", "packets" from the driver and the same from the device.
I want to distinguish that.





>
> > +		}
> > +	}
> > +
> > +	*data = p;
> > +}
> > +
> > +struct virtnet_stats_ctx {
>
> what is this struct? I don't understand the need for this one either.
> If you want a union of UAPI types just do that.
>
>
> > +	u32 num_cq;
> > +	u32 num_rx;
> > +	u32 num_tx;
> > +
> > +	u64 bitmap_cq;
> > +	u64 bitmap_rx;
> > +	u64 bitmap_tx;
>
> Can't this be an array indexed by type?

Yes, It can be  array. But I like ctx.bitmap_tx no ctx.bitmaps[TX].
I think ctx.bitmap_tx is more convenient.



>
> > +
> > +	u32 size_cq;
> > +	u32 size_rx;
> > +	u32 size_tx;
>
> Using "size" to mean "bytes" is weird.


YES. bytes is better than size here.

Thanks


>
> > +
> > +	u64 *data;
> > +};
> > +
> > +static void virtnet_stats_ctx_init(struct virtnet_info *vi,
> > +				   struct virtnet_stats_ctx *ctx,
> > +				   u64 *data)
> > +{
> > +	struct virtnet_stats_map *m;
> > +	int i;
> > +
> > +	ctx->data = data;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
> > +		m = &virtio_net_stats_map[i];
> > +
> > +		if (vi->device_stats_cap & m->flag) {
> > +			if (m->queue_type == VIRTNET_STATS_Q_TYPE_CQ) {
> > +				ctx->bitmap_cq |= m->flag;
> > +				ctx->num_cq += m->num;
> > +				ctx->size_cq += m->len;
> > +			}
> > +
> > +			if (m->queue_type == VIRTNET_STATS_Q_TYPE_RX) {
> > +				ctx->bitmap_rx |= m->flag;
> > +				ctx->num_rx += m->num;
> > +				ctx->size_rx += m->len;
> > +			}
> > +
> > +			if (m->queue_type == VIRTNET_STATS_Q_TYPE_TX) {
> > +				ctx->bitmap_tx |= m->flag;
> > +				ctx->num_tx += m->num;
> > +				ctx->size_tx += m->len;
> > +			}
> > +		}
> > +	}
> > +}
> > +
> > +static int virtnet_get_hw_stats(struct virtnet_info *vi,
> > +				struct virtnet_stats_ctx *ctx)
> > +{
> > +	struct virtio_net_ctrl_queue_stats *req;
> > +	struct virtio_net_stats_reply_hdr *hdr;
> > +	struct scatterlist sgs_in, sgs_out;
> > +	u32 num_rx, num_tx, num_cq, offset;
> > +	int qnum, i, j,  qid, res_size;
> > +	struct virtnet_stats_map *m;
> > +	void *reply, *p;
> > +	u64 bitmap;
> > +	int ok;
> > +	u64 *v;
> > +
> > +	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
> > +		return 0;
> > +
> > +	qnum = 0;
> > +	if (ctx->bitmap_cq)
> > +		qnum += 1;
> > +
> > +	if (ctx->bitmap_rx)
> > +		qnum += vi->curr_queue_pairs;
> > +
> > +	if (ctx->bitmap_tx)
> > +		qnum += vi->curr_queue_pairs;
> > +
> > +	req = kcalloc(qnum, sizeof(*req), GFP_KERNEL);
> > +	if (!req)
> > +		return -ENOMEM;
> > +
> > +	res_size = (ctx->size_rx + ctx->size_tx) * vi->curr_queue_pairs + ctx->size_cq;
> > +	reply = kmalloc(res_size, GFP_KERNEL);
> > +	if (!reply) {
> > +		kfree(req);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	j = 0;
> > +	for (i = 0; i < vi->curr_queue_pairs; ++i) {
> > +		if (ctx->bitmap_rx) {
> > +			req->stats[j].vq_index = cpu_to_le16(i * 2);
> > +			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_rx);
> > +			++j;
> > +		}
> > +
> > +		if (ctx->bitmap_tx) {
> > +			req->stats[j].vq_index = cpu_to_le16(i * 2 + 1);
> > +			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_tx);
> > +			++j;
> > +		}
> > +	}
> > +
> > +	if (ctx->size_cq) {
> > +		req->stats[j].vq_index = cpu_to_le16(vi->max_queue_pairs * 2);
> > +		req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_cq);
> > +		++j;
> > +	}
> > +
> > +	sg_init_one(&sgs_out, req, sizeof(*req) * j);
> > +	sg_init_one(&sgs_in, reply, res_size);
> > +
> > +	ok = virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
> > +				  VIRTIO_NET_CTRL_STATS_GET,
> > +				  &sgs_out, &sgs_in);
> > +	kfree(req);
> > +
> > +	if (!ok) {
> > +		kfree(reply);
> > +		return ok;
> > +	}
> > +
> > +	num_rx = VIRTNET_RQ_STATS_LEN + ctx->num_rx;
> > +	num_tx = VIRTNET_SQ_STATS_LEN + ctx->num_tx;
> > +	num_cq = ctx->num_tx;
> > +
> > +	for (p = reply; p - reply < res_size; p += virtio16_to_cpu(vi->vdev, hdr->size)) {
> > +		hdr = p;
> > +
> > +		qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
> > +
> > +		if (qid == vi->max_queue_pairs * 2) {
> > +			offset = 0;
> > +			bitmap = ctx->bitmap_cq;
> > +		} else if (qid % 2) {
> > +			offset = num_cq + num_rx * vi->curr_queue_pairs + num_tx * (qid / 2);
> > +			offset += VIRTNET_SQ_STATS_LEN;
> > +			bitmap = ctx->bitmap_tx;
> > +		} else {
> > +			offset = num_cq + num_rx * (qid / 2) + VIRTNET_RQ_STATS_LEN;
> > +			bitmap = ctx->bitmap_rx;
> > +		}
> > +
> > +		for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
> > +			m = &virtio_net_stats_map[i];
> > +
> > +			if (m->flag & bitmap)
> > +				offset += m->num;
> > +
> > +			if (hdr->type != m->type)
> > +				continue;
> > +
> > +			for (j = 0; j < m->num; ++j) {
> > +				v = p + m->desc[j].offset;
> > +				ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
> > +			}
> > +
> > +			break;
> > +		}
> > +	}
> > +
> > +	kfree(reply);
> > +	return 0;
> > +}
> > +
> >  static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> >  {
> >  	struct virtnet_info *vi = netdev_priv(dev);
> > @@ -3165,16 +3473,22 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> >
> >  	switch (stringset) {
> >  	case ETH_SS_STATS:
> > +		virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
> > +
> >  		for (i = 0; i < vi->curr_queue_pairs; i++) {
> >  			for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++)
> >  				ethtool_sprintf(&p, "rx_queue_%u_%s", i,
> >  						virtnet_rq_stats_desc[j].desc);
> > +
> > +			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, i, &p);
> >  		}
> >
> >  		for (i = 0; i < vi->curr_queue_pairs; i++) {
> >  			for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++)
> >  				ethtool_sprintf(&p, "tx_queue_%u_%s", i,
> >  						virtnet_sq_stats_desc[j].desc);
> > +
> > +			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, i, &p);
> >  		}
> >  		break;
> >  	}
> > @@ -3183,11 +3497,35 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> >  static int virtnet_get_sset_count(struct net_device *dev, int sset)
> >  {
> >  	struct virtnet_info *vi = netdev_priv(dev);
> > +	struct virtnet_stats_ctx ctx = {0};
> > +	u32 pair_count;
> >
> >  	switch (sset) {
> >  	case ETH_SS_STATS:
> > -		return vi->curr_queue_pairs * (VIRTNET_RQ_STATS_LEN +
> > -					       VIRTNET_SQ_STATS_LEN);
> > +		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS) &&
> > +		    !vi->device_stats_cap) {
> > +			struct scatterlist sg;
> > +
> > +			sg_init_one(&sg, &vi->ctrl->stats_cap, sizeof(vi->ctrl->stats_cap));
> > +
> > +			if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
> > +						  VIRTIO_NET_CTRL_STATS_QUERY,
> > +						  NULL, &sg)) {
> > +				dev_warn(&dev->dev, "Fail to get stats capability\n");
> > +			} else {
> > +				__le64 v;
> > +
> > +				v = vi->ctrl->stats_cap.supported_stats_types[0];
> > +				vi->device_stats_cap = virtio64_to_cpu(vi->vdev, v);
> > +			}
> > +		}
> > +
> > +		virtnet_stats_ctx_init(vi, &ctx, NULL);
> > +
> > +		pair_count = VIRTNET_RQ_STATS_LEN + VIRTNET_SQ_STATS_LEN;
> > +		pair_count += ctx.num_rx + ctx.num_tx;
> > +
> > +		return ctx.num_cq + vi->curr_queue_pairs * pair_count;
> >  	default:
> >  		return -EOPNOTSUPP;
> >  	}
> > @@ -3197,11 +3535,17 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
> >  				      struct ethtool_stats *stats, u64 *data)
> >  {
> >  	struct virtnet_info *vi = netdev_priv(dev);
> > -	unsigned int idx = 0, start, i, j;
> > +	struct virtnet_stats_ctx ctx = {0};
> > +	unsigned int idx, start, i, j;
> >  	const u8 *stats_base;
> >  	const u64_stats_t *p;
> >  	size_t offset;
> >
> > +	virtnet_stats_ctx_init(vi, &ctx, data);
> > +	virtnet_get_hw_stats(vi, &ctx);
> > +
> > +	idx = ctx.num_cq;
> > +
> >  	for (i = 0; i < vi->curr_queue_pairs; i++) {
> >  		struct receive_queue *rq = &vi->rq[i];
> >
> > @@ -3215,6 +3559,7 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
> >  			}
> >  		} while (u64_stats_fetch_retry(&rq->stats.syncp, start));
> >  		idx += VIRTNET_RQ_STATS_LEN;
> > +		idx += ctx.num_rx;
> >  	}
> >
> >  	for (i = 0; i < vi->curr_queue_pairs; i++) {
> > @@ -3230,6 +3575,7 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
> >  			}
> >  		} while (u64_stats_fetch_retry(&sq->stats.syncp, start));
> >  		idx += VIRTNET_SQ_STATS_LEN;
> > +		idx += ctx.num_tx;
> >  	}
> >  }
> >
> > @@ -4760,7 +5106,7 @@ static struct virtio_device_id id_table[] = {
> >  	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> >  	VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT, VIRTIO_NET_F_NOTF_COAL, \
> >  	VIRTIO_NET_F_VQ_NOTF_COAL, \
> > -	VIRTIO_NET_F_GUEST_HDRLEN
> > +	VIRTIO_NET_F_GUEST_HDRLEN, VIRTIO_NET_F_DEVICE_STATS
> >
> >  static unsigned int features[] = {
> >  	VIRTNET_FEATURES,
> > --
> > 2.32.0.3.g01195cf9f
>

Re: [PATCH net-next v1 3/6] virtio_net: support device stats

[kernel test robot]

Hi Xuan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.7-rc8]
[cannot apply to net-next/main horms-ipvs/master next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xuan-Zhuo/virtio_net-introduce-device-stats-feature-and-structures/20231226-153227
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20231226073103.116153-4-xuanzhuo%40linux.alibaba.com
patch subject: [PATCH net-next v1 3/6] virtio_net: support device stats
config: x86_64-randconfig-121-20240101 (https://download.01.org/0day-ci/archive/20240102/202401020308.rvzTx1oI-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240102/202401020308.rvzTx1oI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401020308.rvzTx1oI-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/virtio_net.c:3432:52: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio16 [usertype] val @@     got restricted __le16 [usertype] vq_index @@
   drivers/net/virtio_net.c:3432:52: sparse:     expected restricted __virtio16 [usertype] val
   drivers/net/virtio_net.c:3432:52: sparse:     got restricted __le16 [usertype] vq_index
   drivers/net/virtio_net.c:3457:83: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio64 [usertype] val @@     got unsigned long long [usertype] @@
   drivers/net/virtio_net.c:3457:83: sparse:     expected restricted __virtio64 [usertype] val
   drivers/net/virtio_net.c:3457:83: sparse:     got unsigned long long [usertype]
>> drivers/net/virtio_net.c:3429:81: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio16 [usertype] val @@     got restricted __le16 [usertype] size @@
   drivers/net/virtio_net.c:3429:81: sparse:     expected restricted __virtio16 [usertype] val
   drivers/net/virtio_net.c:3429:81: sparse:     got restricted __le16 [usertype] size
>> drivers/net/virtio_net.c:3519:82: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio64 [usertype] val @@     got restricted __le64 [assigned] [usertype] v @@
   drivers/net/virtio_net.c:3519:82: sparse:     expected restricted __virtio64 [usertype] val
   drivers/net/virtio_net.c:3519:82: sparse:     got restricted __le64 [assigned] [usertype] v

vim +3432 drivers/net/virtio_net.c

  3352	
  3353	static int virtnet_get_hw_stats(struct virtnet_info *vi,
  3354					struct virtnet_stats_ctx *ctx)
  3355	{
  3356		struct virtio_net_ctrl_queue_stats *req;
  3357		struct virtio_net_stats_reply_hdr *hdr;
  3358		struct scatterlist sgs_in, sgs_out;
  3359		u32 num_rx, num_tx, num_cq, offset;
  3360		int qnum, i, j,  qid, res_size;
  3361		struct virtnet_stats_map *m;
  3362		void *reply, *p;
  3363		u64 bitmap;
  3364		int ok;
  3365		u64 *v;
  3366	
  3367		if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
  3368			return 0;
  3369	
  3370		qnum = 0;
  3371		if (ctx->bitmap_cq)
  3372			qnum += 1;
  3373	
  3374		if (ctx->bitmap_rx)
  3375			qnum += vi->curr_queue_pairs;
  3376	
  3377		if (ctx->bitmap_tx)
  3378			qnum += vi->curr_queue_pairs;
  3379	
  3380		req = kcalloc(qnum, sizeof(*req), GFP_KERNEL);
  3381		if (!req)
  3382			return -ENOMEM;
  3383	
  3384		res_size = (ctx->size_rx + ctx->size_tx) * vi->curr_queue_pairs + ctx->size_cq;
  3385		reply = kmalloc(res_size, GFP_KERNEL);
  3386		if (!reply) {
  3387			kfree(req);
  3388			return -ENOMEM;
  3389		}
  3390	
  3391		j = 0;
  3392		for (i = 0; i < vi->curr_queue_pairs; ++i) {
  3393			if (ctx->bitmap_rx) {
  3394				req->stats[j].vq_index = cpu_to_le16(i * 2);
  3395				req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_rx);
  3396				++j;
  3397			}
  3398	
  3399			if (ctx->bitmap_tx) {
  3400				req->stats[j].vq_index = cpu_to_le16(i * 2 + 1);
  3401				req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_tx);
  3402				++j;
  3403			}
  3404		}
  3405	
  3406		if (ctx->size_cq) {
  3407			req->stats[j].vq_index = cpu_to_le16(vi->max_queue_pairs * 2);
  3408			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_cq);
  3409			++j;
  3410		}
  3411	
  3412		sg_init_one(&sgs_out, req, sizeof(*req) * j);
  3413		sg_init_one(&sgs_in, reply, res_size);
  3414	
  3415		ok = virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
  3416					  VIRTIO_NET_CTRL_STATS_GET,
  3417					  &sgs_out, &sgs_in);
  3418		kfree(req);
  3419	
  3420		if (!ok) {
  3421			kfree(reply);
  3422			return ok;
  3423		}
  3424	
  3425		num_rx = VIRTNET_RQ_STATS_LEN + ctx->num_rx;
  3426		num_tx = VIRTNET_SQ_STATS_LEN + ctx->num_tx;
  3427		num_cq = ctx->num_tx;
  3428	
> 3429		for (p = reply; p - reply < res_size; p += virtio16_to_cpu(vi->vdev, hdr->size)) {
  3430			hdr = p;
  3431	
> 3432			qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
  3433	
  3434			if (qid == vi->max_queue_pairs * 2) {
  3435				offset = 0;
  3436				bitmap = ctx->bitmap_cq;
  3437			} else if (qid % 2) {
  3438				offset = num_cq + num_rx * vi->curr_queue_pairs + num_tx * (qid / 2);
  3439				offset += VIRTNET_SQ_STATS_LEN;
  3440				bitmap = ctx->bitmap_tx;
  3441			} else {
  3442				offset = num_cq + num_rx * (qid / 2) + VIRTNET_RQ_STATS_LEN;
  3443				bitmap = ctx->bitmap_rx;
  3444			}
  3445	
  3446			for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
  3447				m = &virtio_net_stats_map[i];
  3448	
  3449				if (m->flag & bitmap)
  3450					offset += m->num;
  3451	
  3452				if (hdr->type != m->type)
  3453					continue;
  3454	
  3455				for (j = 0; j < m->num; ++j) {
  3456					v = p + m->desc[j].offset;
  3457					ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
  3458				}
  3459	
  3460				break;
  3461			}
  3462		}
  3463	
  3464		kfree(reply);
  3465		return 0;
  3466	}
  3467	
  3468	static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
  3469	{
  3470		struct virtnet_info *vi = netdev_priv(dev);
  3471		unsigned int i, j;
  3472		u8 *p = data;
  3473	
  3474		switch (stringset) {
  3475		case ETH_SS_STATS:
  3476			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
  3477	
  3478			for (i = 0; i < vi->curr_queue_pairs; i++) {
  3479				for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++)
  3480					ethtool_sprintf(&p, "rx_queue_%u_%s", i,
  3481							virtnet_rq_stats_desc[j].desc);
  3482	
  3483				virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, i, &p);
  3484			}
  3485	
  3486			for (i = 0; i < vi->curr_queue_pairs; i++) {
  3487				for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++)
  3488					ethtool_sprintf(&p, "tx_queue_%u_%s", i,
  3489							virtnet_sq_stats_desc[j].desc);
  3490	
  3491				virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, i, &p);
  3492			}
  3493			break;
  3494		}
  3495	}
  3496	
  3497	static int virtnet_get_sset_count(struct net_device *dev, int sset)
  3498	{
  3499		struct virtnet_info *vi = netdev_priv(dev);
  3500		struct virtnet_stats_ctx ctx = {0};
  3501		u32 pair_count;
  3502	
  3503		switch (sset) {
  3504		case ETH_SS_STATS:
  3505			if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS) &&
  3506			    !vi->device_stats_cap) {
  3507				struct scatterlist sg;
  3508	
  3509				sg_init_one(&sg, &vi->ctrl->stats_cap, sizeof(vi->ctrl->stats_cap));
  3510	
  3511				if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
  3512							  VIRTIO_NET_CTRL_STATS_QUERY,
  3513							  NULL, &sg)) {
  3514					dev_warn(&dev->dev, "Fail to get stats capability\n");
  3515				} else {
  3516					__le64 v;
  3517	
  3518					v = vi->ctrl->stats_cap.supported_stats_types[0];
> 3519					vi->device_stats_cap = virtio64_to_cpu(vi->vdev, v);
  3520				}
  3521			}
  3522	
  3523			virtnet_stats_ctx_init(vi, &ctx, NULL);
  3524	
  3525			pair_count = VIRTNET_RQ_STATS_LEN + VIRTNET_SQ_STATS_LEN;
  3526			pair_count += ctx.num_rx + ctx.num_tx;
  3527	
  3528			return ctx.num_cq + vi->curr_queue_pairs * pair_count;
  3529		default:
  3530			return -EOPNOTSUPP;
  3531		}
  3532	}
  3533	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v1 3/6] virtio_net: support device stats

[Zhu Yanjun]

在 2024/1/2 3:56, kernel test robot 写道:
> Hi Xuan,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on mst-vhost/linux-next]
> [also build test WARNING on linus/master v6.7-rc8]
> [cannot apply to net-next/main horms-ipvs/master next-20231222]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Xuan-Zhuo/virtio_net-introduce-device-stats-feature-and-structures/20231226-153227
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
> patch link:    https://lore.kernel.org/r/20231226073103.116153-4-xuanzhuo%40linux.alibaba.com
> patch subject: [PATCH net-next v1 3/6] virtio_net: support device stats
> config: x86_64-randconfig-121-20240101 (https://download.01.org/0day-ci/archive/20240102/202401020308.rvzTx1oI-lkp@intel.com/config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240102/202401020308.rvzTx1oI-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202401020308.rvzTx1oI-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
>>> drivers/net/virtio_net.c:3432:52: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio16 [usertype] val @@     got restricted __le16 [usertype] vq_index @@
>     drivers/net/virtio_net.c:3432:52: sparse:     expected restricted __virtio16 [usertype] val
>     drivers/net/virtio_net.c:3432:52: sparse:     got restricted __le16 [usertype] vq_index
>     drivers/net/virtio_net.c:3457:83: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio64 [usertype] val @@     got unsigned long long [usertype] @@
>     drivers/net/virtio_net.c:3457:83: sparse:     expected restricted __virtio64 [usertype] val
>     drivers/net/virtio_net.c:3457:83: sparse:     got unsigned long long [usertype]
>>> drivers/net/virtio_net.c:3429:81: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio16 [usertype] val @@     got restricted __le16 [usertype] size @@
>     drivers/net/virtio_net.c:3429:81: sparse:     expected restricted __virtio16 [usertype] val
>     drivers/net/virtio_net.c:3429:81: sparse:     got restricted __le16 [usertype] size
>>> drivers/net/virtio_net.c:3519:82: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio64 [usertype] val @@     got restricted __le64 [assigned] [usertype] v @@
>     drivers/net/virtio_net.c:3519:82: sparse:     expected restricted __virtio64 [usertype] val
>     drivers/net/virtio_net.c:3519:82: sparse:     got restricted __le64 [assigned] [usertype] v

I can reproduce these warnings in the local host.

It seems that the followings can fix these warnings.

"

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1f4d9605552f..62e40234e29c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3426,10 +3426,10 @@ static int virtnet_get_hw_stats(struct 
virtnet_info *vi,
         num_tx = VIRTNET_SQ_STATS_LEN + ctx->num_tx;
         num_cq = ctx->num_tx;

-       for (p = reply; p - reply < res_size; p += 
virtio16_to_cpu(vi->vdev, hdr->size)) {
+       for (p = reply; p - reply < res_size; p += 
virtio16_to_cpu(vi->vdev, (__virtio16 __force)hdr->size)) {
                 hdr = p;

-               qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
+               qid = virtio16_to_cpu(vi->vdev, (__virtio16 
__force)(hdr->vq_index));

                 if (qid == vi->max_queue_pairs * 2) {
                         offset = 0;
@@ -3454,7 +3454,7 @@ static int virtnet_get_hw_stats(struct 
virtnet_info *vi,

                         for (j = 0; j < m->num; ++j) {
                                 v = p + m->desc[j].offset;
-                               ctx->data[offset + j] = 
virtio64_to_cpu(vi->vdev, *v);
+                               ctx->data[offset + j] = 
virtio64_to_cpu(vi->vdev, (__virtio64 __force)*v);
                         }

                         break;
@@ -3516,7 +3516,7 @@ static int virtnet_get_sset_count(struct 
net_device *dev, int sset)
                                 __le64 v;

                                 v = 
vi->ctrl->stats_cap.supported_stats_types[0];
-                               vi->device_stats_cap = 
virtio64_to_cpu(vi->vdev, v);
+                               vi->device_stats_cap = 
virtio64_to_cpu(vi->vdev, (__virtio64 __force)v);
                         }
                 }

"

Not sure whether the above diff are good to the whole commit or not.

Zhu Yanjun

>
> vim +3432 drivers/net/virtio_net.c
>
>    3352	
>    3353	static int virtnet_get_hw_stats(struct virtnet_info *vi,
>    3354					struct virtnet_stats_ctx *ctx)
>    3355	{
>    3356		struct virtio_net_ctrl_queue_stats *req;
>    3357		struct virtio_net_stats_reply_hdr *hdr;
>    3358		struct scatterlist sgs_in, sgs_out;
>    3359		u32 num_rx, num_tx, num_cq, offset;
>    3360		int qnum, i, j,  qid, res_size;
>    3361		struct virtnet_stats_map *m;
>    3362		void *reply, *p;
>    3363		u64 bitmap;
>    3364		int ok;
>    3365		u64 *v;
>    3366	
>    3367		if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
>    3368			return 0;
>    3369	
>    3370		qnum = 0;
>    3371		if (ctx->bitmap_cq)
>    3372			qnum += 1;
>    3373	
>    3374		if (ctx->bitmap_rx)
>    3375			qnum += vi->curr_queue_pairs;
>    3376	
>    3377		if (ctx->bitmap_tx)
>    3378			qnum += vi->curr_queue_pairs;
>    3379	
>    3380		req = kcalloc(qnum, sizeof(*req), GFP_KERNEL);
>    3381		if (!req)
>    3382			return -ENOMEM;
>    3383	
>    3384		res_size = (ctx->size_rx + ctx->size_tx) * vi->curr_queue_pairs + ctx->size_cq;
>    3385		reply = kmalloc(res_size, GFP_KERNEL);
>    3386		if (!reply) {
>    3387			kfree(req);
>    3388			return -ENOMEM;
>    3389		}
>    3390	
>    3391		j = 0;
>    3392		for (i = 0; i < vi->curr_queue_pairs; ++i) {
>    3393			if (ctx->bitmap_rx) {
>    3394				req->stats[j].vq_index = cpu_to_le16(i * 2);
>    3395				req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_rx);
>    3396				++j;
>    3397			}
>    3398	
>    3399			if (ctx->bitmap_tx) {
>    3400				req->stats[j].vq_index = cpu_to_le16(i * 2 + 1);
>    3401				req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_tx);
>    3402				++j;
>    3403			}
>    3404		}
>    3405	
>    3406		if (ctx->size_cq) {
>    3407			req->stats[j].vq_index = cpu_to_le16(vi->max_queue_pairs * 2);
>    3408			req->stats[j].types_bitmap[0] = cpu_to_le64(ctx->bitmap_cq);
>    3409			++j;
>    3410		}
>    3411	
>    3412		sg_init_one(&sgs_out, req, sizeof(*req) * j);
>    3413		sg_init_one(&sgs_in, reply, res_size);
>    3414	
>    3415		ok = virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
>    3416					  VIRTIO_NET_CTRL_STATS_GET,
>    3417					  &sgs_out, &sgs_in);
>    3418		kfree(req);
>    3419	
>    3420		if (!ok) {
>    3421			kfree(reply);
>    3422			return ok;
>    3423		}
>    3424	
>    3425		num_rx = VIRTNET_RQ_STATS_LEN + ctx->num_rx;
>    3426		num_tx = VIRTNET_SQ_STATS_LEN + ctx->num_tx;
>    3427		num_cq = ctx->num_tx;
>    3428	
>> 3429		for (p = reply; p - reply < res_size; p += virtio16_to_cpu(vi->vdev, hdr->size)) {
>    3430			hdr = p;
>    3431	
>> 3432			qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
>    3433	
>    3434			if (qid == vi->max_queue_pairs * 2) {
>    3435				offset = 0;
>    3436				bitmap = ctx->bitmap_cq;
>    3437			} else if (qid % 2) {
>    3438				offset = num_cq + num_rx * vi->curr_queue_pairs + num_tx * (qid / 2);
>    3439				offset += VIRTNET_SQ_STATS_LEN;
>    3440				bitmap = ctx->bitmap_tx;
>    3441			} else {
>    3442				offset = num_cq + num_rx * (qid / 2) + VIRTNET_RQ_STATS_LEN;
>    3443				bitmap = ctx->bitmap_rx;
>    3444			}
>    3445	
>    3446			for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
>    3447				m = &virtio_net_stats_map[i];
>    3448	
>    3449				if (m->flag & bitmap)
>    3450					offset += m->num;
>    3451	
>    3452				if (hdr->type != m->type)
>    3453					continue;
>    3454	
>    3455				for (j = 0; j < m->num; ++j) {
>    3456					v = p + m->desc[j].offset;
>    3457					ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
>    3458				}
>    3459	
>    3460				break;
>    3461			}
>    3462		}
>    3463	
>    3464		kfree(reply);
>    3465		return 0;
>    3466	}
>    3467	
>    3468	static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
>    3469	{
>    3470		struct virtnet_info *vi = netdev_priv(dev);
>    3471		unsigned int i, j;
>    3472		u8 *p = data;
>    3473	
>    3474		switch (stringset) {
>    3475		case ETH_SS_STATS:
>    3476			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
>    3477	
>    3478			for (i = 0; i < vi->curr_queue_pairs; i++) {
>    3479				for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++)
>    3480					ethtool_sprintf(&p, "rx_queue_%u_%s", i,
>    3481							virtnet_rq_stats_desc[j].desc);
>    3482	
>    3483				virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, i, &p);
>    3484			}
>    3485	
>    3486			for (i = 0; i < vi->curr_queue_pairs; i++) {
>    3487				for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++)
>    3488					ethtool_sprintf(&p, "tx_queue_%u_%s", i,
>    3489							virtnet_sq_stats_desc[j].desc);
>    3490	
>    3491				virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, i, &p);
>    3492			}
>    3493			break;
>    3494		}
>    3495	}
>    3496	
>    3497	static int virtnet_get_sset_count(struct net_device *dev, int sset)
>    3498	{
>    3499		struct virtnet_info *vi = netdev_priv(dev);
>    3500		struct virtnet_stats_ctx ctx = {0};
>    3501		u32 pair_count;
>    3502	
>    3503		switch (sset) {
>    3504		case ETH_SS_STATS:
>    3505			if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS) &&
>    3506			    !vi->device_stats_cap) {
>    3507				struct scatterlist sg;
>    3508	
>    3509				sg_init_one(&sg, &vi->ctrl->stats_cap, sizeof(vi->ctrl->stats_cap));
>    3510	
>    3511				if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_STATS,
>    3512							  VIRTIO_NET_CTRL_STATS_QUERY,
>    3513							  NULL, &sg)) {
>    3514					dev_warn(&dev->dev, "Fail to get stats capability\n");
>    3515				} else {
>    3516					__le64 v;
>    3517	
>    3518					v = vi->ctrl->stats_cap.supported_stats_types[0];
>> 3519					vi->device_stats_cap = virtio64_to_cpu(vi->vdev, v);
>    3520				}
>    3521			}
>    3522	
>    3523			virtnet_stats_ctx_init(vi, &ctx, NULL);
>    3524	
>    3525			pair_count = VIRTNET_RQ_STATS_LEN + VIRTNET_SQ_STATS_LEN;
>    3526			pair_count += ctx.num_rx + ctx.num_tx;
>    3527	
>    3528			return ctx.num_cq + vi->curr_queue_pairs * pair_count;
>    3529		default:
>    3530			return -EOPNOTSUPP;
>    3531		}
>    3532	}
>    3533	
>

[PATCH net-next v1 4/6] virtio_net: stats map include driver stats

[Xuan Zhuo]

In the last commit, we use the stats map to manage the device stats.

For the consistency, we let the stats map includes the driver stats.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 194 ++++++++++++++++++++-------------------
 1 file changed, 102 insertions(+), 92 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1f4d9605552f..75d68795f2bc 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -124,9 +124,6 @@ static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = {
 	{ "kicks",		VIRTNET_RQ_STAT(kicks) },
 };
 
-#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(qtype, class, name) \
 	{#name, offsetof(struct virtio_net_stats_ ## qtype ## _ ## class, qtype ## _ ## name)}
 
@@ -197,8 +194,12 @@ static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc[] = {
 };
 
 struct virtnet_stats_map {
+	/* just for device stats */
 	u64 flag;
+
+	/* just for device stats */
 	u32 len;
+
 	u32 num;
 
 #define VIRTNET_STATS_Q_TYPE_RX 0
@@ -206,7 +207,10 @@ struct virtnet_stats_map {
 #define VIRTNET_STATS_Q_TYPE_CQ 2
 	u32 queue_type;
 
+	/* just for device stats */
 	u8 type;
+
+	u8 from_driver;
 	const struct virtnet_stat_desc *desc;
 };
 
@@ -217,10 +221,24 @@ struct virtnet_stats_map {
 		ARRAY_SIZE(virtnet_stats_ ## type ##_desc),	\
 		VIRTNET_STATS_Q_TYPE_##queue_type,		\
 		VIRTIO_NET_STATS_TYPE_REPLY_##TYPE,		\
+		false, \
 		&virtnet_stats_##type##_desc[0]			\
 	}
 
+#define VIRTNET_DRIVER_STATS_MAP_ITEM(type, queue_type)	\
+	{							\
+		0, 0,	\
+		ARRAY_SIZE(virtnet_ ## type ## _stats_desc),	\
+		VIRTNET_STATS_Q_TYPE_##queue_type,		\
+		0, true, \
+		&virtnet_##type##_stats_desc[0]			\
+	}
+
 static struct virtnet_stats_map virtio_net_stats_map[] = {
+	/* driver stats should on the start. */
+	VIRTNET_DRIVER_STATS_MAP_ITEM(rq, RX),
+	VIRTNET_DRIVER_STATS_MAP_ITEM(sq, TX),
+
 	VIRTNET_DEVICE_STATS_MAP_ITEM(CVQ, cvq, CQ),
 
 	VIRTNET_DEVICE_STATS_MAP_ITEM(RX_BASIC, rx_basic, RX),
@@ -234,6 +252,11 @@ static struct virtnet_stats_map virtio_net_stats_map[] = {
 	VIRTNET_DEVICE_STATS_MAP_ITEM(TX_SPEED, tx_speed, TX),
 };
 
+#define virtnet_stats_valid(vi, m) ({				\
+	typeof(m) _m = (m);					\
+	(((vi)->device_stats_cap & _m->flag) || _m->from_driver);	\
+})
+
 struct virtnet_interrupt_coalesce {
 	u32 max_packets;
 	u32 max_usecs;
@@ -2206,7 +2229,7 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 
 	u64_stats_set(&stats.packets, packets);
 	u64_stats_update_begin(&rq->stats.syncp);
-	for (i = 0; i < VIRTNET_RQ_STATS_LEN; i++) {
+	for (i = 0; i < ARRAY_SIZE(virtnet_rq_stats_desc); i++) {
 		size_t offset = virtnet_rq_stats_desc[i].offset;
 		u64_stats_t *item, *src;
 
@@ -3267,33 +3290,36 @@ static int virtnet_set_channels(struct net_device *dev,
 	return err;
 }
 
-static void virtnet_get_hw_stats_string(struct virtnet_info *vi, int type, int qid, u8 **data)
+static void virtnet_get_stats_string(struct virtnet_info *vi, int type, int qid, u8 **data)
 {
 	struct virtnet_stats_map *m;
+	const char *tp;
 	int i, j;
 	u8 *p = *data;
 
-	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
 		m = &virtio_net_stats_map[i];
 
 		if (m->queue_type != type)
 			continue;
 
-		if (!(vi->device_stats_cap & m->flag))
+		if (!virtnet_stats_valid(vi, m))
 			continue;
 
 		for (j = 0; j < m->num; ++j) {
+			if (m->from_driver)
+				tp = "";
+			else
+				tp = "_hw";
+
 			if (type == VIRTNET_STATS_Q_TYPE_RX)
-				ethtool_sprintf(&p, "rx_queue_hw_%u_%s", qid, m->desc[j].desc);
+				ethtool_sprintf(&p, "rx_queue%s_%u_%s", tp, qid, m->desc[j].desc);
 
 			else if (type == VIRTNET_STATS_Q_TYPE_TX)
-				ethtool_sprintf(&p, "tx_queue_hw_%u_%s", qid, m->desc[j].desc);
+				ethtool_sprintf(&p, "tx_queue%s_%u_%s", tp, qid, m->desc[j].desc);
 
 			else if (type == VIRTNET_STATS_Q_TYPE_CQ)
-				ethtool_sprintf(&p, "cq_hw_%s", m->desc[j].desc);
+				ethtool_sprintf(&p, "cq%s_%s", tp, m->desc[j].desc);
 		}
 	}
 
@@ -3328,7 +3354,7 @@ static void virtnet_stats_ctx_init(struct virtnet_info *vi,
 	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
 		m = &virtio_net_stats_map[i];
 
-		if (vi->device_stats_cap & m->flag) {
+		if (virtnet_stats_valid(vi, m)) {
 			if (m->queue_type == VIRTNET_STATS_Q_TYPE_CQ) {
 				ctx->bitmap_cq |= m->flag;
 				ctx->num_cq += m->num;
@@ -3350,19 +3376,66 @@ static void virtnet_stats_ctx_init(struct virtnet_info *vi,
 	}
 }
 
+static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid,
+			       struct virtnet_stats_ctx *ctx,
+			       const u8 *base, bool from_driver, u8 type)
+{
+	struct virtnet_stats_map *m;
+	const u64_stats_t *v_stat;
+	u32 queue_type;
+	const u64 *v;
+	u64 offset;
+	int i, j;
+
+	if (qid == vi->max_queue_pairs * 2) {
+		offset = 0;
+		queue_type = VIRTNET_STATS_Q_TYPE_CQ;
+	} else if (qid % 2) {
+		offset = ctx->num_cq + ctx->num_rx * vi->curr_queue_pairs + ctx->num_tx * (qid / 2);
+		queue_type = VIRTNET_STATS_Q_TYPE_TX;
+	} else {
+		offset = ctx->num_cq + ctx->num_rx * (qid / 2);
+		queue_type = VIRTNET_STATS_Q_TYPE_RX;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
+		m = &virtio_net_stats_map[i];
+
+		if (m->queue_type != queue_type)
+			continue;
+
+		if (from_driver != m->from_driver)
+			goto skip;
+
+		if (type != m->type)
+			goto skip;
+
+		for (j = 0; j < m->num; ++j) {
+			if (!from_driver) {
+				v = (const u64 *)(base + m->desc[j].offset);
+				ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
+			} else {
+				v_stat = (const u64_stats_t *)(base + m->desc[j].offset);
+				ctx->data[offset + j] = u64_stats_read(v_stat);
+			}
+		}
+
+		break;
+skip:
+		if (virtnet_stats_valid(vi, m))
+			offset += m->num;
+	}
+}
+
 static int virtnet_get_hw_stats(struct virtnet_info *vi,
 				struct virtnet_stats_ctx *ctx)
 {
 	struct virtio_net_ctrl_queue_stats *req;
 	struct virtio_net_stats_reply_hdr *hdr;
 	struct scatterlist sgs_in, sgs_out;
-	u32 num_rx, num_tx, num_cq, offset;
 	int qnum, i, j,  qid, res_size;
-	struct virtnet_stats_map *m;
 	void *reply, *p;
-	u64 bitmap;
 	int ok;
-	u64 *v;
 
 	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
 		return 0;
@@ -3422,43 +3495,10 @@ static int virtnet_get_hw_stats(struct virtnet_info *vi,
 		return ok;
 	}
 
-	num_rx = VIRTNET_RQ_STATS_LEN + ctx->num_rx;
-	num_tx = VIRTNET_SQ_STATS_LEN + ctx->num_tx;
-	num_cq = ctx->num_tx;
-
 	for (p = reply; p - reply < res_size; p += virtio16_to_cpu(vi->vdev, hdr->size)) {
 		hdr = p;
-
 		qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
-
-		if (qid == vi->max_queue_pairs * 2) {
-			offset = 0;
-			bitmap = ctx->bitmap_cq;
-		} else if (qid % 2) {
-			offset = num_cq + num_rx * vi->curr_queue_pairs + num_tx * (qid / 2);
-			offset += VIRTNET_SQ_STATS_LEN;
-			bitmap = ctx->bitmap_tx;
-		} else {
-			offset = num_cq + num_rx * (qid / 2) + VIRTNET_RQ_STATS_LEN;
-			bitmap = ctx->bitmap_rx;
-		}
-
-		for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
-			m = &virtio_net_stats_map[i];
-
-			if (m->flag & bitmap)
-				offset += m->num;
-
-			if (hdr->type != m->type)
-				continue;
-
-			for (j = 0; j < m->num; ++j) {
-				v = p + m->desc[j].offset;
-				ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
-			}
-
-			break;
-		}
+		virtnet_fill_stats(vi, qid, ctx, p, false, hdr->type);
 	}
 
 	kfree(reply);
@@ -3468,28 +3508,18 @@ static int virtnet_get_hw_stats(struct virtnet_info *vi,
 static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
-	unsigned int i, j;
+	unsigned int i;
 	u8 *p = data;
 
 	switch (stringset) {
 	case ETH_SS_STATS:
-		virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
+		virtnet_get_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
 
-		for (i = 0; i < vi->curr_queue_pairs; i++) {
-			for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++)
-				ethtool_sprintf(&p, "rx_queue_%u_%s", i,
-						virtnet_rq_stats_desc[j].desc);
+		for (i = 0; i < vi->curr_queue_pairs; ++i)
+			virtnet_get_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, i, &p);
 
-			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, i, &p);
-		}
-
-		for (i = 0; i < vi->curr_queue_pairs; i++) {
-			for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++)
-				ethtool_sprintf(&p, "tx_queue_%u_%s", i,
-						virtnet_sq_stats_desc[j].desc);
-
-			virtnet_get_hw_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, i, &p);
-		}
+		for (i = 0; i < vi->curr_queue_pairs; ++i)
+			virtnet_get_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, i, &p);
 		break;
 	}
 }
@@ -3522,8 +3552,7 @@ static int virtnet_get_sset_count(struct net_device *dev, int sset)
 
 		virtnet_stats_ctx_init(vi, &ctx, NULL);
 
-		pair_count = VIRTNET_RQ_STATS_LEN + VIRTNET_SQ_STATS_LEN;
-		pair_count += ctx.num_rx + ctx.num_tx;
+		pair_count = ctx.num_rx + ctx.num_tx;
 
 		return ctx.num_cq + vi->curr_queue_pairs * pair_count;
 	default:
@@ -3536,46 +3565,27 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
 {
 	struct virtnet_info *vi = netdev_priv(dev);
 	struct virtnet_stats_ctx ctx = {0};
-	unsigned int idx, start, i, j;
+	unsigned int start, i;
 	const u8 *stats_base;
-	const u64_stats_t *p;
-	size_t offset;
 
 	virtnet_stats_ctx_init(vi, &ctx, data);
 	virtnet_get_hw_stats(vi, &ctx);
 
-	idx = ctx.num_cq;
-
 	for (i = 0; i < vi->curr_queue_pairs; i++) {
 		struct receive_queue *rq = &vi->rq[i];
+		struct send_queue *sq = &vi->sq[i];
 
 		stats_base = (const u8 *)&rq->stats;
 		do {
 			start = u64_stats_fetch_begin(&rq->stats.syncp);
-			for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++) {
-				offset = virtnet_rq_stats_desc[j].offset;
-				p = (const u64_stats_t *)(stats_base + offset);
-				data[idx + j] = u64_stats_read(p);
-			}
+			virtnet_fill_stats(vi, i * 2, &ctx, stats_base, true, 0);
 		} while (u64_stats_fetch_retry(&rq->stats.syncp, start));
-		idx += VIRTNET_RQ_STATS_LEN;
-		idx += ctx.num_rx;
-	}
-
-	for (i = 0; i < vi->curr_queue_pairs; i++) {
-		struct send_queue *sq = &vi->sq[i];
 
 		stats_base = (const u8 *)&sq->stats;
 		do {
 			start = u64_stats_fetch_begin(&sq->stats.syncp);
-			for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++) {
-				offset = virtnet_sq_stats_desc[j].offset;
-				p = (const u64_stats_t *)(stats_base + offset);
-				data[idx + j] = u64_stats_read(p);
-			}
+			virtnet_fill_stats(vi, i * 2 + 1, &ctx, stats_base, true, 0);
 		} while (u64_stats_fetch_retry(&sq->stats.syncp, start));
-		idx += VIRTNET_SQ_STATS_LEN;
-		idx += ctx.num_tx;
 	}
 }
 
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net-next v1 4/6] virtio_net: stats map include driver stats

[kernel test robot]

Hi Xuan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.7-rc8]
[cannot apply to net-next/main horms-ipvs/master next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xuan-Zhuo/virtio_net-introduce-device-stats-feature-and-structures/20231226-153227
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20231226073103.116153-5-xuanzhuo%40linux.alibaba.com
patch subject: [PATCH net-next v1 4/6] virtio_net: stats map include driver stats
config: x86_64-randconfig-121-20240101 (https://download.01.org/0day-ci/archive/20240102/202401020928.JpmKXuhu-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240102/202401020928.JpmKXuhu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401020928.JpmKXuhu-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/virtio_net.c:3416:83: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio64 [usertype] val @@     got unsigned long long const [usertype] @@
   drivers/net/virtio_net.c:3416:83: sparse:     expected restricted __virtio64 [usertype] val
   drivers/net/virtio_net.c:3416:83: sparse:     got unsigned long long const [usertype]
   drivers/net/virtio_net.c:3500:52: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio16 [usertype] val @@     got restricted __le16 [usertype] vq_index @@
   drivers/net/virtio_net.c:3500:52: sparse:     expected restricted __virtio16 [usertype] val
   drivers/net/virtio_net.c:3500:52: sparse:     got restricted __le16 [usertype] vq_index
   drivers/net/virtio_net.c:3498:81: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio16 [usertype] val @@     got restricted __le16 [usertype] size @@
   drivers/net/virtio_net.c:3498:81: sparse:     expected restricted __virtio16 [usertype] val
   drivers/net/virtio_net.c:3498:81: sparse:     got restricted __le16 [usertype] size
   drivers/net/virtio_net.c:3549:82: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __virtio64 [usertype] val @@     got restricted __le64 [assigned] [usertype] v @@
   drivers/net/virtio_net.c:3549:82: sparse:     expected restricted __virtio64 [usertype] val
   drivers/net/virtio_net.c:3549:82: sparse:     got restricted __le64 [assigned] [usertype] v

vim +3416 drivers/net/virtio_net.c

  3378	
  3379	static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid,
  3380				       struct virtnet_stats_ctx *ctx,
  3381				       const u8 *base, bool from_driver, u8 type)
  3382	{
  3383		struct virtnet_stats_map *m;
  3384		const u64_stats_t *v_stat;
  3385		u32 queue_type;
  3386		const u64 *v;
  3387		u64 offset;
  3388		int i, j;
  3389	
  3390		if (qid == vi->max_queue_pairs * 2) {
  3391			offset = 0;
  3392			queue_type = VIRTNET_STATS_Q_TYPE_CQ;
  3393		} else if (qid % 2) {
  3394			offset = ctx->num_cq + ctx->num_rx * vi->curr_queue_pairs + ctx->num_tx * (qid / 2);
  3395			queue_type = VIRTNET_STATS_Q_TYPE_TX;
  3396		} else {
  3397			offset = ctx->num_cq + ctx->num_rx * (qid / 2);
  3398			queue_type = VIRTNET_STATS_Q_TYPE_RX;
  3399		}
  3400	
  3401		for (i = 0; i < ARRAY_SIZE(virtio_net_stats_map); ++i) {
  3402			m = &virtio_net_stats_map[i];
  3403	
  3404			if (m->queue_type != queue_type)
  3405				continue;
  3406	
  3407			if (from_driver != m->from_driver)
  3408				goto skip;
  3409	
  3410			if (type != m->type)
  3411				goto skip;
  3412	
  3413			for (j = 0; j < m->num; ++j) {
  3414				if (!from_driver) {
  3415					v = (const u64 *)(base + m->desc[j].offset);
> 3416					ctx->data[offset + j] = virtio64_to_cpu(vi->vdev, *v);
  3417				} else {
  3418					v_stat = (const u64_stats_t *)(base + m->desc[j].offset);
  3419					ctx->data[offset + j] = u64_stats_read(v_stat);
  3420				}
  3421			}
  3422	
  3423			break;
  3424	skip:
  3425			if (virtnet_stats_valid(vi, m))
  3426				offset += m->num;
  3427		}
  3428	}
  3429	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH net-next v1 5/6] virtio_net: add the total stats field

[Xuan Zhuo]

Now, we just show the stats of every queue.

But for the user, the total values of every stat may are valuable.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 72 ++++++++++++++++++++++++++++++++++------
 1 file changed, 61 insertions(+), 11 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 75d68795f2bc..e97a9474d91a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3290,6 +3290,7 @@ static int virtnet_set_channels(struct net_device *dev,
 	return err;
 }
 
+/* qid == -1: for rx/tx queue total field */
 static void virtnet_get_stats_string(struct virtnet_info *vi, int type, int qid, u8 **data)
 {
 	struct virtnet_stats_map *m;
@@ -3312,14 +3313,23 @@ static void virtnet_get_stats_string(struct virtnet_info *vi, int type, int qid,
 			else
 				tp = "_hw";
 
-			if (type == VIRTNET_STATS_Q_TYPE_RX)
-				ethtool_sprintf(&p, "rx_queue%s_%u_%s", tp, qid, m->desc[j].desc);
-
-			else if (type == VIRTNET_STATS_Q_TYPE_TX)
-				ethtool_sprintf(&p, "tx_queue%s_%u_%s", tp, qid, m->desc[j].desc);
-
-			else if (type == VIRTNET_STATS_Q_TYPE_CQ)
+			if (type == VIRTNET_STATS_Q_TYPE_RX) {
+				if (qid < 0)
+					ethtool_sprintf(&p, "rx%s_%s", tp, m->desc[j].desc);
+				else
+					ethtool_sprintf(&p, "rx_queue%s_%u_%s", tp, qid,
+							m->desc[j].desc);
+
+			} else if (type == VIRTNET_STATS_Q_TYPE_TX) {
+				if (qid < 0)
+					ethtool_sprintf(&p, "tx%s_%s", tp, m->desc[j].desc);
+				else
+					ethtool_sprintf(&p, "tx_queue%s_%u_%s", tp, qid,
+							m->desc[j].desc);
+
+			} else if (type == VIRTNET_STATS_Q_TYPE_CQ) {
 				ethtool_sprintf(&p, "cq%s_%s", tp, m->desc[j].desc);
+			}
 		}
 	}
 
@@ -3376,6 +3386,38 @@ static void virtnet_stats_ctx_init(struct virtnet_info *vi,
 	}
 }
 
+static void stats_sum_queue(u64 *sum, u32 num, u64 *q_value, u32 q_num)
+{
+	u32 step = num;
+	int i, j;
+	u64 *p;
+
+	for (i = 0; i < num; ++i) {
+		p = sum + i;
+		*p = 0;
+
+		for (j = 0; j < q_num; ++j)
+			*p += *(q_value + i + j * step);
+	}
+}
+
+static void virtnet_fill_total_fields(struct virtnet_info *vi,
+				      struct virtnet_stats_ctx *ctx)
+{
+	u64 *data, *first_rx_q, *first_tx_q;
+
+	first_rx_q = ctx->data + ctx->num_rx + ctx->num_tx + ctx->num_cq;
+	first_tx_q = first_rx_q + vi->curr_queue_pairs * ctx->num_rx;
+
+	data = ctx->data;
+
+	stats_sum_queue(data, ctx->num_rx, first_rx_q, vi->curr_queue_pairs);
+
+	data = ctx->data + ctx->num_rx;
+
+	stats_sum_queue(data, ctx->num_tx, first_tx_q, vi->curr_queue_pairs);
+}
+
 static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid,
 			       struct virtnet_stats_ctx *ctx,
 			       const u8 *base, bool from_driver, u8 type)
@@ -3387,14 +3429,17 @@ static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid,
 	u64 offset;
 	int i, j;
 
+	/* skip the total fields of pairs */
+	offset = ctx->num_rx + ctx->num_tx;
+
 	if (qid == vi->max_queue_pairs * 2) {
-		offset = 0;
 		queue_type = VIRTNET_STATS_Q_TYPE_CQ;
 	} else if (qid % 2) {
-		offset = ctx->num_cq + ctx->num_rx * vi->curr_queue_pairs + ctx->num_tx * (qid / 2);
+		offset += ctx->num_cq + ctx->num_rx * vi->curr_queue_pairs +
+			ctx->num_tx * (qid / 2);
 		queue_type = VIRTNET_STATS_Q_TYPE_TX;
 	} else {
-		offset = ctx->num_cq + ctx->num_rx * (qid / 2);
+		offset += ctx->num_cq + ctx->num_rx * (qid / 2);
 		queue_type = VIRTNET_STATS_Q_TYPE_RX;
 	}
 
@@ -3513,6 +3558,9 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 
 	switch (stringset) {
 	case ETH_SS_STATS:
+		virtnet_get_stats_string(vi, VIRTNET_STATS_Q_TYPE_RX, -1, &p);
+		virtnet_get_stats_string(vi, VIRTNET_STATS_Q_TYPE_TX, -1, &p);
+
 		virtnet_get_stats_string(vi, VIRTNET_STATS_Q_TYPE_CQ, 0, &p);
 
 		for (i = 0; i < vi->curr_queue_pairs; ++i)
@@ -3554,7 +3602,7 @@ static int virtnet_get_sset_count(struct net_device *dev, int sset)
 
 		pair_count = ctx.num_rx + ctx.num_tx;
 
-		return ctx.num_cq + vi->curr_queue_pairs * pair_count;
+		return pair_count + ctx.num_cq + vi->curr_queue_pairs * pair_count;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -3587,6 +3635,8 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
 			virtnet_fill_stats(vi, i * 2 + 1, &ctx, stats_base, true, 0);
 		} while (u64_stats_fetch_retry(&sq->stats.syncp, start));
 	}
+
+	virtnet_fill_total_fields(vi, &ctx);
 }
 
 static void virtnet_get_channels(struct net_device *dev,
-- 
2.32.0.3.g01195cf9f

[PATCH net-next v1 6/6] virtio_net: rename stat tx_timeout to timeout

[Xuan Zhuo]

Now, we have this:

    tx_queue_0_tx_timeouts

This is used to record the tx schedule timeout.
But this has two "tx". I think the below is enough.

    tx_queue_0_timeouts

So I rename this field.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e97a9474d91a..49625638ad43 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -86,7 +86,7 @@ struct virtnet_sq_stats {
 	u64_stats_t xdp_tx;
 	u64_stats_t xdp_tx_drops;
 	u64_stats_t kicks;
-	u64_stats_t tx_timeouts;
+	u64_stats_t timeouts;
 };
 
 struct virtnet_rq_stats {
@@ -110,7 +110,7 @@ static const struct virtnet_stat_desc virtnet_sq_stats_desc[] = {
 	{ "xdp_tx",		VIRTNET_SQ_STAT(xdp_tx) },
 	{ "xdp_tx_drops",	VIRTNET_SQ_STAT(xdp_tx_drops) },
 	{ "kicks",		VIRTNET_SQ_STAT(kicks) },
-	{ "tx_timeouts",	VIRTNET_SQ_STAT(tx_timeouts) },
+	{ "timeouts",		VIRTNET_SQ_STAT(timeouts) },
 };
 
 static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = {
@@ -2696,7 +2696,7 @@ static void virtnet_stats(struct net_device *dev,
 			start = u64_stats_fetch_begin(&sq->stats.syncp);
 			tpackets = u64_stats_read(&sq->stats.packets);
 			tbytes   = u64_stats_read(&sq->stats.bytes);
-			terrors  = u64_stats_read(&sq->stats.tx_timeouts);
+			terrors  = u64_stats_read(&sq->stats.timeouts);
 		} while (u64_stats_fetch_retry(&sq->stats.syncp, start));
 
 		do {
@@ -4316,7 +4316,7 @@ static void virtnet_tx_timeout(struct net_device *dev, unsigned int txqueue)
 	struct netdev_queue *txq = netdev_get_tx_queue(dev, txqueue);
 
 	u64_stats_update_begin(&sq->stats.syncp);
-	u64_stats_inc(&sq->stats.tx_timeouts);
+	u64_stats_inc(&sq->stats.timeouts);
 	u64_stats_update_end(&sq->stats.syncp);
 
 	netdev_err(dev, "TX timeout on queue: %u, sq: %s, vq: 0x%x, name: %s, %u usecs ago\n",
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net-next v1 0/6] virtio-net: support device stats

[Xuan Zhuo]

On Tue, 26 Dec 2023 15:30:57 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> As the spec:
>
> https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
>
> The virtio net supports to get device stats.

Hi Jason,

Any comments for this?

Thanks



>
> Please review.
>
> Thanks.
>
> v1:
>     1. fix some definitions of the marco and the struct
>
>
> Xuan Zhuo (6):
>   virtio_net: introduce device stats feature and structures
>   virtio_net: virtnet_send_command supports command-specific-result
>   virtio_net: support device stats
>   virtio_net: stats map include driver stats
>   virtio_net: add the total stats field
>   virtio_net: rename stat tx_timeout to timeout
>
>  drivers/net/virtio_net.c        | 533 ++++++++++++++++++++++++++++----
>  include/uapi/linux/virtio_net.h | 137 ++++++++
>  2 files changed, 610 insertions(+), 60 deletions(-)
>
> --
> 2.32.0.3.g01195cf9f
>

Re: [PATCH net-next v1 0/6] virtio-net: support device stats

[Jason Wang]

On Tue, Jan 16, 2024 at 1:56 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Tue, 26 Dec 2023 15:30:57 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > As the spec:
> >
> > https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> >
> > The virtio net supports to get device stats.
>
> Hi Jason,
>
> Any comments for this?
>
> Thanks
>

I see comments from both Simon and Michael, let's try to address them
and I will review v2.

Does this sound good to you?

Thanks

Re: [PATCH net-next v1 0/6] virtio-net: support device stats

[Xuan Zhuo]

On Tue, 30 Jan 2024 11:20:10 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Tue, Jan 16, 2024 at 1:56 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Tue, 26 Dec 2023 15:30:57 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > As the spec:
> > >
> > > https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> > >
> > > The virtio net supports to get device stats.
> >
> > Hi Jason,
> >
> > Any comments for this?
> >
> > Thanks
> >
>
> I see comments from both Simon and Michael, let's try to address them
> and I will review v2.
>
> Does this sound good to you?

OK.

Thanks.


>
> Thanks
>