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

Cornelia Huck
2024-06-11T14:04:32Z

On Mon, Jun 10 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jun 10, 2024 at 09:35:45PM +0800, Heng Qi wrote:
>> On Mon, 10 Jun 2024 14:46:02 +0200, Halil Pasic <pasic@linux.ibm.com> wrote:
>> > On Sat, 8 Jun 2024 10:34:22 +0800
>> > Heng Qi <hengqi@linux.alibaba.com> wrote:
>> >
>> > > On Fri, 7 Jun 2024 22:02:46 +0200, Halil Pasic <pasic@linux.ibm.com> wrote:
>> > > > On Tue, 28 May 2024 12:47:02 +0800
>> > > > Heng Qi <hengqi@linux.alibaba.com> wrote:
>> > > >
>> > > > > The device can set any initial coalescing parameters (0 or non-zero)
>> > > > > for the receive/send queue before the setting command is executed,
>> > > > > not just 0, enhancing device performance even without DIM enabled.
>> > > > >
>> > > > > So we need to clarify descriptions that don't fit the behavior.
>> > > >
>> > > > Sorry I'm late to the party -- again! Just for my understanding: how/why
>> > > > is this a clarification and not just a (basically incompatible) change?
>> > >
>> > > In my opinion, "clarification" means that something may have been described
>> > > incorrectly before, and we now need to discuss, explain clearly, and correct
>> > > the possibly incorrect description.
>> > >
>> >
>> > I figure the difference in perceived semantics of the word
>> > "clarification" is at the root of my confusion. Let us have a look at
>> > https://dictionary.cambridge.org/de/worterbuch/englisch/clarification
>> >
>> > According to my understanding a "clarification", while an improvement in
>> > ease of understanding and/or decrease of ambiguity (possibly to no
>> > ambiguity at all) implies that what receiving a clarification is not
>> > outright wrong.
>> >
>> > When rectifying something that is outright incorrect or wrong, I would
>> > refer to that with words like "correction", "fix", "erratum" or
>> > "corrigendum".
>> >
>> > > >
>> > > > I mean if I read this correctly, before the driver had the guaranty
>> > > > that if the parameters are not set by the driver, negotiating the
>> > > > feature does not introduce any coalescing. After this in theory
>> > > > the device could just pick some max value and potentially introduce
>> > > > maximal latency in certain scenarios.
>> > >
>> > > "maximum latency" also means "throughput improvement".
>> > >
>> >
>> > Under certain assumptions. But not necessarily. Again my concern is
>> > mostly the type of change. The virtio standard maintain a revision
>> > history appendix, and I would like to avoid the nature of this change
>> > being misrepresented there. If Connie and/or Michael think it is worth
>> > fixing, I believe it can be fixed with an editorial change.
>> >
>> > AFAIU VIRTIO_NET_F_NOTF_COAL and VIRTIO_NET_F_VQ_NOTF_COAL are about to
>> > land with virtio-1.3, i.e. there is no released/standardized virtio
>> > version where the "initialize to 0" is released. In that sense it looks
>> > like we are still on time to change this. But I am not 100% certain. In
>> > any case I don't think this as a huge impact and I'm fine going ahead
>> > with the change.
>>
>> Sorry for the late reply, I'm on vacation.
>>
>> I agree with this, and I prefer to release this patch as a correction for
>> virtio1.3 instead of a new patch for virtio1.4, because if devices support moderation coalescing based on virtio1.3,
>> and after virtio1.4 is released, these devices need to be updated again for a
>> more reasonable coalescing parameters.
>>
>> Cornelia and Michael, what do you think?
>
> The TC is just voting to start the public review process.
> We can defer that by a couple more weeks if there is
> a known issue to address.
>
> For that I expect we want a final patch and a couple of acks
> on list from TC members by end of the ballot, June 13.

FTR, I'd be fine with that if there's a consensus, but I'll not be
around to handle this personally. I also have no objection to proceeding
with the process.