Here is some more work on the migration stuff. Two of these are fixes, alas, for the already merged "safe" patches. Then there's some extra debugging. David Gibson (3): migrate: Fix several errors with passt-repair migrate, tcp: Report more error conditions during connection repair tcp: Simplify handling of getsockname() .gitignore | 1 + Makefile | 2 +- passt-repair.c | 2 +- tcp.c | 35 ++++++++++++++++------------------- 4 files changed, 19 insertions(+), 21 deletions(-) -- 2.48.1
The passt-repair helper is now merged, but alas it contains several small
bugs:
* close() is not in the seccomp profile, meaning it will immediately
SIGSYS when you make a request of it
* The generated header, seccomp_repair.h isn't listed in .gitignore or
removed by "make clean"
Fixes: 8c24301462c3 ("Introduce passt-repair")
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
.gitignore | 1 +
Makefile | 2 +-
passt-repair.c | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/.gitignore b/.gitignore
index 5824a71e..3c16adc7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,5 +7,6 @@
/qrap
/pasta.1
/seccomp.h
+/seccomp_repair.h
/c*.json
README.plain.md
diff --git a/Makefile b/Makefile
index fde66701..d4e10967 100644
--- a/Makefile
+++ b/Makefile
@@ -117,7 +117,7 @@ valgrind: all
.PHONY: clean
clean:
- $(RM) $(BIN) *~ *.o seccomp.h pasta.1 \
+ $(RM) $(BIN) *~ *.o seccomp.h seccomp_repair.h pasta.1 \
passt.tar passt.tar.gz *.deb *.rpm \
passt.pid README.plain.md
diff --git a/passt-repair.c b/passt-repair.c
index 767a8211..dd8578f5 100644
--- a/passt-repair.c
+++ b/passt-repair.c
@@ -46,7 +46,7 @@
*
* Return: 0 on success (EOF), 1 on error, 2 on usage error
*
- * #syscalls:repair connect setsockopt write exit_group
+ * #syscalls:repair connect setsockopt write close exit_group
* #syscalls:repair socket s390x:socketcall i686:socketcall
* #syscalls:repair recvfrom recvmsg arm:recv ppc64le:recv
* #syscalls:repair sendto sendmsg arm:send ppc64le:send
--
2.48.1
On Tue, 4 Feb 2025 16:42:13 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:The passt-repair helper is now merged, but alas it contains several small bugs: * close() is not in the seccomp profile, meaning it will immediately SIGSYS when you make a request of it * The generated header, seccomp_repair.h isn't listed in .gitignore or removed by "make clean" Fixes: 8c24301462c3 ("Introduce passt-repair") Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>Applied. -- Stefano
This should help debugging problems with migration.
[This is a candidate to be folded into the earlier patch]
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
tcp.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tcp.c b/tcp.c
index 3760e67a..7ce0f4cb 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1011,10 +1011,17 @@ int tcp_flow_repair_connect(struct ctx *c, struct tcp_tap_conn
*conn)
{
struct flowside *tgt = &conn->f.side[TGTSIDE];
struct tcp_repair_opt opts[2];
+ int rc;
- tcp_flow_repair_seq(c, conn, true);
+ rc = tcp_flow_repair_seq(c, conn, true);
+ if (rc)
+ return rc;
- flowside_connect(c, conn->sock, PIF_HOST, tgt);
+ rc = flowside_connect(c, conn->sock, PIF_HOST, tgt);
+ if (rc) {
+ err("Failed to connect repaired socket: %s", strerror_(errno));
+ return rc;
+ }
/* FIXME: Fetch those with TCP_REPAIR_OPTIONS and store in migration
* data. These hardcoded values just happen to be good enough.
--
2.48.1
For migration we need to get the specific local address and port for
connected sockets with getsockname(). We currently open code marshalling
the results into the flow entry.
However, we already have inany_from_sockaddr() which handles the fiddly
parts of this, so use it. Also report failures, which may make debugging
problems easier.
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
tcp.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/tcp.c b/tcp.c
index 7ce0f4cb..dbdaa9d9 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1712,24 +1712,14 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t
af,
tcp_epoll_ctl(c, conn);
if (c->mode == MODE_VU) { /* To rebind to same oport after migration */
- if (af == AF_INET) {
- struct sockaddr_in s_in;
-
- sl = sizeof(s_in);
- if (!getsockname(s, (struct sockaddr *)&s_in, &sl)) {
- /* NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) */
- tgt->oport = ntohs(s_in.sin_port);
- tgt->oaddr = inany_from_v4(s_in.sin_addr);
- }
- } else {
- struct sockaddr_in6 s_in6;
+ union sockaddr_inany sa;
+ socklen_t sl = sizeof(sa);
- sl = sizeof(s_in6);
- if (!getsockname(s, (struct sockaddr *)&s_in6, &sl)) {
- /* NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) */
- tgt->oport = ntohs(s_in6.sin6_port);
- tgt->oaddr.a6 = s_in6.sin6_addr;
- }
+ if (!getsockname(s, &sa.sa, &sl)) {
+ inany_from_sockaddr(&tgt->oaddr, &tgt->oport, &sa);
+ } else {
+ err("Failed to get local address for socket: %s",
+ strerror_(errno));
}
}
--
2.48.1
On Tue, 4 Feb 2025 16:42:15 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:For migration we need to get the specific local address and port for connected sockets with getsockname(). We currently open code marshalling the results into the flow entry. However, we already have inany_from_sockaddr() which handles the fiddly parts of this, so use it. Also report failures, which may make debugging problems easier. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/tcp.c b/tcp.c index 7ce0f4cb..dbdaa9d9 100644 --- a/tcp.c +++ b/tcp.c @@ -1712,24 +1712,14 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af, tcp_epoll_ctl(c, conn); if (c->mode == MODE_VU) { /* To rebind to same oport after migration */ - if (af == AF_INET) { - struct sockaddr_in s_in; - - sl = sizeof(s_in); - if (!getsockname(s, (struct sockaddr *)&s_in, &sl)) { - /* NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) */ - tgt->oport = ntohs(s_in.sin_port); - tgt->oaddr = inany_from_v4(s_in.sin_addr); - } - } else { - struct sockaddr_in6 s_in6; + union sockaddr_inany sa; + socklen_t sl = sizeof(sa);This shadows the previous declaration of 'sl'. Turning into an assignment.- sl = sizeof(s_in6); - if (!getsockname(s, (struct sockaddr *)&s_in6, &sl)) { - /* NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) */ - tgt->oport = ntohs(s_in6.sin6_port); - tgt->oaddr.a6 = s_in6.sin6_addr; - } + if (!getsockname(s, &sa.sa, &sl)) { + inany_from_sockaddr(&tgt->oaddr, &tgt->oport, &sa); + } else { + err("Failed to get local address for socket: %s", + strerror_(errno)); } }-- Stefano
On Tue, 4 Feb 2025 16:42:15 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:For migration we need to get the specific local address and port for connected sockets with getsockname(). We currently open code marshalling the results into the flow entry. However, we already have inany_from_sockaddr() which handles the fiddly parts of this, so use it. Also report failures, which may make debugging problems easier. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>Applied with re-declarations of 'sa' and 'sl' dropped. -- Stefano