On Wed, Jan 29, 2025 at 09:46:10AM +0100, Stefano Brivio wrote: > On Wed, 29 Jan 2025 17:09:07 +1100 > David Gibson wrote: > > > On Wed, Jan 29, 2025 at 12:39:39AM +0100, Stefano Brivio wrote: > > > In vhost-user mode, by default, create a second UNIX domain socket > > > accepting connections from passt-repair, with the usual listener > > > socket. > > > > > > When we need to set or clear TCP_REPAIR on sockets, we'll send them > > > via SCM_RIGHTS to passt-repair, who sets the socket option values we > > > ask for. > > > > > > To that end, introduce batched functions to request TCP_REPAIR > > > settings on sockets, so that we don't have to send a single message > > > for each socket, on migration. When needed, repair_flush() will > > > send the message and check for the reply. > > > > > > Signed-off-by: Stefano Brivio > > > --- > > > Makefile | 12 ++-- > > > conf.c | 46 ++++++++++-- > > > epoll_type.h | 4 ++ > > > passt.1 | 11 +++ > > > passt.c | 9 +++ > > > passt.h | 7 ++ > > > repair.c | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > > repair.h | 16 +++++ > > > tap.c | 65 +---------------- > > > util.c | 62 +++++++++++++++++ > > > util.h | 1 + > > > 11 files changed, 353 insertions(+), 72 deletions(-) > > > create mode 100644 repair.c > > > create mode 100644 repair.h > > > > > > diff --git a/Makefile b/Makefile > > > index 1b71cb0..f67a20b 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -38,9 +38,9 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) > > > > > > PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd.c \ > > > icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \ > > > - ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c tap.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 > > > + ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c \ > > > + repair.c tap.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 = qrap.c > > > PASST_REPAIR_SRCS = passt-repair.c > > > SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) > > > @@ -50,9 +50,9 @@ MANPAGES = passt.1 pasta.1 qrap.1 > > > PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h flow.h fwd.h \ > > > flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h isolation.h \ > > > lineread.h log.h migrate.h ndp.h netlink.h packet.h passt.h pasta.h \ > > > - pcap.h pif.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h tcp_internal.h \ > > > - tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h udp_vu.h util.h \ > > > - vhost_user.h virtio.h vu_common.h > > > + pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h \ > > > + tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h \ > > > + udp_vu.h util.h vhost_user.h virtio.h vu_common.h > > > HEADERS = $(PASST_HEADERS) seccomp.h > > > > > > C := \#include \nint main(){int a=getrandom(0, 0, 0);} > > > diff --git a/conf.c b/conf.c > > > index df2b016..85dec44 100644 > > > --- a/conf.c > > > +++ b/conf.c > > > @@ -816,6 +816,9 @@ static void usage(const char *name, FILE *f, int status) > > > " UNIX domain socket is provided by -s option\n" > > > " --print-capabilities print back-end capabilities in JSON format,\n" > > > " only meaningful for vhost-user mode\n"); > > > + FPRINTF(f, > > > + " --repair-path PATH path for passt-repair(1)\n" > > > > Nit: as a privileged helper, should it be passt-repair(8)? > > So, I spent a couple of minutes on this as I wrote this, and I spent a > bit longer now: the most authoritative definition I can find of section > 8 is from man-pages(7): > > 8 System management commands > Commands like mount(8), many of which only root can > execute. > > It's not really a system management command, and the idea is to run it > with CAP_NET_ADMIN, but not necessarily as root. So I would rather keep > it in section 1, unless there's some other conflicting definition I'm > not aware of. Yeah oh, on that basis (1) makes more sense than (8). > There's also the topic of where it should be installed (/sbin, > /usr/sbin/, /bin, /usr/bin). I'd pick /usr/bin, because /sbin doesn't > really mean much nowadays, and it's anyway fitting with the FHS 3.0: > > Utilities used for system administration (and other root-only > commands) are stored in /sbin, /usr/ sbin, and /usr/local/sbin. > > /sbin contains binaries essential for booting, restoring, recovering, > and/or repairing the system in addition to the binaries in /bin. > 18 Programs executed after /usr is known to be mounted (when there > are no problems) are generally placed into /usr/sbin. > > ...and I don't think this helper would qualify for /sbin or /usr/sbin. > > > > > > + " default: append '.repair' to UNIX domain path\n"); > > > } > > > > > > FPRINTF(f, > > > @@ -1240,8 +1243,30 @@ static void conf_nat(const char *arg, struct in_addr *addr4, > > > */ > > > static void conf_open_files(struct ctx *c) > > > { > > > - if (c->mode != MODE_PASTA && c->fd_tap == -1) > > > - c->fd_tap_listen = tap_sock_unix_open(c->sock_path); > > > + if (c->mode != MODE_PASTA && c->fd_tap == -1) { > > > + c->fd_tap_listen = sock_unix(c->sock_path); > > > + > > > + if (c->mode == MODE_VU && strcmp(c->repair_path, "none")) { > > > + if (!strncmp(c->repair_path, "./", 2)) { > > > + memmove(c->repair_path, c->repair_path + 2, > > > + sizeof(c->repair_path) - 2); > > > + } > > > > Do you need this? Shouldn't "./whatever" be usable as-is? > > Ah, yes, I didn't know. I explicitly added this for the '--repair-path > ./none' case, but it's not actually needed. I'll drop this. > > > > + > > > + if (!*c->repair_path && > > > + snprintf_check(c->repair_path, > > > + sizeof(c->repair_path), "%s.repair", > > > + c->sock_path)) { > > > + warn("passt-repair path %s not usable", > > > + c->repair_path); > > > > I'd prefer a die() here - I think omitting a possibly expected > > feature, with just a warning that could easily be lost in the logs is > > not a good idea. > > I was thinking about that, but should we really risk *not* starting > because with ".repair" the path is now too long, for a feature that, > realistically, most users won't actually use? > > If we're started by any kind of framework (which is where we run the > risk of the warning being ignored), then there should be explicit > checks about the path and the usability of passt-repair (or equivalent). Hm, yeah, I guess. Honestly I'm not sure which option will cause us less trouble. > > > + c->fd_repair_listen = -1; > > > + } else { > > > + c->fd_repair_listen = sock_unix(c->repair_path); > > > + } > > > + } else { > > > + c->fd_repair_listen = -1; > > > + } > > > + c->fd_repair = -1; > > > + } > > > > > > if (*c->pidfile) { > > > c->pidfile_fd = output_file_open(c->pidfile, O_WRONLY); > > > @@ -1354,9 +1379,12 @@ void conf(struct ctx *c, int argc, char **argv) > > > {"host-lo-to-ns-lo", no_argument, NULL, 23 }, > > > {"dns-host", required_argument, NULL, 24 }, > > > {"vhost-user", no_argument, NULL, 25 }, > > > + > > > /* vhost-user backend program convention */ > > > {"print-capabilities", no_argument, NULL, 26 }, > > > {"socket-path", required_argument, NULL, 's' }, > > > + > > > + {"repair-path", required_argument, NULL, 27 }, > > > { 0 }, > > > }; > > > const char *logname = (c->mode == MODE_PASTA) ? "pasta" : "passt"; > > > @@ -1824,8 +1852,8 @@ void conf(struct ctx *c, int argc, char **argv) > > > if (c->ifi4 && IN4_IS_ADDR_UNSPECIFIED(&c->ip4.guest_gw)) > > > c->no_dhcp = 1; > > > > > > - /* Inbound port options & DNS can be parsed now (after IPv4/IPv6 > > > - * settings) > > > + /* Inbound port options, DNS, and --repair-path can be parsed now, after > > > + * IPv4/IPv6 settings and --vhost-user. > > > */ > > > fwd_probe_ephemeral(); > > > udp_portmap_clear(); > > > @@ -1871,6 +1899,16 @@ void conf(struct ctx *c, int argc, char **argv) > > > } > > > > > > die("Cannot use DNS address %s", optarg); > > > + } else if (name == 27) { > > > + if (c->mode != MODE_VU && strcmp(optarg, "none")) > > > + die("--repair-path is for vhost-user mode only"); > > > + > > > + if (snprintf_check(c->repair_path, > > > + sizeof(c->repair_path), "%s", > > > + optarg)) > > > + die("Invalid passt-repair path: %s", optarg); > > > + > > > + break; > > > } > > > } while (name != -1); > > > > > > diff --git a/epoll_type.h b/epoll_type.h > > > index fd9eac3..706238a 100644 > > > --- a/epoll_type.h > > > +++ b/epoll_type.h > > > @@ -42,6 +42,10 @@ enum epoll_type { > > > EPOLL_TYPE_VHOST_KICK, > > > /* vhost-user migration socket */ > > > EPOLL_TYPE_VHOST_MIGRATION, > > > + /* TCP_REPAIR helper listening socket */ > > > + EPOLL_TYPE_REPAIR_LISTEN, > > > + /* TCP_REPAIR helper socket */ > > > + EPOLL_TYPE_REPAIR, > > > > > > EPOLL_NUM_TYPES, > > > }; > > > diff --git a/passt.1 b/passt.1 > > > index d9cd33e..63a3a01 100644 > > > --- a/passt.1 > > > +++ b/passt.1 > > > @@ -418,6 +418,17 @@ Enable vhost-user. The vhost-user command socket is provided by \fB--socket\fR. > > > .BR \-\-print-capabilities > > > Print back-end capabilities in JSON format, only meaningful for vhost-user mode. > > > > > > +.TP > > > +.BR \-\-repair-path " " \fIpath > > > +Path for UNIX domain socket used by the \fBpasst-repair\fR(1) helper to connect > > > > passt-repair(8)? > > See above. > > > > +to \fBpasst\fR in order to set or clear the TCP_REPAIR option on sockets, during > > > +migration. \fB--repair-path none\fR disables this interface (if you need to > > > +specify a socket path called "none" you can prefix the path by \fI./\fR). > > > + > > > +Default, for \-\-vhost-user mode only, is to append \fI.repair\fR to the path > > > +chosen for the hypervisor UNIX domain socket. No socket is created if not in > > > +\-\-vhost-user mode. > > > + > > > .TP > > > .BR \-F ", " \-\-fd " " \fIFD > > > Pass a pre-opened, connected socket to \fBpasst\fR. Usually the socket is opened > > > diff --git a/passt.c b/passt.c > > > index 184d4e5..1fa2ddd 100644 > > > --- a/passt.c > > > +++ b/passt.c > > > @@ -51,6 +51,7 @@ > > > #include "tcp_splice.h" > > > #include "ndp.h" > > > #include "vu_common.h" > > > +#include "repair.h" > > > > > > #define EPOLL_EVENTS 8 > > > > > > @@ -76,6 +77,8 @@ char *epoll_type_str[] = { > > > [EPOLL_TYPE_VHOST_CMD] = "vhost-user command socket", > > > [EPOLL_TYPE_VHOST_KICK] = "vhost-user kick socket", > > > [EPOLL_TYPE_VHOST_MIGRATION] = "vhost-user migration socket", > > > + [EPOLL_TYPE_REPAIR_LISTEN] = "TCP_REPAIR helper listening socket", > > > + [EPOLL_TYPE_REPAIR] = "TCP_REPAIR helper socket", > > > }; > > > static_assert(ARRAY_SIZE(epoll_type_str) == EPOLL_NUM_TYPES, > > > "epoll_type_str[] doesn't match enum epoll_type"); > > > @@ -360,6 +363,12 @@ loop: > > > case EPOLL_TYPE_VHOST_MIGRATION: > > > vu_migrate(&c, eventmask); > > > break; > > > + case EPOLL_TYPE_REPAIR_LISTEN: > > > + repair_listen_handler(&c, eventmask); > > > + break; > > > + case EPOLL_TYPE_REPAIR: > > > + repair_handler(&c, eventmask); > > > + break; > > > default: > > > /* Can't happen */ > > > ASSERT(0); > > > diff --git a/passt.h b/passt.h > > > index 0dd4efa..85b0a10 100644 > > > --- a/passt.h > > > +++ b/passt.h > > > @@ -20,6 +20,7 @@ union epoll_ref; > > > #include "siphash.h" > > > #include "ip.h" > > > #include "inany.h" > > > +#include "migrate.h" > > > #include "flow.h" > > > #include "icmp.h" > > > #include "fwd.h" > > > @@ -193,6 +194,7 @@ struct ip6_ctx { > > > * @foreground: Run in foreground, don't log to stderr by default > > > * @nofile: Maximum number of open files (ulimit -n) > > > * @sock_path: Path for UNIX domain socket > > > + * @repair_path: TCP_REPAIR helper path, can be "none", empty for default > > > * @pcap: Path for packet capture file > > > * @pidfile: Path to PID file, empty string if not configured > > > * @pidfile_fd: File descriptor for PID file, -1 if none > > > @@ -203,6 +205,8 @@ struct ip6_ctx { > > > * @epollfd: File descriptor for epoll instance > > > * @fd_tap_listen: File descriptor for listening AF_UNIX socket, if any > > > * @fd_tap: AF_UNIX socket, tuntap device, or pre-opened socket > > > + * @fd_repair_listen: File descriptor for listening TCP_REPAIR socket, if any > > > + * @fd_repair: Connected AF_UNIX socket for TCP_REPAIR helper > > > * @our_tap_mac: Pasta/passt's MAC on the tap link > > > * @guest_mac: MAC address of guest or namespace, seen or configured > > > * @hash_secret: 128-bit secret for siphash functions > > > @@ -244,6 +248,7 @@ struct ctx { > > > int foreground; > > > int nofile; > > > char sock_path[UNIX_PATH_MAX]; > > > + char repair_path[UNIX_PATH_MAX]; > > > char pcap[PATH_MAX]; > > > > > > char pidfile[PATH_MAX]; > > > @@ -260,6 +265,8 @@ struct ctx { > > > int epollfd; > > > int fd_tap_listen; > > > int fd_tap; > > > + int fd_repair_listen; > > > + int fd_repair; > > > unsigned char our_tap_mac[ETH_ALEN]; > > > unsigned char guest_mac[ETH_ALEN]; > > > uint64_t hash_secret[2]; > > > diff --git a/repair.c b/repair.c > > > new file mode 100644 > > > index 0000000..24966f5 > > > --- /dev/null > > > +++ b/repair.c > > > @@ -0,0 +1,192 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > + > > > +/* PASST - Plug A Simple Socket Transport > > > + * for qemu/UNIX domain socket mode > > > + * > > > + * PASTA - Pack A Subtle Tap Abstraction > > > + * for network namespace/tap device mode > > > + * > > > + * repair.c - Interface (server) for passt-repair, set/clear TCP_REPAIR > > > + * > > > + * Copyright (c) 2025 Red Hat GmbH > > > + * Author: Stefano Brivio > > > + */ > > > + > > > +#include > > > +#include > > > + > > > +#include "util.h" > > > +#include "ip.h" > > > +#include "passt.h" > > > +#include "inany.h" > > > +#include "flow.h" > > > +#include "flow_table.h" > > > + > > > +#include "repair.h" > > > + > > > +#define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */ > > > + > > > +static int fds[SCM_MAX_FD]; > > > > Even 'static', I'd prefer a longer name for a global variable. > > A longer name for this also means a longer name for nfds below, which > is not necessarily practical, but I'll check and try to change these to > repair_fds[] / repair_nfds if doable. > > > > +static int current_cmd; > > > > Is there any particular rationale behind these being globals, whereas > > fd_repair is in struct ctx? AFAICT they're basically equally global > > in practice. > > Yes: c->fd_repair_listen needs to be in struct ctx, and I want to keep > this consistent with c->fd_tap_listen / c->fd_tap. > > Besides, the variables declared here are really hacks representing > state for this compilation unit only. Oh, right. Essentially these (static) globals are less global than ctx, which is truly world global. Ok, that makes sense. > > > +static int nfds; > > > + > > > +/** > > > + * repair_sock_init() - Start listening for connections on helper socket > > > + * @c: Execution context > > > + */ > > > +void repair_sock_init(const struct ctx *c) > > > +{ > > > + union epoll_ref ref = { .type = EPOLL_TYPE_REPAIR_LISTEN }; > > > + struct epoll_event ev = { 0 }; > > > + > > > + listen(c->fd_repair_listen, 0); > > > + > > > + ref.fd = c->fd_repair_listen; > > > + ev.events = EPOLLIN | EPOLLHUP | EPOLLET; > > > + ev.data.u64 = ref.u64; > > > + epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_repair_listen, &ev); > > > +} > > > + > > > +/** > > > + * repair_listen_handler() - Handle events on TCP_REPAIR helper listening socket > > > + * @c: Execution context > > > + * @events: epoll events > > > + */ > > > +void repair_listen_handler(struct ctx *c, uint32_t events) > > > +{ > > > + union epoll_ref ref = { .type = EPOLL_TYPE_REPAIR }; > > > + struct epoll_event ev = { 0 }; > > > + struct ucred ucred; > > > + socklen_t len; > > > + > > > + if (events != EPOLLIN) { > > > + debug("Spurious event 0x%04x on TCP_REPAIR helper socket", > > > + events); > > > + return; > > > + } > > > + > > > + len = sizeof(ucred); > > > + > > > + /* Another client is already connected: accept and close right away. */ > > > > For the repair socket, last-connection-wins would make more sense to > > me than first-connection-wins. While hacking/debugging seems it might > > be useful to fix something in the passt-repair, re-run it and have it > > displace the stale version for existing passt instances. > > In the whole debugging I've done so far I'm actually using passt-repair > (otherwise I don't even start it), so it already terminates once it's > done. > > I think it's more secure to keep it like this, because it's more robust > against races. > > Let's say we have an issue in KubeVirt's virt-handler so that the > migration starts a bit before the helper is started, and somebody > manages to use this time window to connect another helper, this attempt > will actually go unnoticed. > > If passt-repair fails to connect, it will be very obvious. Ok, you convinced me. > > > + if (c->fd_repair != -1) { > > > + int discard = accept4(c->fd_repair_listen, NULL, NULL, > > > + SOCK_NONBLOCK); > > > + > > > + if (discard == -1) > > > + return; > > > + > > > + if (!getsockopt(discard, SOL_SOCKET, SO_PEERCRED, &ucred, &len)) > > > + info("Discarding TCP_REPAIR helper, PID %i", ucred.pid); > > > + > > > + close(discard); > > > + return; > > > + } > > > + > > > + c->fd_repair = accept4(c->fd_repair_listen, NULL, NULL, 0); > > > + > > > + if (!getsockopt(c->fd_repair, SOL_SOCKET, SO_PEERCRED, &ucred, &len)) > > > + info("Accepted TCP_REPAIR helper, PID %i", ucred.pid); > > > + > > > + ref.fd = c->fd_repair; > > > + ev.events = EPOLLHUP | EPOLLET; > > > + ev.data.u64 = ref.u64; > > > + epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_repair, &ev); > > > +} > > > + > > > +/** > > > + * repair_close() - Close connection to TCP_REPAIR helper > > > + * @c: Execution context > > > + */ > > > +void repair_close(struct ctx *c) > > > +{ > > > + debug("Closing TCP_REPAIR helper socket"); > > > + > > > + epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_repair, NULL); > > > + close(c->fd_repair); > > > + c->fd_repair = -1; > > > +} > > > + > > > +/** > > > + * repair_handler() - Handle EPOLLHUP and EPOLLERR on TCP_REPAIR helper socket > > > + * @c: Execution context > > > + * @events: epoll events > > > + */ > > > +void repair_handler(struct ctx *c, uint32_t events) > > > +{ > > > + (void)events; > > > + > > > + repair_close(c); > > > +} > > > + > > > +/** > > > + * repair_flush() - Flush current set of sockets to helper, with current command > > > + * @c: Execution context > > > + * > > > + * Return: 0 on success, negative error code on failure > > > + */ > > > +int repair_flush(struct ctx *c) > > > +{ > > > + struct iovec iov = { &((int8_t){ current_cmd }), sizeof(int8_t) }; > > > + char buf[CMSG_SPACE(sizeof(int) * SCM_MAX_FD)] > > > + __attribute__ ((aligned(__alignof__(struct cmsghdr)))); > > > + struct cmsghdr *cmsg; > > > + struct msghdr msg; > > > + int ret = 0; > > > + > > > + if (!nfds) > > > + return 0; > > > + > > > + msg = (struct msghdr){ NULL, 0, &iov, 1, > > > + buf, CMSG_SPACE(sizeof(int) * nfds), 0 }; > > > + cmsg = CMSG_FIRSTHDR(&msg); > > > + > > > + cmsg->cmsg_level = SOL_SOCKET; > > > + cmsg->cmsg_type = SCM_RIGHTS; > > > + cmsg->cmsg_len = CMSG_LEN(sizeof(int) * nfds); > > > + memcpy(CMSG_DATA(cmsg), fds, sizeof(int) * nfds); > > > + > > > + nfds = 0; > > > + > > > + if (sendmsg(c->fd_repair, &msg, 0) < 0) { > > > + ret = -errno; > > > > This error code won't be reported to the caller: you'll continue on to > > the recv() below, which will return EBADF, clobbering ret. > > Oops, right, I'll return early in this case (or equivalent). > > > > + err_perror("Failed to send sockets to TCP_REPAIR helper"); > > > + repair_close(c); > > > + } > > > + > > > + if (recv(c->fd_repair, &((int8_t){ 0 }), 1, 0) < 0) { > > > + ret = -errno; > > > + err_perror("Failed to receive reply from TCP_REPAIR helper"); > > > + repair_close(c); > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +/** > > > + * repair_flush() - Add socket to TCP_REPAIR set with given command > > > + * @c: Execution context > > > + * @s: Socket to add > > > + * @cmd: TCP_REPAIR_ON, TCP_REPAIR_OFF, or TCP_REPAIR_OFF_NO_WP > > > + * > > > + * Return: 0 on success, negative error code on failure > > > + */ > > > +/* cppcheck-suppress unusedFunction */ > > > +int repair_set(struct ctx *c, int s, int cmd) > > > +{ > > > + int rc; > > > + > > > + if (nfds && current_cmd != cmd) { > > > + if ((rc = repair_flush(c))) > > > + return rc; > > > + } > > > + > > > + current_cmd = cmd; > > > + fds[nfds++] = s; > > > + > > > + if (nfds >= SCM_MAX_FD) { > > > + if ((rc = repair_flush(c))) > > > + return rc; > > > + } > > > + > > > + return 0; > > > +} > > > diff --git a/repair.h b/repair.h > > > new file mode 100644 > > > index 0000000..693c515 > > > --- /dev/null > > > +++ b/repair.h > > > @@ -0,0 +1,16 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-or-later > > > + * Copyright (c) 2025 Red Hat GmbH > > > + * Author: Stefano Brivio > > > + */ > > > + > > > +#ifndef REPAIR_H > > > +#define REPAIR_H > > > + > > > +void repair_sock_init(const struct ctx *c); > > > +void repair_listen_handler(struct ctx *c, uint32_t events); > > > +void repair_handler(struct ctx *c, uint32_t events); > > > +void repair_close(struct ctx *c); > > > +int repair_flush(struct ctx *c); > > > +int repair_set(struct ctx *c, int s, int cmd); > > > + > > > +#endif /* REPAIR_H */ > > > diff --git a/tap.c b/tap.c > > > index cd32a90..0e60eb4 100644 > > > --- a/tap.c > > > +++ b/tap.c > > > @@ -56,6 +56,7 @@ > > > #include "netlink.h" > > > #include "pasta.h" > > > #include "packet.h" > > > +#include "repair.h" > > > #include "tap.h" > > > #include "log.h" > > > #include "vhost_user.h" > > > @@ -1151,68 +1152,6 @@ void tap_handler_pasta(struct ctx *c, uint32_t events, > > > tap_pasta_input(c, now); > > > } > > > > > > -/** > > > - * tap_sock_unix_open() - Create and bind AF_UNIX socket > > > - * @sock_path: Socket path. If empty, set on return (UNIX_SOCK_PATH as prefix) > > > - * > > > - * Return: socket descriptor on success, won't return on failure > > > - */ > > > -int tap_sock_unix_open(char *sock_path) > > > -{ > > > - int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); > > > - struct sockaddr_un addr = { > > > - .sun_family = AF_UNIX, > > > - }; > > > - int i; > > > - > > > - if (fd < 0) > > > - die_perror("Failed to open UNIX domain socket"); > > > - > > > - for (i = 1; i < UNIX_SOCK_MAX; i++) { > > > - char *path = addr.sun_path; > > > - int ex, ret; > > > - > > > - if (*sock_path) > > > - memcpy(path, sock_path, UNIX_PATH_MAX); > > > - else if (snprintf_check(path, UNIX_PATH_MAX - 1, > > > - UNIX_SOCK_PATH, i)) > > > - die_perror("Can't build UNIX domain socket path"); > > > - > > > - ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, > > > - 0); > > > - if (ex < 0) > > > - die_perror("Failed to check for UNIX domain conflicts"); > > > - > > > - ret = connect(ex, (const struct sockaddr *)&addr, sizeof(addr)); > > > - if (!ret || (errno != ENOENT && errno != ECONNREFUSED && > > > - errno != EACCES)) { > > > - if (*sock_path) > > > - die("Socket path %s already in use", path); > > > - > > > - close(ex); > > > - continue; > > > - } > > > - close(ex); > > > - > > > - unlink(path); > > > - ret = bind(fd, (const struct sockaddr *)&addr, sizeof(addr)); > > > - if (*sock_path && ret) > > > - die_perror("Failed to bind UNIX domain socket"); > > > - > > > - if (!ret) > > > - break; > > > - } > > > - > > > - if (i == UNIX_SOCK_MAX) > > > - die_perror("Failed to bind UNIX domain socket"); > > > - > > > - info("UNIX domain socket bound at %s", addr.sun_path); > > > - if (!*sock_path) > > > - memcpy(sock_path, addr.sun_path, UNIX_PATH_MAX); > > > - > > > - return fd; > > > -} > > > - > > > /** > > > * tap_backend_show_hints() - Give help information to start QEMU > > > * @c: Execution context > > > @@ -1423,6 +1362,8 @@ void tap_backend_init(struct ctx *c) > > > tap_sock_tun_init(c); > > > break; > > > case MODE_VU: > > > + repair_sock_init(c); > > > + /* fall through */ > > > case MODE_PASST: > > > tap_sock_unix_init(c); > > > > > > diff --git a/util.c b/util.c > > > index 36857d4..e98da74 100644 > > > --- a/util.c > > > +++ b/util.c > > > @@ -178,6 +178,68 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, > > > return fd; > > > } > > > > > > +/** > > > + * sock_unix() - Create and bind AF_UNIX socket > > > + * @sock_path: Socket path. If empty, set on return (UNIX_SOCK_PATH as prefix) > > > + * > > > + * Return: socket descriptor on success, won't return on failure > > > + */ > > > > I like making tap_sock_unix_open() more general. Could be split into > > a separate patch. > > I need it here though, and keeping it here saves lines in the commit > message... Oh very well. -- 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