Re: [virtio-comment] Re: [PATCH v2] virtio-net: support the virtqueue coalescing moderation

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

On Sat, Feb 11, 2023 at 12:18:45PM +0200, Alvaro Karsz wrote:
> > >> Please add short description something like,
> > >>
> > >> When the driver prefers to use per virtqueue notifications coalescing, and if queue group (transmit or receive) level notification coalescing is enabled, driver SHOULD first disable device level notification coalescing.
> > >> Or it should be,
> > >>
> > > I disagree here.
> > > IMO "queue group level notification coalescing" is not something to
> > > enable or disable, but a shortcut to set all TX/RX queues at once.
> > > Why should the spec force a driver to "disable device level
> > > notification coalescing" (I assume you mean send a
> > > VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)?
> > > What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET
> > > command, and then a single queue traffic increases? why should it zero
> > > the parameters to all other queues?
> >
> > Hi, Alvaro! Thanks for your reply!
> >
> > I think Parav refers more to the scene where ethool sets parameters at
> > the queue group level and
> > the scene where netdim sets parameters for a single queue. In this
> > scenario, netdim should really
> > determine the coalescing parameters of the device, and the parameters at
> > the queue group level set
> > by ethtool should be ignored (many drivers are designed this way, such
> > as mlx), that is, we need to give netdim the right,
> > because it It has the ability to dynamically adjust parameters.
> > (However, I think this friendly constraint is also possible in driver
> > implementation.)
> >
> > Of course, if we consider setting coalescing parameters at the queue
> > group level and single queue level separately through ethtool,
> > then as you said, we should not set any priority for them.
> >
> > Back to reality, I think the function of ethtool to set single queue
> > parameters may come later, which is thankless for users because of netdim.
> >
> > Therefore, if our specification tends to be practical, we can add
> > Parav's proposal, and if our specification tends to be more general,
> > then hand over
> > the constraints to the driver implementation. what do you think?
> 
> Hi,
> I understand your and Parav's point, and that this is needed for netdim.
> I just think that the spec should not dictate this, and this should be
> implementation specific.
> What if a OS with a different adaptive functionality wants to support
> these features?
> 
> This is just my opinion though.

I'm not sure the specific property is even a feature and not a bug.
One can argue e.g. that ethtool should fail with EBUSY rather
than being silently overwritten.
Thats why I feel this kind of policy is best set by driver.

-- 
MST