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

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

On Thu, Jun 27, 2024 at 12:37:32PM +0200, Halil Pasic wrote:
> On Tue, 25 Jun 2024 18:14:15 -0700
> Si-Wei Liu <si-wei.liu@oracle.com> wrote:
> 
> > On 6/24/2024 10:56 PM, Parav Pandit wrote:
> [..]
> > > I saw the need of this proposal slightly differently in the discussion with Heng in v4.
> > > The way I understood is, proposed relaxation enables below Linux driver flow to work as equally as without device offering VIRTIO_NET_F_VQ_NOTF_COAL.
> > >
> > > Flow is:
> > > 1. The device offered feature VIRTIO_NET_F_VQ_NOTF_COAL
> > > 2. The virtio-net driver negotiated VIRTNET_FEATURES that has VIRTIO_NET_F_VQ_NOTF_COAL
> > >
> > > 3. Because VIRTIO_NET_F_VQ_NOTF_COAL is negotiated, device is not applying any coalescing on the VQ, in a good hope that driver will perform VQ notification coalescing.
> 
> I have certainly understood this differently. When
> VIRTIO_NET_F_VQ_NOTF_COAL is not negotiated then the device is not supposed/allowed to do any interrupt coalescing (notification suppression may still apply).
> 
> If VIRTIO_NET_F_VQ_NOTF_COAL is negotiated the device is supposed
> to/MUST do the coalescing according to the parameters as described by
> the virtio spec.
> 
> Michael, Jason: Can you guys weigh in on this?

I still don't understand why this change is needed.
We have this text:

	The device may generate notifications more or less frequently than
	specified by set commands of the VIRTIO_NET_CTRL_NOTF_COAL class.

and

	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.

So with no spec changes, devices already can do what this patch says they can do -
send notifications less frequently.

Re-reading this spec text, maybe the confusion is that it mentions set
commands specifically? And it's also stuck in the middle where it's easy
to miss.

So it would seem that the following should be sufficient, and it looks
like a small clarification we could just apply and include in the
vote for the csd. What do you guys think?


Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---- 


diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 76585b0..d6788df 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1711,8 +1711,6 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
                                         for an enabled transmit/receive virtqueue whose index is \field{vq_index}.
 \end{enumerate}
 
-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
 virtqueue, regardless of the command used to set the parameters. Use the following command sequence
 with two pairs of virtqueues as an example:
@@ -1726,6 +1724,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vq_index} = 1, the device responds with coalescing parameters of index 1 set by command5.
 \end{itemize}
 
+The device can generate notifications more or less frequently
+than specified by the coalescing parameters.
+
 \subparagraph{Operation}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Operation}
 
 The device sends a used buffer notification once the notification conditions are met and if the notifications are not suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
@@ -1798,7 +1799,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 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.
 
-The behavior of the device in response to set commands of the VIRTIO_NET_CTRL_NOTF_COAL class is best-effort:
+The behavior of the device in response to specific coalescing parameters 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.

-- 
MST