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

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

On Sat, Jun 29, 2024 at 08:47:47AM +0200, Halil Pasic wrote:
> On Thu, 27 Jun 2024 08:35:16 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > 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.
> 
> The less frequently could be understood like due to notification
> suppression.

I think you are missing that there is a separate text about notification suppression
immediately after it:
	A device SHOULD NOT send used buffer notifications to the driver
	if the notifications are suppressed, even if the notification conditions
	are met.
I think that makes it clear this text is separate from suppression.


> > 
> > 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.
> 
> To me best-effort implies making an effort. Here we talk about
> intentionally ignoring the parameter values. For me stuff like not being
> precise about the counting or timekeeping, or running into the danger of
> having to drop packets because the queue is about to fill up, would
> qualify as best-effort deviations from the specified behavior.
> 
> I believe we want the drivers written under the assumption that the
> workings of a notification coalescing in any device that implements it
> are close enough to what is described in the spec. Just my opinion.
> 
> Regards,
> Halil

OK, I guess we can clarify:
	in particular, the device MAY coalesce notifications when
	coalescing parameters are set to 0.

will that make it better?

-- 
MST