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=202412 header.b=gKbqPIhy; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id ED5015A0624 for ; Tue, 28 Jan 2025 02:52:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202412; t=1738029116; bh=lL8mB1RZM51BVnXO1f61rMFaAXB3KPS2Vho8bPVAkLA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=gKbqPIhypZpgymisSxUywhmMKYSl+aXrnlZQP1WAPTCQ02t+U9gjh6bISCLHD9jKm kRE15fwuOBzXVxAb5dixw1skcDzzASTrJLZy2mUANHhOWspNnYZ9+X3vdRLE4w77JI qH93qLQpUYmlUrpba9lrSmBc3YXkGU/SlVvqn8lM1wbEtfaSL8aO1xtdoVWsH4ucpu D2Zq+yot5Gxu4irDchDvA4ppkdNSnvbbAZXY6Za9AN2vd9KNtNx8FWz7bZLTtdaEPw 0pEEskOo3PdDr/rAQMHC2T+xRR4eeo8NwmuyYYyMHFnZ/hOsOQecPaX23+YbIAmjbn mgOHG3t1ujwpQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4YhpDr04dFz4x0G; Tue, 28 Jan 2025 12:51:56 +1100 (AEDT) Date: Tue, 28 Jan 2025 12:51:59 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 7/7] Introduce passt-repair Message-ID: References: <20250127231532.672363-1-sbrivio@redhat.com> <20250127231532.672363-8-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="jgLMI5oOw1S8d3QC" Content-Disposition: inline In-Reply-To: <20250127231532.672363-8-sbrivio@redhat.com> Message-ID-Hash: PI7B5K5OEEF637QKTLULSG3FRE64MLCN X-Message-ID-Hash: PI7B5K5OEEF637QKTLULSG3FRE64MLCN 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, Laurent Vivier 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: --jgLMI5oOw1S8d3QC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 28, 2025 at 12:15:32AM +0100, Stefano Brivio wrote: > A privileged helper to set/clear TCP_REPAIR on sockets on behalf of > passt. Not used yet. >=20 > Signed-off-by: Stefano Brivio > --- > Makefile | 10 +++-- > passt-repair.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 118 insertions(+), 3 deletions(-) > create mode 100644 passt-repair.c >=20 > diff --git a/Makefile b/Makefile > index 1383875..1b71cb0 100644 > --- a/Makefile > +++ b/Makefile > @@ -42,7 +42,8 @@ PASST_SRCS =3D arch.c arp.c checksum.c conf.c dhcp.c dh= cpv6.c flow.c fwd.c \ > tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c \ > vhost_user.c virtio.c vu_common.c > QRAP_SRCS =3D qrap.c > -SRCS =3D $(PASST_SRCS) $(QRAP_SRCS) > +PASST_REPAIR_SRCS =3D passt-repair.c > +SRCS =3D $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) > =20 > MANPAGES =3D passt.1 pasta.1 qrap.1 > =20 > @@ -72,9 +73,9 @@ mandir ?=3D $(datarootdir)/man > man1dir ?=3D $(mandir)/man1 > =20 > ifeq ($(TARGET_ARCH),x86_64) > -BIN :=3D passt passt.avx2 pasta pasta.avx2 qrap > +BIN :=3D passt passt.avx2 pasta pasta.avx2 qrap passt-repair > else > -BIN :=3D passt pasta qrap > +BIN :=3D passt pasta qrap passt-repair > endif > =20 > all: $(BIN) $(MANPAGES) docs > @@ -101,6 +102,9 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% > qrap: $(QRAP_SRCS) passt.h > $(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) -DARCH=3D\"$(TARGET_ARCH)\" $(QRAP= _SRCS) -o qrap $(LDFLAGS) > =20 > +passt-repair: $(PASST_REPAIR_SRCS) > + $(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repa= ir $(LDFLAGS) > + > valgrind: EXTRA_SYSCALLS +=3D rt_sigprocmask rt_sigtimedwait rt_sigactio= n \ > rt_sigreturn getpid gettid kill clock_gettime mmap \ > mmap2 munmap open unlink gettimeofday futex statx \ > diff --git a/passt-repair.c b/passt-repair.c > new file mode 100644 > index 0000000..e9b9609 > --- /dev/null > +++ b/passt-repair.c > @@ -0,0 +1,111 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > + > +/* PASST - Plug A Simple Socket Transport > + * for qemu/UNIX domain socket mode > + * > + * passt-repair.c - Privileged helper to set/clear TCP_REPAIR on sockets > + * > + * Copyright (c) 2025 Red Hat GmbH > + * Author: Stefano Brivio > + * > + * Connect to passt via UNIX domain socket, receive sockets via SCM_RIGH= TS along > + * with commands mapping to TCP_REPAIR values, and switch repair mode on= or > + * off. Reply by echoing the command. Exit if the command is INT_MAX. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in = UAPI */ > + > +int main(int argc, char **argv) > +{ > + char buf[CMSG_SPACE(sizeof(int) * SCM_MAX_FD)] > + __attribute__ ((aligned(__alignof__(struct cmsghdr)))); > + struct sockaddr_un a =3D { AF_UNIX, "" }; > + int cmd, fds[SCM_MAX_FD], s, ret, i; > + struct cmsghdr *cmsg; > + struct msghdr msg; > + struct iovec iov; > + > + iov =3D (struct iovec){ &cmd, sizeof(cmd) }; I mean, local to local, it's *probably* fine, but still a network protocol not defined in terms of explicit width fields makes me nervous. I'd prefer to see the cmd being a packed structure with fixed width elements. I also think we should do some sort of basic magic / version exchange. I don't see any reason we'd need to extend the protocol, but I'd rather have the option if we have to. Plus checking a magic number should make things less damaging and more debuggable if you were to point the repair helper at an entirely unrelated unix socket instead of passt's repair socket. > + msg =3D (struct msghdr){ NULL, 0, &iov, 1, buf, sizeof(buf), 0 }; > + cmsg =3D CMSG_FIRSTHDR(&msg); > + > + if (argc !=3D 2) { > + fprintf(stderr, "Usage: %s PATH\n", argv[0]); > + return -1; > + } > + > + ret =3D snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]); > + if (ret <=3D 0 || ret >=3D (int)sizeof(a.sun_path)) { > + fprintf(stderr, "Invalid socket path: %s\n", argv[1]); > + return -1; > + } > + > + if ((s =3D socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { Hmm.. would a datagram socket better suit our needs here? > + perror("Failed to create AF_UNIX socket"); > + return -1; > + } > + > + if (connect(s, (struct sockaddr *)&a, sizeof(a))) { > + fprintf(stderr, "Failed to connect to %s: %s\n", argv[1], > + strerror(errno)); > + return -1; > + } > + > + while (1) { > + int n; > + > + if (recvmsg(s, &msg, 0) < 0) { > + perror("Failed to receive message"); > + return -1; > + } > + > + if (!cmsg || > + cmsg->cmsg_len < CMSG_LEN(sizeof(int)) || > + cmsg->cmsg_len > CMSG_LEN(sizeof(int) * SCM_MAX_FD) || > + cmsg->cmsg_type !=3D SCM_RIGHTS) > + return -1; > + > + n =3D cmsg->cmsg_len / CMSG_LEN(sizeof(int)); > + memcpy(fds, CMSG_DATA(cmsg), sizeof(int) * n); > + > + switch (cmd) { > + case INT_MAX: > + return 0; > + case TCP_REPAIR_ON: > + case TCP_REPAIR_OFF: > + case TCP_REPAIR_OFF_NO_WP: > + for (i =3D 0; i < n; i++) { > + if (setsockopt(fds[i], SOL_TCP, TCP_REPAIR, > + &cmd, sizeof(int))) { > + perror("Setting TCP_REPAIR"); > + return -1; We probably want this to report errors back to passt, rather than just dying in this case. That way if for some weird reason one socket can't be placed in repair mode, we can still migrate all the other connections. > + } > + } > + > + /* Confirm setting by echoing the command back */ > + if (send(s, &cmd, sizeof(int), 0) < 0) { > + fprintf(stderr, "Reply to command %i: %s\n", > + cmd, strerror(errno)); > + return -1; > + } > + > + break; > + default: > + fprintf(stderr, "Unsupported command 0x%04x\n", cmd); > + return -1; > + } > + } > +} --=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 --jgLMI5oOw1S8d3QC Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmeYOD4ACgkQzQJF27ox 2Gc+Bw/9Haxl1oEgcOs93afSqqUQcCi8ND2Or+uVxlPJatLHty+kvlJ6Gm0MS6g5 QzMqIG027EKH3i29bXee7rXsghK1GLNjtUBgCMv8s6tbdqxQtD/G3WTcqJLdkUIt ztZGwxWYfg03RFbIyXGbesPCBDuJQ9DfdNxE3lEw/UT66UfZjXd24TS8+w2cbWl/ ptFiIVmrLdUPttXLRSaOpazGV5B93wLcK8smL2h5xTx0Eu7KWF93MUzA6OPcgxty n98wHlGTW8C6qPq0gMMk9/yKL3xWRJM/n7k8p8x3U+oFI5qoHblX+dAIEr0FHlgh O+56EBqvE6iqefGDAMprGGAOTAKLP7lefVMd1uIo+vsqPwZ8OwXweTJzoaC2fqlx njvgXDrpKGYnp96kagaVl6/AgoZZtPPlME5Djqhns4Yh7x36t+ELljjpYxf/LpFn mur4nhkt6IxkDovByjSASrpx5td1vs9rRGbjFSrYR7hfr4vqZL3oh//XxnpBnnL0 G/JOfH91W31RVV1i62/AYREuoY9baaGFMrNUOx6d4wWV26GM8+obW31zF8FLirYQ mrTc+P02FJOouinyQtG4vHL36Klhxhryqz5yS3VXEvsoBzuOysEmiBLEYfwTXdk0 84zcMZ7wxBYrNJBXAbafgNIB2/JYgmbCYuthi+HxQ0wpPGhhjMM= =uBZf -----END PGP SIGNATURE----- --jgLMI5oOw1S8d3QC--