Re: RE: RE: RE: RE: RE: RE: [PATCH v2] virtio-net: improve description of default coalescing parameters

[Heng Qi <hengqi@linux.alibaba.com>]

On Wed, 22 May 2024 11:49:00 +0800, Heng Qi <hengqi@linux.alibaba.com> wrote:
> On Wed, 22 May 2024 03:44:45 +0000, Parav Pandit <parav@nvidia.com> wrote:
> > 
> > 
> > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > Sent: Wednesday, May 22, 2024 7:42 AM
> > > 
> > > On Tue, 21 May 2024 13:55:19 +0000, Parav Pandit <parav@nvidia.com>
> > > wrote:
> > > >
> > > > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > > > Sent: Tuesday, May 21, 2024 3:34 PM
> > > > >
> > > > > On Tue, 21 May 2024 06:02:15 +0000, Parav Pandit <parav@nvidia.com>
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > > > > > Sent: Tuesday, May 21, 2024 7:54 AM On Tue, 14 May 2024 13:09:32
> > > > > > > +0000, Parav Pandit <parav@nvidia.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > > > > > > > Sent: Tuesday, May 14, 2024 1:23 PM
> > > > > > > > >
> > > > > > > > > On Tue, 14 May 2024 07:48:55 +0000, Parav Pandit
> > > > > > > > > <parav@nvidia.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > > > > > > > > > Sent: Tuesday, May 14, 2024 11:23 AM
> > > > > > > > > > >
> > > > > > > > > > > On Tue, 14 May 2024 03:59:52 +0000, Parav Pandit
> > > > > > > > > > > <parav@nvidia.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > > > Hi Heng,
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > > > > > > > > > > > Sent: Monday, May 13, 2024 6:27 PM
> > > > > > > > > > > > >
> > > > > > > > > > > > > Currently, a device may initialize each vq's
> > > > > > > > > > > > > coalescing parameters to empirically non-zero
> > > > > > > > > > > > > values, so the description of this part is supplemented for
> > > the virtio spec.
> > > > > > > > > > > > >
> > > > > > > > > > > > Currently the spec says to initialize the vq's
> > > > > > > > > > > > coalescing parameters to
> > > > > > > zero.
> > > > > > > > > > > >
> > > > > > > > > > > > Spec snippet: " Upon reset, a device MUST initialize
> > > > > > > > > > > > all coalescing
> > > > > > > > > > > parameters to 0."
> > > > > > > > > > > >
> > > > > > > > > > > > So I didn't understand why you say "current the device
> > > > > > > > > > > > may
> > > > > > > initialize".
> > > > > > > > > > > >
> > > > > > > > > > > > Do you mean, you want to change the spec to say, by
> > > > > > > > > > > > default the device has
> > > > > > > > > > > chosen to set zero or non-zero default parameter as it likes.
> > > > > > > > > > > > (and not zero).
> > > > > > > > > > >
> > > > > > > > > > > Yes.
> > > > > > > > > > >
> > > > > > > > > > > If a device supports interrupt coalescing moderation, it
> > > > > > > > > > > usually has a non- zero default value to provide better
> > > performance.
> > > > > > > > > > When VIRTIO_NET_F_VQ_NOTF_COAL or
> > > VIRTIO_NET_F_NOTF_COAL
> > > > > is
> > > > > > > not
> > > > > > > > > negotiated, the device can apply any notification coalescing value.
> > > > > > > > > > (assuming EVENT_IDX is off).
> > > > > > > > > >
> > > > > > > > > > But when they above features are negotiated, it is
> > > > > > > > > > expected that driver is going to driver it,
> > > > > > > > >
> > > > > > > > > The driver drives the runtime parameters without conflicting
> > > > > > > > > with the initial default values.
> > > > > > > > > And most users/drivers without dim enable may not drive it.
> > > > > > > > >
> > > > > > > > There is no conflict in runtime or default parameters.
> > > > > > > > For driver it starts after 10msec or 4 minutes, both are
> > > > > > > > runtime
> > > > > parameters.
> > > > > > > >
> > > > > > > > > The device need to be defensive about default performance.
> > > > > > > > >
> > > > > > > > The device can surely offer the default performance with non
> > > > > > > > zero default
> > > > > > > values.
> > > > > > >
> > > > > > > That's what I'm mentioning.
> > > > > > >
> > > > > > > >
> > > > > > > > > >in such case what is the good motivation to start with some
> > > > > > > > > >arbitrary
> > > > > > > value?
> > > > > > > > > >
> > > > > > > > > > > I'm trying to fix this, and we shouldn't force the
> > > > > > > > > > > device to always initialize with 0.
> > > > > > > > > > >
> > > > > > > > > > If driver is in control, driver decides what values to set.
> > > > > > > > > > And hence, arbitrary value is
> > > > > > > > >
> > > > > > > > > Then maybe the driver is never set and the device uses a 0
> > > > > > > > > which messes up most data scenarios.
> > > > > > > > >
> > > > > > > > If driver does not want to set it, why did it enable the
> > > > > > > > feature in first
> > > > > place?
> > > > > > > > So asking again, is it because driver enabled featured too
> > > > > > > > early in the driver
> > > > > > > load sequence, and not DIM is also disabled?
> > > > > > >
> > > > > > > I don't see the connection.
> > > > > > >
> > > > > > There is a connection.
> > > > > >
> > > > > > Basically, what you want is,
> > > > > >
> > > > > > 1. Driver negotiates feature bits during driver load.
> > > > > >
> > > > > > 2. Driver or user sometimes never configures the right coalescing
> > > > > parameters.
> > > > > >
> > > > > > 3. Device to continue to apply the best coalescing parameters,
> > > > > > typically keeps
> > > > > changing based on the workload.
> > > > >
> > > > > Wait. Why does the device keeping changing the coalescing parameters
> > > > > if the driver does not modify parameters? Do you mean some kind of
> > > > > hw-DIM running on the device?
> > > > >
> > > > Yes, If the device can choose one sane default one time to reduce
> > > performance regression, it likely can do few more times too.
> > > 
> > > Ok, I think we are now on a line:). I agree with this --> I saw that some
> > > devices have hw-DIM or device-side dim, but in general, many devices only
> > > have default values and no hardware DIM.
> > > 
> > I am not sure. each one has different view of how they see the device in their own and other cloud operators place.
> > The key part to me is, the device may apply some coalescing value until the driver tells him to stop applying its values.
> > The device may apply same value or different value, it is in the device boundary.
> > 
> > Hence, the below spec wording is what matters.
> > 
> > 
> > > More below.
> > > 
> > > >
> > > > > > The device to keep doing this until the driver takes the control
> > > > > > of doing it by-
> > > > > itself.
> > > > > > For example via net-dim enablement or user doing via ethtool.
> > > > >
> > > > > Right.
> > > > >
> > > > > >
> > > > > > So what is needed is, not just have _one_ default value, but the
> > > > > > device ability
> > > > > to constantly applying notification coalescing as desired.
> > > > >
> > > > > The "default parameters" I'm talking about is the "initial
> > > > > parameters", are we on the same line?
> > > > Yes, but it does not have to be the initial parameters if the intention is to
> > > avoid the performance regression.
> > > 
> > > I'm not sure if most devices carry hw-dim or device-side dim, but for those
> > > devices that are not with hw-dim, there will be a default value, whether it is
> > > 0 or non-zero. It is necessary to allow users to query this value.
> > > 
> > > Right?
> > > 
> > Yes, the driver can query and supply to users; whether that value is default or current, it depends on when the driver queries them.
> > So to keep it simple, driver always queries the current value before or after the VQ_SET command from driver point of view.
> > The device may apply any value before the VQ_SET command is done and driver should be able to query it as long as _F bit is negotiated.
> > It is straight forward.
> 
> +1
> 
> Everything looks clear.

Hi all!

A new version has been released with a new patch name:

https://lore.kernel.org/virtio-comment/20240523032332.50995-1-hengqi@linux.alibaba.com/T/#u

Thanks.

> 
> Thanks!
> 
> 
> > 
> > > >
> > > > >
> > > > > >
> > > > > > When the driver issues the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET
> > > > > command
> > > > > > for the first time onwards, at that point, the device receives the
> > > > > hint/indication, That now on, driver is in the control, hence the
> > > > > device should stop applying its own logic.
> > > > > >
> > > > > > Just one default is not enough.
> > > > > > And feature negotiation is of no help here at such an early stage.
> > > > > >
> > > > > > This is the limitation of feature bits negotiation and for now we
> > > > > > need to live it
> > > > > it.
> > > > > >
> > > > > > And heavy alternative that I prefer to avoid is keep the
> > > > > > notification features
> > > > > disabled at default, re-initialize the driver when user enables the
> > > > > notification coaleasing.
> > > > > > However, this is disruptive enough.
> > > > > >
> > > > > > So I propose that we draft the spec in device requirements as:.
> > > > > >
> > > > > > The device MAY apply any notification coalescing values when
> > > > > VIRTIO_NET_F_VQ_NOTF_COAL or VIRTIO_NET_F_NOTF_COAL is
> > > negotiated,
> > > > > until driver instruct the values for the first time.
> > > > > > Once the driver has performed VIRTIO_NET_F_VQ_NOTF_COAL
> > > command
> > > > > for a VQ or VIRTIO_NET_F_NOTF_COAL command for set of VQs, after
> > > > > that the device SHOULD generate notifications based on driver supplied
> > > parameters.
> > > > >
> > > > > I've always agreed with this.
> > > > >
> > > > Ok. so lets please rephase this proposal to define how a devices should
> > > behave.
> > > 
> > > Ok. V3 will rephase the text.
> > > 
> > Ok. thanks.
> > 
> > > >
> > > >
> > > > > >
> > > > > > > >
> > > > > > > > If feature is negotiated, than DIM can be enabled by default,
> > > > > > > > and therefore
> > > > > > > there is no need for the defaults?
> > > > > > >
> > > > > > > Right. But if DIM is not enabled (pls remember that DIM is just
> > > > > > > one of the application scenarios of VQ_NOTF_COAL, right?), the
> > > > > > > user can query the initial value of the device, which may be any value.
> > > > > > >
> > > > > > It is going to be just more than initial value, as device will
> > > > > > have to keep
> > > > > changing the value.
> > > > > >
> > > > > > > >
> > > > > > > > Or you are saying, that enabling DIM as default has some
> > > > > > > > issue, and until
> > > > > > > that point you prefer to have some default in the device?
> > > > > > >
> > > > > > > Partly true. The complete purpose is that the device can have
> > > > > > > any default values
> > > > > > > (0 or non-zero).
> > > > > > >
> > > > > > > >
> > > > > > > > > Cloud vendors cannot do this. And I don’t seem to see any
> > > > > > > > > interference with other devices from this proposal?
> > > > > > > > > If the device does not want a non-zero value, a value of 0
> > > > > > > > > can be used as the default value.
> > > > > > > > >
> > > > > > > > I didn’t understand above.
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > Especially when the DIM is disabled from the enabled
> > > > > > > > > > > state, allowing the driver to restore default parameters
> > > > > > > > > > > for device will prevent occasional performance degradation.
> > > > > > > > > > >
> > > > > > > > > > When DIM is disabled, do you mean driver does not have any
> > > > > > > > > > good
> > > > > > > defaults?
> > > > > > > > >
> > > > > > > > > DIM is not guaranteed to have a friendly value when disabled.
> > > > > > > > >
> > > > > > > > I didn’t follow.
> > > > > > > >
> > > > > > > > (a) When DIM is enabled, DIM decides the value.
> > > > > > > > (b) When DIM is disabled, zero is not good default in the device.
> > > > > > > > Did I
> > > > > > > understand right?
> > > > > > > >
> > > > > > > > I agree with (a) and (b).
> > > > > > >
> > > > > > > Right.
> > > > > > >
> > > > > > > >
> > > > > > > > > > I guess this issue surfaces from the feature negotiation
> > > > > > > > > > limitation that, it
> > > > > > > > > must be done early enough before DRIVER_OK.
> > > > > > > > > > And DIM is still disabled in driver.
> > > > > > > > >
> > > > > > > > > We lack the capability filed of default coalescing
> > > > > > > > > parameters, so GET is used to alleviate this.
> > > > > > > > >
> > > > > > > > When GET is not supported, ethtool should get -ENOSUPP instead of
> > > 0.
> > > > > > >
> > > > > > > We don't have a separate feature bit for GET, right? So if
> > > > > > > VQ_NOTF_COAL is negotiated, the device supports the GET
> > > command.
> > > > > > >
> > > > > > Yes, I meant to say that VIRTIO_NET_CTRL_NOTF_COAL and
> > > > > > VQ_NOTF_COAL
> > > > > both may not be offered.
> > > > > > When only VIRTIO_NET_CTRL_NOTF_COAL is done, the driver can
> > > report
> > > > > > -
> > > > > ENOSUPP.
> > > > > >
> > > > >
> > > > > I think "VIRTIO_NET_F_NOTF_COAL" and VQ_NOTF_COAL both may not
> > > be
> > > > > offered?
> > > > >
> > > > Right. Don’t see a value of F_NOTF_COAL, once the VQ_NOTF_COAL is
> > > offered.
> > > >
> > > > >
> > > > > >
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > That explains.
> > > > > > > > > >
> > > > > > > > > > Commit log needs to explain this limitation and the gain
> > > > > > > > > > by relaxing it to be
> > > > > > > > > non-zero.
> > > > > > > > > >
> > > > > > > > > > Any reason DIM is disabled by default in the driver?
> > > > > > > > > > If VQ_NOTF_COAL is supported by device, DIM should be
> > > > > > > > > > enabled by
> > > > > > > > > default.
> > > > > > > > >
> > > > > > > > > virtio-net support for netdim needs to be optimized. Of
> > > > > > > > > course, I also found that other network cards also have
> > > optimization points.
> > > > > > > > >
> > > > > > > > > I'm trying to enable DIM for upstream by default, after I've
> > > > > > > > > done the profile list tuning etc.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > > Since the VIRTIO_NET_F_NOTF_COAL feature does not
> > > > > > > > > > > > > provide a related GET command or config field, we
> > > > > > > > > > > > > still consider its default values
> > > > > > > > > to be 0.
> > > > > > > > > > > > >
> > > > > > > > > > > > The behavior for VIRTIO_NET_F_NOTF_COAL and
> > > > > > > > > > > VIRTIO_NET_F_VQ_NOTF_COAL can stay same for the default
> > > value.
> > > > > > > > > > > > Can you please explain the motivation for running
> > > > > > > > > > > > different default values
> > > > > > > > > > > for two different features in this commit log?
> > > > > > > > > > >
> > > > > > > > > > > The current spec leaves coalescing parameters for both
> > > features at 0.
> > > > > > > > > > >
> > > > > > > > > > > F_VQ_NOTF_COAL provides a GET command, that is, non-zero
> > > > > > > > > > > 0 can be obtained by the driver from the device, but
> > > > > > > > > > > F_NOTF_COAL does not have a similar GET command.
> > > > > > > > > > >
> > > > > > > > > > GET command doesn't give the ability to define the defaults.
> > > > > > > > > > So both can have non zero defaults.
> > > > > > > > >
> > > > > > > > > Makes sense.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > So when vq reset occurs or dim is disabled from the
> > > > > > > > > > > enabled state, the driver with F_VQ_NOTF_COAL negotiated
> > > > > > > > > > > has the ability to restore non-zero values, but only
> > > > > > > > > > > when F_NOTF_COAL is negotiated, the driver has no path
> > > > > > > > > > > to obtain the non-zero value, let alone restore non-zero
> > > > > > > > > values.
> > > > > > > > > > >
> > > > > > > > > > When F_NOTF_COAL is negotiated, if the device has its non
> > > > > > > > > > zero defaults,
> > > > > > > > > why does driver need to restore anything?
> > > > > > > > >
> > > > > > > > > Yes, device will set these.
> > > > > > > > >
> > > > > > > > Ok. so no restore needed.
> > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > > Suggested-by: Jason Wang <jasowang@redhat.com>
> > > > > > > > > > > > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > > > > > > > > > > > > ---
> > > > > > > > > > > > > v1->v2:
> > > > > > > > > > > > >   - Update description @Jason.
> > > > > > > > > > > > >
> > > > > > > > > > > > >  device-types/net/description.tex | 22
> > > > > > > > > > > > > +++++++++++++++-------
> > > > > > > > > > > > >  1 file changed, 15 insertions(+), 7 deletions(-)
> > > > > > > > > > > > >
> > > > > > > > > > > > > diff --git a/device-types/net/description.tex
> > > > > > > > > > > > > b/device- types/net/description.tex index
> > > > > > > > > > > > > 61cce1f..cf4b12c 100644
> > > > > > > > > > > > > --- a/device-types/net/description.tex
> > > > > > > > > > > > > +++ b/device-types/net/description.tex
> > > > > > > > > > > > > @@ -1805,6 +1805,10 @@ \subsubsection{Control
> > > > > > > > > > > > > Virtqueue}\label{sec:Device Types / Network Device /
> > > > > > > > > > > > > Devi
> > > > > > > > > > > > >
> > > > > > > > > > > > >  The device may generate notifications more or less
> > > > > > > > > > > > > frequently than specified by set commands of the
> > > > > > > > > > > > > VIRTIO_NET_CTRL_NOTF_COAL
> > > > > > > > > class.
> > > > > > > > > > > > >
> > > > > > > > > > > > > +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is
> > > > > > > > > > > > > +offered, the device may initialize the coalescing
> > > > > > > > > > > > > +parameters for each transmit or receive virtqueue
> > > > > > > > > > > > > +to non-zero values, otherwise, to 0, which are
> > > > > > > > > > > > > +called default
> > > > > > > > > > > > > coalescing parameters.
> > > > > > > > > > > > > +
> > > > > > > > > > > > Instead of 'offered', it should be 'negotiated'.
> > > > > > > > > > > > Because if may be offered,
> > > > > > > > > > > but driver may not have enabled it.
> > > > > > > > > > >
> > > > > > > > > > > I do not think so.
> > > > > > > > > > >
> > > > > > > > > > > Even if the driver does not negotiate, as long as the
> > > > > > > > > > > device has the ability to adjust coalescing parameters (i.e.
> > > > > > > > > > > offered), the device will initialize any values.
> > > > > > > > > > >
> > > > > > > > > > The device can initialize coalescing parameters regardless
> > > > > > > > > > of VQ_NOTF_COAL
> > > > > > > > > offered or not.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Right.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > >  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:
> > > > > > > > > > > > > @@ -1873,6 +1877,10 @@ \subsubsection{Control
> > > > > > > > > > > > > Virtqueue}\label{sec:Device Types / Network Device /
> > > > > > > > > > > > > Devi
> > > > > > > > > > > > >
> > > > > > > > > > > > >  The driver MUST ignore the values of coalescing
> > > > > > > > > > > > > parameters received from the
> > > > > > > > > > > > > VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET
> > > > > > > command
> > > > > > > > > > > > > if the device responds with VIRTIO_NET_ERR.
> > > > > > > > > > > > >
> > > > > > > > > > > > > +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is
> > > > > > > > > > > > > +negotiated, the driver
> > > > > > > > > > > > > MUST
> > > > > > > > > > > > > +get coalescing parameters for each enabled transmit
> > > > > > > > > > > > > +or receive virtqueue through the
> > > > > > > > > > > > > +VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET
> > > > > > > > > > > command
> > > > > > > > > > > > > after a successful device reset.
> > > > > > > > > > > > > +
> > > > > > > > > > > > The driver is free to not call GET call at all. It can always just
> > > do set.
> > > > > > > > > > > > So it cannot be must requirement for the driver to get them.
> > > > > > > > > > >
> > > > > > > > > > > This is mainly forced to MUST during the probe phase.
> > > > > > > > > > > Other timings, the existing spec has already described it clearly.
> > > > > > > > > > >
> > > > > > > > > > I don't see it is needed to force driver to read.
> > > > > > > > > > Why is it must for the driver to read it? It can operate without
> > > read.
> > > > > > > > >
> > > > > > > > > Otherwise the device has a non-zero default value, but users
> > > > > > > > > using ethtool -c see a value of 0.
> > > > > > > > >
> > > > > > > > When GET is supported, ethtool callback should query the current
> > > value.
> > > > > > > > Driver can read it once at start time, but it is not a MUST
> > > requirement.
> > > > > > > > Driver can read it runtime too. It is the driver implementation
> > > choice.
> > > > > > > > We don’t need to tell in the spec, when driver MUST read it.
> > > > > > >
> > > > > > > I agree with this.
> > > > > > >
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > The wording should be, the driver MAY get ...
> > > > > > > > > > > >
> > > > > > > > > > > > >  \devicenormative{\subparagraph}{Notifications
> > > > > > > > > > > > > Coalescing}{Device Types / Network Device / Device
> > > > > > > > > > > > > Operation / Control Virtqueue / Notifications
> > > > > > > > > > > > > Coalescing}
> > > > > > > > > > > > >
> > > > > > > > > > > > >  The device MUST ignore \field{reserved}.
> > > > > > > > > > > > > @@ -1884,18 +1892,18 @@ \subsubsection{Control
> > > > > > > > > > > > > Virtqueue}\label{sec:Device Types / Network Device /
> > > > > > > > > > > > > Devi The device MUST respond to
> > > > > > > VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET
> > > > > > > > > > > > > and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands
> > > with
> > > > > > > > > > > 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.
> > > > > > > > > > > > > -
> > > > > > > > > > > > > -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 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 MUST set default coalescing
> > > > > > > > > > > > > +parameters for all transmit or receive virtqueues.
> > > > > > > > > > > > > +
> > > > > > > > > > > > The device MAY set parameters to zero or non zero values..
> > > > > > > > > > >
> > > > > > > > > > > Ok.
> > > > > > > > > > >
> > > > > > > > > > > > I am still missing the motivation part of why to set
> > > > > > > > > > > > non zero value, even if
> > > > > > > > > > > the driver has negotiated the feature.
> > > > > > > > > > >
> > > > > > > > > > > Please see the above description.
> > > > > > > > > > >
> > > > > > > > > > If I understood is right, the case is:
> > > > > > > > > > _VQ_NOTF_COAL is negotiated, and DIM is disabled.
> > > > > > > > > > And device is unable to apply some good non zero defaults.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > If _VQ_NOTF_COAL is negotiated, and DIM is disabled:
> > > > > > > > >
> > > > > > > > > 1. The device wants to set a non-zero value for good perf,
> > > > > > > > >    but the user sees a value of 0.
> > > > > > > > >
> > > > > > > > The only limitation that we want to remove is, on device
> > > > > > > > reset, the device
> > > > > > > may have any default value.
> > > > > > >
> > > > > > > Right, and the driver may query the value using the GET command.
> > > > > > >
> > > > > > > >
> > > > > > > > > 2. When the user has not actively modified the value, DIM
> > > > > > > > > switches from
> > > > > > > the
> > > > > > > > >    enabled state to the disabled state, then the driver expects to
> > > issue a
> > > > > > > > >    setting command with a default value. To avoid
> > > > > > > > > performance
> > > > > > > degradation
> > > > > > > > >    caused by a bad value.
> > > > > > > > >
> > > > > > > > This #2 seems like some hack. Should I read the Linux code in this
> > > area?
> > > > > > >
> > > > > > > No. This is some kind of performance optimization at the code level.
> > > > > > > When DIM is turned off, the driver configures default values for
> > > > > > > the device to avoid any big performance regression.
> > > > > > >
> > > > > > If the driver configures the default, we don’t need device to
> > > > > > apply any
> > > > > defaults.
> > > > >
> > > > > when does the driver configure the default value (actually I mean
> > > > > the initial
> > > > > value) and how does the driver know which default (initial value)
> > > > > value applies to all devices?
> > > > >
> > > > I don’t think the driver can apply.
> > > > I understood that you wanted the driver to configure some default from
> > > > what you wrote as " When DIM is turned off, the driver configures default
> > > values for the device to avoid any big performance regression."
> > > 
> > > Maybe I didn't describe it clearly, consider the following scenario:
> > > 1. The driver may obtain the initial value. If not, the value is 0. ---> This initial
> > > value called default value.
> > Lets just keep it current value in the spec and keep it simple.
> > 
> > > 2. User query the coalescing parameters.  ---> Driver return the default value.
> > > 3. dim is disabled from enabled state -> Driver restores the device using the
> > > default value obtained from the device.
> > > 
> > Ok. looks fine to me as long as we keep it current value and say that the device may have any non zero or zero notification coalescing parameters before VQ_SET is done.
> > 
> > > Thanks.
> > > 
> > > >
> > > > > Thanks.
> > > > >
> > > > > > I thought your intention was, the driver does not know what is the
> > > > > > best
> > > > > notification rate to configure and feature is enabled in the device.
> > > > > > And the device does not have any good defaults (because driver is
> > > > > > not
> > > > > configuring any defaults).
> > > > > >
> > > > > > But here you say, the driver configures the drefault. I am lost. ☹
> > > > > >
> > > > > > > >
> > > > > > > > > We assume that most users do not have the prerequisite
> > > > > > > > > knowledge to modify coalecing parameters.
> > > > > > > > >
> > > > > > > > Right. Hence the DIM should be used.
> > > > > > > > When DIM is not used, a non zero default is good to have.
> > > > > > > > (without forcing the driver to read it).
> > > > > > >
> > > > > > > +1
> > > > > > >
> > > > > > > Thanks.
> > > > > > >
> > > > > > > >
> > > > > > > > > Thanks.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Right?
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > Thanks.
> > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > > +Regardless of whether the coalescing parameters
> > > > > > > > > > > > > +have been overridden by set commands from the
> > > > > > > > > VIRTIO_NET_CTRL_NOTF_COAL
> > > > > > > > > > > > > +class, upon
> > > > > > > > > > > > > disabling
> > > > > > > > > > > > > +and re-enabling a transmit or receive virtqueue,
> > > > > > > > > > > > > +the device MUST set the previous parameters for the
> > > virtqueue.
> > > > > > > > > > > > >
> > > > > > > > > > > > >  \paragraph{Device Statistics}\label{sec:Device
> > > > > > > > > > > > > Types / Network Device / Device Operation / Control
> > > > > > > > > > > > > Virtqueue / Device Statistics}
> > > > > > > > > > > > >
> > > > > > > > > > > > > --
> > > > > > > > > > > > > 2.32.0.3.g01195cf9f
> > > > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> > 
>