linux-kernel - Re: [PATCH v9 4/4] vhost: Add a KConfig knob to enable IOCTL VHOST_FORK_FROM

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CACGkMEteBReoezvqp0za98z7W3k_gHOeSpALBxRMhjvj_oXcOw@mail.gmail.com>
Date: Wed, 30 Apr 2025 11:34:49 +0800
From: Jason Wang <jasowang@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: Cindy Lu <lulu@...hat.com>, michael.christie@...cle.com, sgarzare@...hat.com, 
	linux-kernel@...r.kernel.org, virtualization@...ts.linux-foundation.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH v9 4/4] vhost: Add a KConfig knob to enable IOCTL VHOST_FORK_FROM_OWNER

On Tue, Apr 29, 2025 at 6:56 PM Michael S. Tsirkin <mst@...hat.com> wrote:
>
> On Tue, Apr 29, 2025 at 11:39:37AM +0800, Jason Wang wrote:
> > On Mon, Apr 21, 2025 at 11:46 AM Jason Wang <jasowang@...hat.com> wrote:
> > >
> > > On Mon, Apr 21, 2025 at 11:45 AM Jason Wang <jasowang@...hat.com> wrote:
> > > >
> > > > On Mon, Apr 21, 2025 at 10:45 AM Cindy Lu <lulu@...hat.com> wrote:
> > > > >
> > > > > Introduce a new config knob `CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL`,
> > > > > to control the availability of the `VHOST_FORK_FROM_OWNER` ioctl.
> > > > > When CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL is set to n, the ioctl
> > > > > is disabled, and any attempt to use it will result in failure.
> > > >
> > > > I think we need to describe why the default value was chosen to be false.
> > > >
> > > > What's more, should we document the implications here?
> > > >
> > > > inherit_owner was set to false: this means "legacy" userspace may
> > >
> > > I meant "true" actually.
> >
> > MIchael, I'd expect inherit_owner to be false. Otherwise legacy
> > applications need to be modified in order to get the behaviour
> > recovered which is an impossible taks.
> >
> > Any idea on this?
> >
> > Thanks
>
> At this point, as we changed the behaviour, we have two types of legacy applications
> - ones expecting inherit_owner false
> - ones expecting inherit_owner true

Considering vhost has been used for more than a decade, I would expect
more will expect inhert_owner to be false.

>
> Whatever we do, some of these will have to be changed.

If we must choose one to break, I'd expect to break less.

> Given current
> kernel has it as true, and given it is a cleaner behaviour that will
> keep working when we disable CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL in 10
> years, I think it's the better default.

The problem is, if we set it to true and only an ioctl will bring the
old behavior back. Who will use this ioctl? We can't modify all legacy
applications.

> If you want to change it transparently, look for ways to
> distinguish between the two types.
>
> The application in question is qemu, is it not?

Looks not, it's the application or management layer that tries to set
the affinity to the owner of the vhost.

For example, if I start a testpmd + vhost_net and pinn testpmd runs on
cpu0. I will get half of the performance since vhost will contend with
cpu with testpmd.

> I do not see how sticking an ioctl call into its source is such
> a big deal, if this is what we want to do.
> A bit of short term pain but we get clear maintainable semantics.

What's more important, the way that introduces new fork behaviors
without a new uAPI is a bug. We need to fix that by introducing new
uAPI for the behaviour. This seems to be the short term pain instead
of introducing new uAPI for the old behaviour.

Thanks

>
> --
> MST
>