netdev - Re: [PATCH V6 mlx5-next 10/15] vfio: Remove migration protocol v1

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]

Date:   Tue, 01 Feb 2022 14:26:29 +0100
From:   Cornelia Huck <cohuck@...hat.com>
To:     Jason Gunthorpe <jgg@...dia.com>
Cc:     Yishai Hadas <yishaih@...dia.com>, alex.williamson@...hat.com,
        bhelgaas@...gle.com, saeedm@...dia.com, linux-pci@...r.kernel.org,
        kvm@...r.kernel.org, netdev@...r.kernel.org, kuba@...nel.org,
        leonro@...dia.com, kwankhede@...dia.com, mgurtovoy@...dia.com,
        maorg@...dia.com
Subject: Re: [PATCH V6 mlx5-next 10/15] vfio: Remove migration protocol v1

On Tue, Feb 01 2022, Jason Gunthorpe <jgg@...dia.com> wrote:

> On Tue, Feb 01, 2022 at 01:39:23PM +0100, Cornelia Huck wrote:
>> On Tue, Feb 01 2022, Jason Gunthorpe <jgg@...dia.com> wrote:
>> 
>> > On Tue, Feb 01, 2022 at 12:23:05PM +0100, Cornelia Huck wrote:
>> >> On Sun, Jan 30 2022, Yishai Hadas <yishaih@...dia.com> wrote:
>> >> 
>> >> > From: Jason Gunthorpe <jgg@...dia.com>
>> >> >
>> >> > v1 was never implemented and is replaced by v2.
>> >> >
>> >> > The old uAPI definitions are removed from the header file. As per Linus's
>> >> > past remarks we do not have a hard requirement to retain compilation
>> >> > compatibility in uapi headers and qemu is already following Linus's
>> >> > preferred model of copying the kernel headers.
>> >> 
>> >> If we are all in agreement that we will replace v1 with v2 (and I think
>> >> we are), we probably should remove the x-enable-migration stuff in QEMU
>> >> sooner rather than later, to avoid leaving a trap for the next
>> >> unsuspecting person trying to update the headers.
>> >
>> > Once we have agreement on the kernel patch we plan to send a QEMU
>> > patch making it support the v2 interface and the migration
>> > non-experimental. We are also working to fixing the error paths, at
>> > least least within the limitations of the current qemu design.
>> 
>> I'd argue that just ripping out the old interface first would be easier,
>> as it does not require us to synchronize with a headers sync (and does
>> not require to synchronize a headers sync with ripping it out...)
>
> We haven't worked out the best way to organize the qemu patch series,
> currently it is just one patch that updates everything together, but
> that is perhaps a bit too big...
>
> I have thought that a 3 patch series deleting the existing v1 code and
> then readding it is a potential option, but we don't change
> everything, just almost everything..

Even in that case, removing the old code and adding the new one is
probably much easier to review. (Also, you obviously need to have the
header update in between those two stages.)

>
>> > The v1 support should remain in old releases as it is being used in
>> > the field "experimentally".
>> 
>> Of course; it would be hard to rip it out retroactively :)
>> 
>> But it should really be gone in QEMU 7.0.
>
> Seems like you are arguing from both sides, we can't put the v2 in to
> 7.0 because Linus has not accepted it but we have to rip the v1 out
> even though Linus hasn't accepted that?
>
> We can certainly defer the kernels removal patch for a release if it
> makes qemu's life easier?

No, I'm only talking about the QEMU implementation (i.e. the code that
uses the v1 definitions and exposes x-enable-migration). Any change in
the headers needs to be done via a sync with upstream Linux.

>
>> Considering adding the v2 uapi, we might get unlucky: The Linux 5.18
>> merge window will likely be in mid-late March (and we cannot run a
>> headers sync before the patches hit Linus' tree), while QEMU 7.0 will
>> likely enter freeze in mid-late March as well. So there's a non-zero
>> chance that the new uapi will need to be deferred to 7.1.
>
> Usually in rdma land we start advancing the user side once the kernel
> patches hit the kernel maintainer tree, not Linus's. I run a
> non-rebasing tree so that gives a permanent git hash. It works well
> enough and avoids these kinds of artificial delays.

QEMU policy is "it must be in Linus' tree [*]", because we run a full
header sync. We have been bitten by premature updates in the
past. Updates of only parts of the headers are only acceptable during
development of a patch series, and must be marked as "will be replaced
with a proper header sync".

[*] Preferrably a (full or -rc) release, but the very minimum is a git
hash from his tree.