On Fri, Dec 22, 2023 at 11:30:18AM +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>.
>
> Signed-off-by: Xuan Zhuo <xuanz...@linux.alibaba.com>
Hi Xuan Zhuo,
some minor feedback from my side relating to Sparse warnings.
...
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
...
> +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;
...
> + for (p = reply; p - reply < res_size; p += virtio16_to_cpu(vi->vdev,
> hdr->size)) {
The type of the second parameter of _virtio16_to_cpu() is __virtio16.
But the type of the size field of virtio_net_stats_reply_hdr is __le16.
This does not seem correct.
> + hdr = p;
> +
> + qid = virtio16_to_cpu(vi->vdev, hdr->vq_index);
Similarly, the vq_index field of virtio_net_stats_reply_hdr is also __le16.
...
> + 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);
Here the type of the second parameter of virtio64_to_cpu() is __virtio64.
But the type of *v is u64.
> + }
> +
> + break;
> + }
> + }
> +
> + kfree(reply);
> + return 0;
> +}
> +
...
> @@ -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);
Similarly, the type of v is __le64.
> + }
> + }
> +
> + 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;
> }
...
