On Mon, Nov 22, 2021 at 5:08 PM Eli Cohen <elic at 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 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.
> >
>
> 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.
> > > > >
> > > >
> > >
> >
>