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

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

On Mon, 17 Jun 2024 16:31:25 -0700, "Si-Wei Liu" <si-wei.liu@oracle.com> wrote:
> 
> 
> On 6/16/2024 7:27 PM, Heng Qi wrote:
> > On Thu, 13 Jun 2024 02:13:50 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> On Tue, Jun 11, 2024 at 05:43:18PM +0000, Parav Pandit wrote:
> >>>
> >>>> From: Michael S. Tsirkin <mst@redhat.com>
> >>>> Sent: Tuesday, June 11, 2024 10:00 PM
> >>>
> >>>> How we can we make progress with
> >>>> the realease but sure we don't make backwards compat a pain?
> >>>>
> >>>> Ideas?
> >>>>
> >>> There is no functional break with this relaxation.
> >>> Device set some non-zero defaults and driver didn't modify them....
> >>> Anything broken? Unlikely.
> Generally it's inappropriate to leave this decision making to the device 
> for what would be the best / most performant default config, as the 
> device is generally considered agnostic to the guest load...

Instead, the performance of the virtual machine and the driver depends heavily
on how the device is implemented, just as we have proposed various ways to
offload the data queue in the device to the hardware. The reason why most
devices use software to simulate ctrlq instead of using hardware offload is
that the driver has no requirements for the performance of ctrlq before, that is,
the device implementation is responsible for and meets the driver's performance
requirements.


>Unless the 
> device is specially hard wired to some fixed guest setup that users 
> couldn't change, it doesn't seem logical that the device could derive 
> the best or most performant config on driver's behalf. What if the guest 
> wants best latency for its load but the device just blindlessly guess 
> the guest might prefer throughput friendly that it miserably uses 
> latency impacting non-zero default? 

The device does not want to guess and cannot guess. This patch does not force
the device to choose a non-zero value, but relaxes it to allow the device to
choose 0 or non-zero, which is very friendly to virtual machines with different
performance requirements, right?

>Could this device side change for 
> the default config regress boot time performance (which may need best 
> latency over throughput)?

Don't make these assumptions, what if the driver needs better throughput?

> 
> >>> And device/driver has better performance, is that a problem? Unlikely.
> >>>
> Even for rare case with a hard wired setup, the way to tackle the very 
> problem using device's default is still quite questionable. Usually the 
> mgmt software or network config utility should be equipped with some 
> default value if need be. And we know the guest has the best position to 
> impose the best / most performant config for its own load. What is the 
> issue or use case that this initial config couldn't be applied by the 
> guest mgmt software ahead but has to resort to the device to load some 
> default (which is odd and irrelevant to any guest load), before the 
> interface is brought up for operation i.e. performing I/O?

Use cases are everywhere, Alibaba Cloud, MLX and all other modern network cards
have a default value that is not 0.
(0 is actually a kind of default value)

> 
> > Sorry, my vacation just ended.
> >
> >> Yes, it is possible. Driver can cache values it sets
> >> and never query device with get.
> > Don't we already have a lot of behaviors to drive queries from devices?
> > RSS context, device stats.
> >
> >> Before anything is set, driver will report incorrect values.
> > Devices that are widely supported and supported by good practices should have
> > any initialization value. Just reporting 0 is incorrect value. Although the
> > spec now says so.
> I don't have an aligned view here, sorry. As I recall having 0 as the 
> default is just to keep device started in a state where coalescing is 
> disabled, so it's backward compatible and consistent with a 
> non-coalescing supporting driver - such that it won't yield surprising 
> effect (for e.g. regressed latency) inadvertently after user's getting 
> driver software upgraded. Unlike the other virtio-net features that 
> could 100% improve performance in all aspect, this coalescing feature is 
> more of a performance tuning knob that may improve performance metrics 
> (such as cpu usage or throughput) of one dimension while demoting the 
> others (such as latency, jitter or connection rate) from the equation. 
> That said, there's not a single and fixed set of default config that 
> device could supply which is able to satisfy all kind of guest load. 
> Rather than rely on the device to offer a matching default for driver 
> (which I think it's technically wrong), I'd lean to having guest 
> software or network utility to apply the initial config for the guest, 
> where they should have best knowledge for the specific guest workload 
> than what device could do.

Before this feature, a good device implementation should also support coalescing
(of course we don't necessarily assume it has coalescing). In addition, virtual
machines that tend to favor latency and throughput exist. If the device supported
by the manufacturer needs to provide a low-latency virtual machine, please
continue to keep the default value of 0.

> 
> >
> >> What will break as a result? Hard to predict.
> >>
> >>
> >> Given the patch is big, I am inclined to say it just should use
> >> a feature bit.
> 
> > This doesn't seem to break anything in my opinion, we just told the device that
> > now you can set more values.
> Another thing this could break is live migration between devices with 
> varied default. How do you make sure the guest doesn't rely on some 
> default from the source device, while on the destination it just doesn't 
> get the same default coalescingjjj value? To get rid of this side effect 
> the guest would still need to apply the initial config for its own, 
> anyway... Which eventually would render this proposal with arbitrary 
> default rather pointless.

I don't quite understand why this would affect hot migration, the values
would be migrated over.

Thanks.

> 
> 
> Thanks,
> -Siwei
> >
> > Using new feature bits does not seem necessary.
> >
> > Thanks.
> >
> >> -- 
> >> MST
> >>
>