[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]