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

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

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?

> >
> > 4. the virtio-net driver even after negotiating VIRTIO_NET_F_VQ_NOTF_COAL, still kept the its dim disabled.
> > vi->rq[queue].dim_enabled = false by default.
> >
> > 5. The virtio net driver enables dim only on user request via ethtool.
> >
> > So here, just because one is using a new driver and new device, it may get lower performance.  
> Impossible, older driver / device doesn't have coalescing advertise / 
> enabled, and new device & new driver with coalescing negotiated starts 
> safe with 0 with coalescing disabled effectively, how can it get lower 
> performance?

Si-Wei and Parav, I think it is the other way around. Without the
change proposed here, with no user/management software intervention
assumed, we are guaranteed to not see any performance regression
but also no performance benefit because of the coalescing mechanism,
because with zeros as default it is essentially dormant.

With the change proposed here however, we open up the possibility
to see either a performance improvement or a performance degradation
depending on the defaults chosen by the device and the workload (sill
assuming new device & new driver & no intervention by management
software).


> 
> >
> > Do we agree to above problem description?  
> This narrative has two problems:
> 
> - The term "lower performance" is misleading and inaccurate, the fact or 
> implication of coalescing is that it yields worse latency, causing 
> additional delay and jitter that would hurt quite a lot of real world 
> workload that is latency sensitive.  So, rather than say coalescing 
> helps performance, it should clearly tell the truth that the coalescing 
> parameter is no more than a tuning knob that has performance 
> impact/implication, being positive or negative it is very subject to the 
> guest workload. So the default disposition or initial parameters loaded 
> from device just reflects the preference of the host admin/ device 
> owner/ cloud vendor of one's own, for e.g. it is optimized for 
> throughput oriented load versus latency oriented. Be noted, generally 
> end users don't care what coalescing is about, they do not connect 
> performance to "throughput" or "bandwidth" as that in your mind.
> 

Si-Wei, I sympathies with your argument but I'm willing to tolerate this
change nevertheless.

Yes the device and via the device whoever controls these parameters can
shoot the guest in the foot, if the guest admin is not going out of his
way to do something about controlling coalescing.

But via the same mechanism we could end up in a better place as well.

Because I assume that the device and its "admin" are at least trying to
do good, I'm not straight out against this proposal, as long nothing
wonky happens with the code which is already out there, in a sense of a
split brain scenario where Linux thinks those values are 0 but they
aren't. Unfortunately can't tell you how we stand on this.

[..]
> > If no, I likely missed something in the long discussion and need to read all of it. :(
> >
> > If yes, few solutions are:
> > 1. A user intervention is must to avoid this regression. The user in the guest VM must enable DIM if the device supports it.

I don't think burdening up the users with avoiding regressions is a good
idea.

Regards,
Halil

> > Very hard to do this plumbing.
> >
> > 2. A net driver enables the DIM by default as long as VIRTIO_NET_F_VQ_NOTF_COAL is negotiated.
> > (without ethtool setting)
> >
> > 3. A device relaxes the limitation and continue to apply the coalescing, until driver overrides it.
> >
> > In my humble opinion, Heng is solving it using option #3, that tends to work with existing and future drivers who may/may not enable the DIM.
> >
> >
> >
> >  
>