From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202502 header.b=AfdZyIR1; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id C2D245A0008 for ; Wed, 26 Feb 2025 01:29:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1740529733; bh=kcfmOOUqkDX+jQYFknP198M4zqura1MZICJfGgpV4vg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=AfdZyIR1gY+ncWkWUXaqzGzxfDhG+KeSHOlNrXd7mbLxBSIqhzels38sCHuR6MMS2 ogVtnq79coxnnhxJYZR4IrVBuLYCyLNQtT/lsO0XP7SI+M43KShy52X4FWT44tZtfk SCqC99ur9qwa2nHz8llKBRaUrIo4iUbTcQOhfrTIsgT1HlDTRKCDvb1cUcfko7IEzu 6lXY9QfT2qWA3EonJ2p4F7/23Q4+wk2ghwNAzM4IjsBwxaWdhdyi3Fl6/IBQ0s9ODj vmTzaA1AXN0y0UNxS9j4botWR7ottBUzYRDNG/NrwdoIvpV3RnfmZLhNRVhRhE/6Mk yIGmhFU9ibmeA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Z2b1d27ZZz4wyh; Wed, 26 Feb 2025 11:28:53 +1100 (AEDT) Date: Wed, 26 Feb 2025 11:27:32 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 0/2] More graceful handling of migration without passt-repair Message-ID: References: <20250225055132.3677190-1-david@gibson.dropbear.id.au> <20250225184316.407247f4@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0N/lmFf26lDueIjX" Content-Disposition: inline In-Reply-To: <20250225184316.407247f4@elisabeth> Message-ID-Hash: T24V3AKTLDQXSYFBCX5XZSAHZLLCCBIX X-Message-ID-Hash: T24V3AKTLDQXSYFBCX5XZSAHZLLCCBIX X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --0N/lmFf26lDueIjX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 25, 2025 at 06:43:16PM +0100, Stefano Brivio wrote: > On Tue, 25 Feb 2025 16:51:30 +1100 > David Gibson wrote: >=20 > > From Red Hat internal testing we've had some reports that if > > attempting to migrate without passt-repair, the failure mode is uglier > > than we'd like. The migration fails, which is somewhat expected, but > > we don't correctly roll things back on the source, so it breaks > > network there as well. > >=20 > > Handle this more gracefully allowing the migration to proceed in this > > case, but allow TCP connections to break > >=20 > > I've now tested this reasonably: > > * I get a clean migration if there are now active flows > > * Migration completes, although connections are broken if > > passt-repair isn't connected > > * Basic test suite (minus perf) > >=20 > > I didn't manage to test with libvirt yet, but I'm pretty convinced the > > behaviour should be better than it was. >=20 > I did, and it is. The series looks good to me and I would apply it as > it is, but I'm waiting a bit longer in case you want to try out some > variations based on my tests as well. Here's what I did. [snip] Thanks for the detailed instructions. More complex than I might have liked, but oh well. > $ virsh migrate --verbose --p2p --live --unsafe alpine --tunneled qemu+= ssh://88.198.0.161:10951/session > Migration: [97.59 %]error: End of file while reading data: : Input/outp= ut error >=20 > ...despite --verbose the error doesn't tell much (perhaps I need > LIBVIRT_DEBUG=3D1 instead?), but passt terminates at this point. With > this series (I just used 'make install' from the local build), migration > succeeds instead: >=20 > $ virsh migrate --verbose --p2p --live --unsafe alpine --tunneled qemu+= ssh://88.198.0.161:10951/session > Migration: [100.00 %] >=20 > Now, on the target, I still have to figure out how to tell libvirt > to start QEMU and prepare for the migration (equivalent of '-incoming' > as we use in our tests), instead of just starting a new instance like > it does. Otherwise, I have no chance to start passt-repair there. > Perhaps it has something to do with persistent mode described here: Ah. So I'm pretty sure virsh migrate will automatically start qemu with --incoming on the target. IIUC the problem here is more about timing: we want it to start it early, so that we have a chance to start passt-repair and let it connect before the migration actually happens. Crud... I didn't think of this before. I don't know that there's any sensible way to do this without having libvirt managing passt-repair as well. I mean it's not impossible there's some option to do this, but I doubt there's been any reason before for something outside of libvirt to control the timing of the target qemu's creation. I think we need to ask libvirt people about this. > https://libvirt.org/migration.html#configuration-file-handling Yeah.. I don't think this is relevant. > and --listen-address, but I'm not quite sure yet. >=20 > That is, I could only test different failures (early one on source, or > later one on target) with this, not a complete successful migration. >=20 > > There are more fragile cases that I'm looking to fix, particularly the > > die()s in flow_migrate_source_rollback() and elsewhere, however I ran > > into various complications that I didn't manage to sort out today. > > I'll continue looking at those tomorrow. I'm now pretty confident > > that those additional fixes won't entirely supersede the changes in > > this series, so it should be fine to apply these on their own. >=20 > By the way, I think the somewhat less fragile/more obvious case where > we fail clumsily is when the target doesn't have the same address as > the source (among other possible addresses). In that case, we fail (and > terminate) with a rather awkward: Ah, yes, that is a higher priority fragile case. > 93.7217: ERROR: Failed to bind socket for migrated flow: Cannot assig= n requested address > 93.7218: ERROR: Flow 0 (TCP connection): Can't set up socket: (null),= drop > 93.7331: ERROR: Selecting TCP_SEND_QUEUE, socket 1: Socket operation = on non-socket > 93.7333: ERROR: Unexpected reply from TCP_REPAIR helper: -100 >=20 > that's because, oops, I only took care of socket() failures in > tcp_flow_repair_socket(), but not bind() failures (!). Sorry. No, you check for errors on both. The problem is that in tcp_flow_migrate_target() we cancel the flow allocation and carry on - but the source will still send information for this flow, putting us out of sync with the stream. > Once that's fixed, flow_migrate_target() should also take care of > decreasing 'count' accordingly. I just had a glimpse but didn't > really try to sketch a fix. Adjusting count won't do the job. Instead we'd need to keep the flow around, but marked as "dead" somehow, so that we read but discard the incoming information for it. The MIGRATING state I added in one of my drafts was supposed to help with this sort of thing. But that's quite a complex change. Hrm... at least in the near term, I think it might actually be easier to set IP_FREEBIND when we create sockets for in-migrating flows. That way we can process them normally, they just won't do much without the address set. It has the additional advantage that it should work if the higher layers only move the IP just after the migration, instead of in advance. --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --0N/lmFf26lDueIjX Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAme+X+MACgkQzQJF27ox 2GftIg//Y0ho3a4Fo2Ksgzu2G2vKMymuyy5KKIeMJ51PY+RlRLy1OildzYbtk5VU dLf6XSfyX+0fHllll1vvSwRy+d163SOuQAQfclpwqwqQeHBJ3yWmtcexZCM8yC56 xYJDgK4N/1og5UDaRswimLEqDo+SoKEznMynCj9PTvD8pVABX0V2l0i1EOHWzlez C6s/YcrSVyZzHGPgC4AUpuok2H+zDV14B3vtV0rJzTJdbxLkvsARTQWhRMxHxfOl GBocoCx59a9SaHRjkmYj3jD7vz1U2eS4LkFEeva5jGPnMA53ieP6Z91OIfVZynjz TEeONwltUxK1iSPR7RwyM6pBTqIcaEUu3swwfYh+x84qnZRjraj9vFQeEiIfY2Eu rmea+MKcaG3BMY/uQvSzaeTL4ar3I1OW3JPx/c5wPcdSK8/Ozluzg7h2uADZ3/hR BgGtypVdfHdGmxA95r2LBCYxFNgF7TPRiiTdY+7Rb0NvFWX+aCldJ+l19kbT/K+8 XV78WxpvwDGtw2niGA2iB0rbHCXHwhgQt4xLNzYFOSzAgHgd/QIe3NqT/DB8r6Pv noZlM/2IWzJqUTEkOPZ2DIgQwjr6Udfj6HUHnVKkJXhiI8E2AmPMreyDKDwXOkeM P/qYgqz5JgcI2tj++T9RAEbm2iWyRhXBvU0nmz1VeDQx5VbJ9LI= =ciIZ -----END PGP SIGNATURE----- --0N/lmFf26lDueIjX--