OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [virtio-dev] Re: [PATCH v11] virtio-net: support device stats


On Mon, Mar 7, 2022 at 5:07 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Mon, 7 Mar 2022 15:58:03 +0800, Jason Wang <jasowang@redhat.com> wrote:
> >
> > å 2022/3/2 äå4:52, Xuan Zhuo åé:
> > > This patch allows the driver to obtain some statistics from the device.
> > >
> > > In the back-end implementation, we can count a lot of such information,
> > > which can be used for debugging and judging the running status of the
> > > back-end. We hope to directly display it to the user through ethtool.
> > >
> > > To get stats atomically, try to get stats for all queue pairs in one
> > > command.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > v11:
> > > 1. Use michael's advice to introduce virtio_net_ctrl_queue_stats to get vq stats
> > >     based on vq num and type
> > > 2. split stats structure

[...]

> > What happens if driver tries to query RX stats through a TX index?
>
> Device setting ack to VIRTIO_NET_ERR.
>
> I relegated this case to the next case. "The type of vq does not match \field{type}."
>
> I will explain in the next release:
>
>   \item The type of vq does not match \field{type}. Such as the driver tries to query RX stats through a TX index.
>
> >
> >
> > > +    \item The type of vq does not match \field{type}.
> > > +    \item The feature corresponding to the specified \field{type} was not successfully
> > > +        negotiated.
> > > +    \item The size of \field{command-specific-data-reply} is less than the sum
> > > +        of \field{length}.
> >
> >
> > I'm not sure I get here, I guess this proposal allows the driver to
> > query an array of stats. So I guess it means the size of required stats
> > instead of the size of \field{command-specific-data-reply}.
>
> The size of \field{command-specific-data-reply} refers to the size of memory
> allocated by the driver for \field{command-specific-data-reply}.
>
> Sorry, I will make it clearer in the next version.
>
> >
> >
> > > +\end{itemize}
> > > +
> > > +The device MUST write the requested stats structures in
> > > +\field{command-specific-data-reply} in the order specified by the structure
> > > +virtio_net_ctrl_queue_stats.
> >
> >
> > Do we need per stat error code here? Or device can simply fail a batch
> > of query if one of those fails?
>
> Device can simply fail a batch of query if one of those fails.

We need clarify, and I guess we should use MUST instead of MAY without
a per stat error value.

>
> >
> >
> > > +
> > > +The size of each stats MUST be less than or equal to the corresponding
> > > +\field{length}, but the space it occupies MUST be equal to the corresponding
> > > +\field{length}.
> >
> >
> > I wonder how the length trick can work here. Is this used for extension?
> > If yes, how can driver know a suitable length?
>
> It only allows the possibility of expansion.
>
> The driver must set the length based on the size of the stats it knows.

Ok, this seems like a way to make a new driver that can run on an old device.

But then I wonder if it would be simpler to just use fixed length and
just extend the types.

Thanks

> It seems that I am not very clear on this point. I will make this clear in the
> next version.
>
> Of course, it is allowed to set a larger length. As long as the driver allocates
> the corresponding memory for it.
>
>
> > What happens if device support more stats?
>
> "The size of each stats MUST be less than or equal to the corresponding \field{length}."
>
> >
> >
> > > +
> > > +\drivernormative{\subparagraph}{Device Stats}{Device Types / Network Device / Device Operation / Control Virtqueue / Device Stats}
> > > +
> > > +When a driver tries to obtain a certain stats, it MUST confirm that the relevant
> > > +feature negotiation is successful.
> > > +
> > > +\field{type} in struct virtio_net_ctrl_queue_stats MUST correspond to the vq
> > > +specified by \field{queue_num}.
> > > +
> > > +\field{length} in struct virtio_net_ctrl_queue_stats MUST be greater than or
> > > +equal to the size of the structure corresponding to \field{type}. It MUST be a
> > > +multiple of 8.
> >
> >
> > Why do we need the padding here?
>
> This is to ensure structure alignment.
>
> >
> >
> > > +
> > > +The size of \field{command-specific-data-reply} allocated by the driver MUST be
> > > +greater than or equal to the sum of \field{legnth} in struct
> >
> >
> > typo.
>
> I will fix it.
>
> >
> >
> > > +virtio_net_ctrl_queue_stats.
> >
> >
> > Any value that we can allocate more than the sum of the length?
>
> It is theoretically allowed, as long as the allocated space is greater than the
> sum of length, there will be no error.
>
> Thanks.
>
> >
> > Thanks
> >
> >
> > > +
> > > +When the driver reads the response, it MUST read
> > > +\field{command-specific-data-reply} one by one based on the set \field{length}
> > > +and \field{type}.
> > >
> > >   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > >   Types / Network Device / Legacy Interface: Framing Requirements}
> > > --
> > > 2.31.0
> > >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]