[PATCH 0/4] RFC: bind() migrated connections in repair mode
Downstream testing recently discovered that inbound connections can't be migrated properly, because the new socket gets an address conflict with its corresponding listening socket. It turns out this can be avoided by delaying bind() until after we're already in repair mode. Patch 4/4 is the actual fix here. Patch 3/4 adds a test program checking the behaviour to doc/platform-requirements. Patches 1 & 2 fix minor problems I spotted in doc/platform-requirements writing 3/4. Only 4/4 will need to be backported. David Gibson (4): platform requirements: Fix clang-tidy warning platform requirements: Add attributes to die() function platform requirements: Add test for address conflicts with TCP_REPAIR migrate, tcp: bind() migrated sockets in repair mode doc/platform-requirements/.gitignore | 1 + doc/platform-requirements/Makefile | 4 +- doc/platform-requirements/common.h | 1 + doc/platform-requirements/listen-vs-repair.c | 128 ++++++++++++++++++ .../reuseaddr-priority.c | 6 +- tcp.c | 38 ++++-- 6 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 doc/platform-requirements/listen-vs-repair.c -- 2.49.0
Recent clang-tidy versions complain about enums defined with some but not
all entries given explicit values. I'm not entirely convinced about
whether that's a useful warning, but in any case we really don't need the
explicit values in doc/platform-requirements/reuseaddr-priority.c, so
remove them to make clang happy.
Signed-off-by: David Gibson
Add both format string and ((noreturn)) attributes to the version of die()
used in the test programs in doc/platform-requirements. As well as
potentially catching problems in format strings, this means that the
compiler and static checkers can properly reason about the fact that it
will exit, preventing bogus warnings.
Signed-off-by: David Gibson
Simple test program to check the behaviour we need for bind() address
conflicts between listening sockets and repair mode sockets.
Signed-off-by: David Gibson
Currently on a migration target, we create then immediately bind() new
sockets for the TCP connections we're reconstructing. Mostly, this works,
since a socket() that is bound but hasn't had listen() or connect() called
is essentially passive. However, this bind() is subject to the usual
address conflict checking. In particular that means if we already have
a listening socket on that port, we'll get an EADDRINUSE. This will happen
for every connection we try to migrate that was initiated from outside to
the guest, since we necessarily created a listening socket for that case.
We set SO_REUSEADDR on the socket in an attempt to avoid this, but that's
not sufficient; even with SO_REUSEADDR address conflicts are still
prohibited for listening sockets. Of course once these incoming sockets
are fully repaired and connect()ed they'll no longer conflict, but that
doesn't help us if we fail at the bind().
We can avoid this by not calling bind() until we're already in repair mode
which suppresses this transient conflict. Because of the batching of
setting repair mode, to do that we need to move the bind to a step in
tcp_flow_migrate_target_ext().
Signed-off-by: David Gibson
On Wed, 2 Apr 2025 14:13:15 +1100
David Gibson
Downstream testing recently discovered that inbound connections can't be migrated properly, because the new socket gets an address conflict with its corresponding listening socket. It turns out this can be avoided by delaying bind() until after we're already in repair mode.
Patch 4/4 is the actual fix here. Patch 3/4 adds a test program checking the behaviour to doc/platform-requirements. Patches 1 & 2 fix minor problems I spotted in doc/platform-requirements writing 3/4. Only 4/4 will need to be backported.
David Gibson (4): platform requirements: Fix clang-tidy warning platform requirements: Add attributes to die() function platform requirements: Add test for address conflicts with TCP_REPAIR migrate, tcp: bind() migrated sockets in repair mode
Applied. -- Stefano
participants (2)
-
David Gibson
-
Stefano Brivio