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

[Jason Wang <jasowang@redhat.com>]

On Mon, Nov 22, 2021 at 5:08 PM Eli Cohen <elic@nvidia.com> wrote:
>
> On Mon, Nov 22, 2021 at 04:07:24PM +0800, Jason Wang wrote:
> > On Mon, Nov 22, 2021 at 3:56 PM Eli Cohen <elic@nvidia.com> wrote:
> > >
> > > On Mon, Nov 22, 2021 at 10:30:12AM +0800, Jason Wang wrote:
> > > > On Fri, Nov 19, 2021 at 11:09 AM Parav Pandit <parav@nvidia.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > > From: Jason Wang <jasowang@redhat.com>
> > > > > > Sent: Friday, November 19, 2021 8:12 AM
> > > > > >
> > > > > > On Thu, Nov 18, 2021 at 1:58 PM Eli Cohen <elic@nvidia.com> wrote:
> > > > > > >
> > > > > > > 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.
> >
>
> received and completed descriptors are very basic and I assume any
> vendor would support those.

At least it was not supported by virtio.

> If we go with vendor stats, how can we
> communicate the information to userspace? Currenlty we use netlink
> attributes defined to pass this information.

It can be done exactly as what have been done in the patch, we can
document it as vendor stats.

>
> > >
> > > > >
> > > > > > 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.
> > > > >
> > > > > > 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.
> > >
> > > > > $ 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?
> >
>
> I am not sure I am following you. You do query twice. You can never know
> how many rounds the virtqueue has gone through so the information is
> useless.

Ok, I think I get you. So I wonder if it's more useful to use device
specific counters. For networking, it could be packets send/received
etc.

Thanks

>
> > Thanks
> >
> > >
> > > > Thanks
> > > >
> > > > >
> > > > > 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