Re: [PATCH v9] virtio-net: support device stats

[Xuan Zhuo <xuanzhuo@linux.alibaba.com>]

On Mon, 28 Feb 2022 06:55:27 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Feb 28, 2022 at 07:21:39PM +0800, Xuan Zhuo wrote:
> > > struct stats{
> > > /* the counters that doesn't need any features */
> > > /* the counters that needs CSUM */
> > > /* the counters that needs GSO */
> > > /* the counters that needs reset */
> > > }
> > >
> > > Or even split them into different commands. The reason is that the
> > > current design requires the knowledge of a feature even if it doesn't
> > > support it.
> >
> >
> > I think this is a good idea.
>
> The issue is this: some stats are only useful if they are correlated.
> For example, # of rx interrupts is not useful unless you also know the #
> of packets. Similar for GSO things. Is it necessary to correlate TX and
> RX events? It might be if driver polls TX ring on an RX interrupt. In
> all these cases we need a single command to return multiple stats for
> atomicity. From that POV having a single struct is great, however if we
> do it like this then it is not clear how are we going to extend it. Do
> we just add fields at the end? Then the clean separation between RX and
> TX fields is lost.

It should still be convenient to add new commands or feature flags.

> Is it always enough to query a single pair of VQs?
> E.g. when querying RSS, might we want to correlate between multiple
> queues to see that traffic is balanced?
>
> So maybe a good design will get an array of structures describing things
> to query and return an array of structures describing the result.

Thanks for your analysis, I think you are right.

Thanks.

>
> --
> MST
>