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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 09:44:55 +0800
From: Jason Wang <jasowang@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: Jiri Pirko <jiri@...nulli.us>, Jason Xing <kerneljasonxing@...il.com>, 
	Heng Qi <hengqi@...ux.alibaba.com>, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, xuanzhuo@...ux.alibaba.com, 
	virtualization@...ts.linux.dev, ast@...nel.org, daniel@...earbox.net, 
	hawk@...nel.org, john.fastabend@...il.com, netdev@...r.kernel.org
Subject: Re: [patch net-next] virtio_net: add support for Byte Queue Limits

On Mon, Jun 10, 2024 at 10:19 PM Michael S. Tsirkin <mst@...hat.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@...hat.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
performance.

Thanks

>
> >
> > >
> > >--
> > >MST
> > >
>