Re: [PATCH v5] virtio-net: clarify coalescing parameters settings

Heng Qi
2024-06-17T02:47:23Z

On Tue, 11 Jun 2024 19:03:11 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, May 28, 2024 at 12:47:02PM +0800, Heng Qi wrote:
> > The device can set any initial coalescing parameters (0 or non-zero)
> > for the receive/send queue before the setting command is executed,
> > not just 0, enhancing device performance even without DIM enabled.
> >
> > So we need to clarify descriptions that don't fit the behavior.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/194
> > Suggested-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > Reviewed-by: Parav Pandit <parav@nvidia.com>
> > Reviewed-by: Jason Wang <jasowang@redhat.com>
> > ---
> > v4->v5:
> > - Add RB tags from Parav and Jason. Thanks!
> >
> > v3->v4:
> > - Doesn't force the device to remember more stuff. @Parav
> >
> > v2->v3:
> > - Clarify description to be more generic. @Parav
> >
> > v1->v2:
> > - Update description. @Jason
> >
> > device-types/net/description.tex | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> > index 61cce1f..00c1b36 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -1803,6 +1803,14 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > for an enabled transmit/receive virtqueue whose index is \field{vq_index}.
> > \end{enumerate}
> >
> > +If the VIRTIO_NET_F_NOTF_COAL or VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated,
> > +the device may apply any coalescing parameters to each transmit/receive virtqueue
> > +before the driver successfully performs one of the VIRTIO_NET_CTRL_NOTF_COAL set commands.
>
> may outside of conformance section, not good.

This is outside of the conformance section. Do you want inside?

>
> > +
> > +The driver can query the coalescing parameters of any enabled transmit/receive
> > +virtqueue using the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command, before or after any
> > +VIRTIO_NET_CTRL_NOTF_COAL set command is done.
> > +
>
> Vague. There are just 3 set commands ust list them.
>
> > The device may generate notifications more or less frequently than specified by set commands of the VIRTIO_NET_CTRL_NOTF_COAL class.
> >
> > If coalescing parameters are being set, the device applies the last coalescing parameters set for a
>
> This needs more work but I guess we can skip the above part.
>
> > @@ -1885,17 +1893,17 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > VIRTIO_NET_ERR if the designated virtqueue is not an enabled transmit or receive virtqueue.
> >
> > Upon disabling and re-enabling a transmit virtqueue, the device MUST set the coalescing parameters of the virtqueue
> > -to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or, if the driver did not set any TX coalescing parameters, to 0.
> > +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or, if the driver did not set any TX coalescing parameters, the device MAY initialize them to any values.
> >
>
> If device does not initialize them then what?

The device's customized behavior, at least, does not conform to the spec.

> Actually something like
> 0 -> the default TX coalescing parameters chosen by the device?

Deefault coalescing parameters should also have a concrete "value".

>
>
> > Upon disabling and re-enabling a receive virtqueue, the device MUST set the coalescing parameters of the virtqueue
> > -to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set any RX coalescing parameters, to 0.
> > +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set any RX coalescing parameters, the device MAY initialize them to any values.
> >
> > The behavior of the device in response to set commands of the VIRTIO_NET_CTRL_NOTF_COAL class is best-effort:
> > the device MAY generate notifications more or less frequently than specified.
> >
> > A device SHOULD NOT send used buffer notifications to the driver if the notifications are suppressed, even if the notification conditions are met.
> >
> > -Upon reset, a device MUST initialize all coalescing parameters to 0.
> > +Upon reset, a device MAY set any coalescing parameters for all transmit and receive virtqueues.
>
> Confusing.
> This seems to allow any vq to have a different set of parameters.

YES.

> Now if driver calls VIRTIO_NET_CTRL_NOTF_COAL_RX_SET then what?
>

Then the coalescing parameters of all RX queues are updated to the values
set by VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, and the coalescing parameters of all
TX queues continue to retain the previous values.

Thanks.

>
> >
> > \paragraph{Device Statistics}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Device Statistics}
>
>
>
>
> >
> > --
> > 2.32.0.3.g01195cf9f
>