Re: [PATCH 1/2] vdpa: Add support for querying statistics

On Mon, Nov 22, 2021 at 3:56 PM Eli Cohen [off-list ref] wrote:

On Mon, Nov 22, 2021 at 10:30:12AM +0800, Jason Wang wrote:

quoted

On Fri, Nov 19, 2021 at 11:09 AM Parav Pandit [off-list ref] wrote:

quoted

quoted

From: Jason Wang <redacted>
Sent: Friday, November 19, 2021 8:12 AM

On Thu, Nov 18, 2021 at 1:58 PM Eli Cohen [off-list ref] wrote:

quoted

Add support for querying virtqueue statistics. Supported statistics are:

Received_desc - number of descriptors received for the virtqueue
completed_desc - number of descriptors completed for the virtqueue

A new callback was added to vdpa_config_ops which provides the means
for the vdpa driver to return statistics results.

The interface allows for reading all the supported virtqueues,
including the control virtqueue if it exists, by returning the next
queue index to query.

Examples:
1. Read statisitics for the virtqueue at index 1 $ vdpa dev stats show
vdpa-a index 1
vdpa-a:
index 1 tx received_desc 21 completed_desc 21

2. Read statisitics for all the virtqueues vdpa dev stats show vdpa-a
vdpa-a:
index 0 rx received_desc 256 completed_desc 12 index 1 tx
received_desc 21 completed_desc 21 index 2 ctrl received_desc 0
completed_desc 0

Adding Adrian and Laurent.

It's quite useful but I think it's vendor specific statistics.

These are vdpa device specific of Linux.
And are very generic of the VQ for all device types.

The question is what happens if the parent doesn't implement those statistics.

Are you suggesting that some parents may support reporting a subeset of
the fields and we should maybe indicate what is actually reported?

It's an open question. E.g do we want a stable API for the those
statistics counter? If yes, it's better to not to let the parents to
report a subset but then it forces the exact same counters to be
supported by other vendors. If not, it should be fine. For any case, I
think it's simpler to start with "vendor-stats" and we can switch it
to a standard way if it was agreed by every vendor.

quoted

quoted

quoted

I wonder if it's better
to use "vendor" prefix in the protocol, then we use this instead:

vdpa dev vendor-stats show vdpa-a

May be. Lets evaluate if stats of this patch are generic enough or not.

quoted

Or if we want to make it standard is exposing virtio index better?

qid 0 last_avail_idx X avail_idx Y last_used_idx M used_idx N

I did consider this option a while back. Shows indices are equally useful.
I think we should show that as vq info, along with other VQ attributes (addr, len).

That may work but it looks to me the receiced_desc/completed_desc is
also per vq.

Another question is that is it more useful to use buffers instead of
descriptors? E.g how indirect descriptors are counted.

I think it's descriptors so indirect descriptors are counted once but I
need to verify that.

quoted

quoted

$ vdpa dev show vq

But showing indices are not less statistics and more current state of the queue.
For example roll over of the indices won't cover absolute number of descriptors processed for the queue.
And even if we make them u64 (not good), non_developer end user needs to keep using side calculator to count the delta.

How about exposing those raw indices via the protocol and letting the
vdpa tool calculate for us?

counters are 16 bit per the virtio spec so I don't know how you could
handle rolover without losing information.

So at most 1 rollover I guess. So it's not hard by comparing the
indices. E.g the if last_avil_idx(device avail idx) > avail_idx, we
know there's one?

Thanks

quoted

Thanks

quoted

So I think useful q indices belong to q info.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization