On Mon, Mar 7, 2022 at 5:07 PM Xuan Zhuo <[email protected]> wrote:
>
> On Mon, 7 Mar 2022 15:58:03 +0800, Jason Wang <[email protected]> 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 <[email protected]>
> > > Suggested-by: Michael S. Tsirkin <[email protected]>
> > > ---
> > > 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: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
