Re: [patch net-next] virtio_net: add support for Byte Queue Limits

[Jiri Pirko <jiri@resnulli.us>]

Mon, Jun 17, 2024 at 03:44:55AM CEST, jasowang@redhat.com wrote:
>On Mon, Jun 10, 2024 at 10:19 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Fri, Jun 07, 2024 at 01:30:34PM +0200, Jiri Pirko wrote:
>> > Fri, Jun 07, 2024 at 12:23:37PM CEST, mst@redhat.com wrote:
>> > >On Fri, Jun 07, 2024 at 11:57:37AM +0200, Jiri Pirko wrote:
>> > >> >True. Personally, I would like to just drop orphan mode. But I'm not
>> > >> >sure others are happy with this.
>> > >>
>> > >> How about to do it other way around. I will take a stab at sending patch
>> > >> removing it. If anyone is against and has solid data to prove orphan
>> > >> mode is needed, let them provide those.
>> > >
>> > >Break it with no warning and see if anyone complains?
>> >
>> > This is now what I suggested at all.
>> >
>> > >No, this is not how we handle userspace compatibility, normally.
>> >
>> > Sure.
>> >
>> > Again:
>> >
>> > I would send orphan removal patch containing:
>> > 1) no module options removal. Warn if someone sets it up
>> > 2) module option to disable napi is ignored
>> > 3) orphan mode is removed from code
>> >
>> > There is no breakage. Only, hypotetically performance downgrade in some
>> > hypotetical usecase nobody knows of.
>>
>> Performance is why people use virtio. It's as much a breakage as any
>> other bug. The main difference is, with other types of breakage, they
>> are typically binary and we can not tolerate them at all.  A tiny,
>> negligeable performance regression might be tolarable if it brings
>> other benefits. I very much doubt avoiding interrupts is
>> negligeable though. And making code simpler isn't a big benefit,
>> users do not care.
>
>It's not just making code simpler. As discussed in the past, it also
>fixes real bugs.
>
>>
>> > My point was, if someone presents
>> > solid data to prove orphan is needed during the patch review, let's toss
>> > out the patch.
>> >
>> > Makes sense?
>>
>> It's not hypothetical - if anything, it's hypothetical that performance
>> does not regress.  And we just got a report from users that see a
>> regression without.  So, not really.
>
>Probably, but do we need to define a bar here? Looking at git history,
>we didn't ask a full benchmark for a lot of commits that may touch

Moreover, there is no "benchmark" to run anyway, is it?


>performance.
>
>Thanks
>
>>
>> >
>> > >
>> > >--
>> > >MST
>> > >
>>
>