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

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Sep 01, 2023 at 01:58:32PM +0000, Parav Pandit wrote:
> 
> 
> > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Sent: Wednesday, August 30, 2023 11:23 AM
> > To: virtio-dev@lists.oasis-open.org
> > Cc: jasowang@redhat.com; Michael S. Tsirkin <mst@redhat.com>; Parav Pandit
> > <parav@nvidia.com>; David Edmondson <david.edmondson@oracle.com>;
> > virtio-comment@lists.oasis-open.org
> 
> We should be following [1].
> Virtio list for conducting technical committee work, which is this patch.
> And to get feedback from community virtio-comment list, which you already CCed.
> 
> So virtio-dev should be replaced with virtio list because this patch is about developing the virtio spec itself, (not implementing the spec in device/driver etc).
> 
> [1] https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio#feedback


No, virtio-comment was already CC'd, and I think it's the best list for
comments (this is how we classify patches, technically) since anyone can subscribe
to that one.

We use virtio mostly for discussion about release schedules, voting,
bylaws and other issues not of interest to TC non members.


Removing virtio-dev is a good idea, copying both is really not necessary.

...

> > +    \item [rx_drop_busy]
> > +        This is the number of packets dropped by the device when the device is
> > +        busy.
> > +
> I think the busy counter does not belong to busy category as it is slightly difficult to define.
> Lets please move to different category.

Or better define it better. What does "busy" mean? I suspect basically
our of internal resources?
> > +    \item [rx_csum_bad]
> > +        The number of packets with abnormal csum.
> s/abnormal/wrong
> 
> no strong opinion though.

checsum mismatch.
avoid abbreviation


> > +
> > +\end{description}
> > +
> > +\subparagraph{Transmitq CSUM Statistic}\label{sec:Device Types /
> > +Network Device / Device Operation / Control Virtqueue / Device
> > +Statistic / Transmitq CSUM Statistic}
> > +
> > +The structure corresponding to the transmitq csum statistics is
> > virtio_net_stats_tx_csum.
> > +The corresponding type is VIRTIO_NET_STATS_TYPE_TX_CSUM. This is for the
> > transmitq.
> > +
> > +Only after the VIRTIO_NET_F_CSUM is negotiated, the transmitq csum
> > +statistics can be obtained.
> > +
> > +The following are the transmitq csum statistics:
> > +
> > +\begin{lstlisting}
> > +struct virtio_net_stats_tx_csum {
> > +    struct virtio_net_stats_reply_hdr hdr;
> > +
> > +    le64 tx_csum_none;
> > +    le64 tx_needs_csum;
> > +};
> > +\end{lstlisting}
> > +
> > +The packets described below are all from a specific virtqueue.
> > +\begin{description}
> > +    \item [tx_csum_none]
> > +        The number of packets which didn't require hardware csum.
> > +
> The number of packets which requires checksum calculation by the device.

which require - it's not the number that requires checksum it's the
packets that require it.

...

> > +The device has the allowance for the speed. If
> > +VIRTIO_NET_F_SPEED_DUPLEX has been negotiated, the driver can get this by
> > \field{speed}.
> > +When the real speed exceeds the speed allowance, some packets will be
> > +dropped by the device.
> > +
> Above description regarding "real speed" is confusing.
> I think you wanted to say,
> 
> When the received packets rate exceeds the negotiated speed, some packets may be dropped by the device.

negotiated how?

yes, I don't understand what this does, either.



---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org