On Mon, Nov 22, 2021 at 3:56 PM Eli Cohen <elic at 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 at
nvidia.com> wrote:
> > >
> > >
> > >
> > > > From: Jason Wang <jasowang at redhat.com>
> > > > Sent: Friday, November 19, 2021 8:12 AM
> > > >
> > > > On Thu, Nov 18, 2021 at 1:58 PM Eli Cohen <elic at
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.
>
> > >
> > > > 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?
Thanks
>
> > Thanks
> >
> > >
> > > So I think useful q indices belong to q info.
> > >
> >
>