Re: [PATCH 1/1] migrate: Use forward table information to close() listening sockets

On Sat, Jan 31, 2026 at 10:47:28AM +0100, Stefano Brivio wrote:

On Fri, 30 Jan 2026 16:58:11 +1100 David Gibson wrote:

...

On incoming migrations we need to bind() reconstructed sockets to their correct local address. We can't do this if the origin passt instance is in the same namespace and still has those addresses bound. Arguably that's a bug in bind()s operation during repair mode, but for now we have to work around it.

So, to allow local-to-local migrations we close() sockets on the outgoing side as we process them. In addition to closing the connected socket we also have to close the associated listen()ing socket, because that can also cause an address conflict.

To do that, we introduced the listening_sock field in the connection state, because we had no other way to find the right listening sockets. Now that we have the forwarding table, we have a complete list of listening sockets elsewhere. We can use that instead, to close all listening sockets on outbound migration, rather than just the ones that might conflict.

This is cleaner and, importantly, saves a valuable 32-bits in the flow state structure. It does mean that there is a longer window where a peer attempting to connect during migration might get a Connection Refused. I think this is an acceptable trade-off for now: arguably we should not allow local-to-local migrations in any case, since the socket closes make it impossible to safely roll back migration as per the qemu model.

Signed-off-by: David Gibson --- flow.c | 12 ++++++++++++ fwd.c | 21 +++++++++++++++++++++ fwd.h | 1 + tcp.c | 9 --------- tcp_conn.h | 3 --- 5 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/flow.c b/flow.c index fd4d5f38..5207143d 100644 --- a/flow.c +++ b/flow.c @@ -1023,6 +1023,9 @@ static int flow_migrate_source_rollback(struct ctx *c, unsigned bound, int ret)

debug("...roll back migration");

+ if (fwd_listen_sync(c, &c->tcp.fwd_in, PIF_HOST, IPPROTO_TCP) < 0) + die("Failed to re-establish listening sockets"); + foreach_established_tcp_flow(flow) { if (FLOW_IDX(flow) >= bound) break; @@ -1147,6 +1150,15 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,

Nit: the comment to this function currently says "Send data (flow table) for flow, close listening". I fixed that up (dropped ", close listening").

Good point, thanks.

...

return flow_migrate_source_rollback(c, FLOW_MAX, rc); }

+ /* HACK: A local to local migrate will fail if the origin passt has the + * listening sockets still open when the destination passt tries to bind + * them. This does mean there's a window where we lost our listen()s, + * even if the migration is rolled back later. The only way to really + * fix that is to not allow local to local migration, which arguably we + * should (use namespaces for testing instead). */

Actually, we already use namespaces in the current tests,

Oh, nice.

but we didn't (always) do that during development, and it might be convenient in general to have the possibility to test *a part* of the implementation using the same namespace as long as it's reasonably cheap (it seems to be).

Depends what cost you're talking about. It's cheap in terms of computational complexity, and code compexity. It means, however, that we can't necessarily roll back failed migrations - i.e. resume on the origin system. That isn't really correct for the qemu migration model, which is why I think allowing local migrations probably isn't the best idea, at least by default.

That's just a part because anyway bind() and connect() will conflict, if we're in the same namespace, which is a kernel issue you already noted:

Well, it's a kernel issue that the bound listen()ing sockets conflict with the half-constructed flow sockets. Having the listening sockets of the origin passt conflict with the listening sockets of the destination passt is pretty much expected, and would still be an impediment to local migration.

https://pad.passt.top/p/TcpRepairTodo#L3 Repair mode sockets should not have address conflicts with non-repair sockets (both bind() and connect())

but even that part is convenient to have, I think,

I'm not really sure what you mean by that.

so I'm a bit worried that somebody might take this comment as a to-do item, while I don't think it should be.

Patch applied anyway, to give this as much testing time and exposure as possible.

-- Stefano

-- 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