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

[Halil Pasic <pasic@linux.ibm.com>]

On Sun, 30 Jun 2024 12:55:35 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Sat, Jun 29, 2024 at 08:47:47AM +0200, Halil Pasic wrote:
[..] 
> > > > 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.
> 

No I did not miss it, and I don't think that the two sentences being in
adjacent paragraphs makes it clear that this is separate from
suppression.

For me the "notification conditions" part and the "even" is making it
sound like normally meeting these conditions would mean that a
notification should be sent but then notification suppression overrules
that, so there will be no notification even though if it wasn't for
the suppression there would be one.
 
[..]
> 
> 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?
> 

I don't think so. Sorry, there are things I would like to understand
before committing. Back then when I voted on the coalescing I convinced
myself that the interaction between used buffer notification suppression
and used buffer notification coalescing on the TX and RX queues of
virtio-net is specified with enough clarity. Now I'm asking myself
if I came to that conclusion along the lines "nothing else would make
any sense, not spelling it out properly is not tragic". Please
see my comment on Jason's proposal.

Regards,
Halil