- dev - passt

[PATCH 0/3] Allow non-127.0.0.1 loopback address as host resolver
by Stefano Brivio 27 Feb '23

27 Feb '23

This series fixes an issue with DNS resolution reported by Andrea: if both guest and host use systemd-resolved, reachable at 127.0.0.53, we need to advertise a reachable address as DNS to the guest, which we already do, but also to map UDP packets for that address and port 53 to the original host resolver address. Otherwise, DNS resolution doesn't work at all in the guest, with default options. Stefano Brivio (3): udp: Actually use host resolver to forward DNS queries conf: Split add_dns{4,6}() out of get_dns() conf, udp: Allow any loopback address to be used as resolver conf.c | 92 ++++++++++++++++++++++++++++++++++++---------------------- udp.c | 20 ++++++------- 2 files changed, 67 insertions(+), 45 deletions(-) -- 2.39.1

3 7

[PATCH] log, conf, tap: Define die() as err() plus exit(), drop cppcheck workarounds
by Stefano Brivio 27 Feb '23

27 Feb '23

If we define die() as a variadic macro, passing __VA_ARGS__ to err(), and calling exit() outside err() itself, we can drop the workarounds introduced in commit 36f0199f6ef4 ("conf, tap: Silence two false positive invalidFunctionArg from cppcheck"). Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 3 --- log.c | 14 +++++--------- log.h | 7 ++++++- tap.c | 6 +----- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/conf.c b/conf.c index 37f25d6..0e512f4 100644 --- a/conf.c +++ b/conf.c @@ -1064,9 +1064,6 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid) if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) { die("Can't determine if we're in init namespace: %s", strerror(errno)); - - /* Silence cppcheck's invalidFunctionArg for 'fd' in read() */ - return; } if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) || diff --git a/log.c b/log.c index bfde6ce..d2f08fa 100644 --- a/log.c +++ b/log.c @@ -46,7 +46,7 @@ int log_trace; /* --trace mode enabled */ #define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) -#define logfn(name, level, doexit) \ +#define logfn(name, level) \ void name(const char *format, ...) { \ struct timespec tp; \ va_list args; \ @@ -75,9 +75,6 @@ void name(const char *format, ...) { \ if (format[strlen(format)] != '\n') \ fprintf(stderr, "\n"); \ } \ - \ - if (doexit) \ - exit(EXIT_FAILURE); \ } /* Prefixes for log file messages, indexed by priority */ @@ -90,11 +87,10 @@ const char *logfile_prefix[] = { " ", /* LOG_DEBUG */ }; -logfn(die, LOG_ERR, 1) -logfn(err, LOG_ERR, 0) -logfn(warn, LOG_WARNING, 0) -logfn(info, LOG_INFO, 0) -logfn(debug,LOG_DEBUG, 0) +logfn(err, LOG_ERR) +logfn(warn, LOG_WARNING) +logfn(info, LOG_INFO) +logfn(debug,LOG_DEBUG) /** * trace_init() - Set log_trace depending on trace (debug) mode diff --git a/log.h b/log.h index d4e9d85..d4ea141 100644 --- a/log.h +++ b/log.h @@ -10,12 +10,17 @@ #define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */ #define LOGFILE_SIZE_MIN (5UL * MAX(BUFSIZ, PAGE_SIZE)) -void die(const char *format, ...); void err(const char *format, ...); void warn(const char *format, ...); void info(const char *format, ...); void debug(const char *format, ...); +#define die(...) \ + do { \ + err(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + extern int log_trace; void trace_init(int enable); #define trace(...) \ diff --git a/tap.c b/tap.c index d6f962e..88eed88 100644 --- a/tap.c +++ b/tap.c @@ -1037,13 +1037,9 @@ static void tap_sock_unix_init(struct ctx *c) snprintf(path, UNIX_PATH_MAX - 1, UNIX_SOCK_PATH, i); ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); - if (ex < 0) { + if (ex < 0) die("UNIX domain socket check: %s", strerror(errno)); - /* Silence cppcheck's invalidFunctionArg for 'ex' */ - return; - } - ret = connect(ex, (const struct sockaddr *)&addr, sizeof(addr)); if (!ret || (errno != ENOENT && errno != ECONNREFUSED && errno != EACCES)) { -- 2.39.1

2 1

[PATCH] doc/demo: Fix and suppress ShellCheck warnings
by Stefano Brivio 27 Feb '23

27 Feb '23

ShellCheck reports (SC2034) that __qemu_arch is not used. Use it, and silence the resulting SC2086 warning as we want word splitting on options we pass with it. While at it, silence SC2317 warnings for commands in cleanup() that appear to be unreachable: cleanup() is only called as trap. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- doc/demo.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/demo.sh b/doc/demo.sh index ed71ffb..5b05ddf 100755 --- a/doc/demo.sh +++ b/doc/demo.sh @@ -110,6 +110,7 @@ next() { } # cleanup() - Terminate pasta and passt, clean up, restore TTY settings +# shellcheck disable=SC2317 cleanup() { [ -f "${DEMO_DIR}/pasta.pid" ] && kill "$(cat "${DEMO_DIR}/pasta.pid")" [ -f "${DEMO_DIR}/passt.pid" ] && kill "$(cat "${DEMO_DIR}/passt.pid")" @@ -223,7 +224,8 @@ into_ns() { echo "Use ^C to terminate it." next - cmd qrap 5 qemu-system-x86_64 -M pc,accel=kvm:tcg \ + # shellcheck disable=SC2086 + cmd qrap 5 ${__qemu_arch} \ -smp "$(nproc)" -m 1024 \ -nographic -serial stdio -nodefaults -no-reboot -vga none \ -initrd "${DEMO_DIR}/demo.img" \ -- 2.39.1

2 1

[PATCH] Fix definitions of SOCKET_MAX, TCP_MAX_CONNS
by Stefano Brivio 27 Feb '23

27 Feb '23

...and, given that I keep getting this wrong, add a convenience macro, MAX_FROM_BITS(). Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt.h | 4 ++-- tcp.h | 4 ++-- tcp_conn.h | 2 +- util.h | 2 ++ 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/passt.h b/passt.h index 3d7e567..e0383eb 100644 --- a/passt.h +++ b/passt.h @@ -42,7 +42,7 @@ union epoll_ref; /** * union epoll_ref - Breakdown of reference for epoll socket bookkeeping * @proto: IP protocol number - * @s: Socket number (implies 2^24 limit on number of descriptors) + * @s: Socket number (implies 2^24-1 limit on number of descriptors) * @tcp: TCP-specific reference part * @udp: UDP-specific reference part * @icmp: ICMP-specific reference part @@ -53,7 +53,7 @@ union epoll_ref { struct { int32_t proto:8, #define SOCKET_REF_BITS 24 -#define SOCKET_MAX (1 << SOCKET_REF_BITS) +#define SOCKET_MAX MAX_FROM_BITS(SOCKET_REF_BITS) s:SOCKET_REF_BITS; union { union tcp_epoll_ref tcp; diff --git a/tcp.h b/tcp.h index 5527c5b..36e5391 100644 --- a/tcp.h +++ b/tcp.h @@ -8,8 +8,8 @@ #define TCP_TIMER_INTERVAL 1000 /* ms */ -#define TCP_CONN_INDEX_BITS 17 /* 128k */ -#define TCP_MAX_CONNS (1 << TCP_CONN_INDEX_BITS) +#define TCP_CONN_INDEX_BITS 17 /* 128k - 1 */ +#define TCP_MAX_CONNS MAX_FROM_BITS(TCP_CONN_INDEX_BITS) struct ctx; diff --git a/tcp_conn.h b/tcp_conn.h index a499f34..c22632b 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -54,7 +54,7 @@ struct tcp_tap_conn { #define TCP_RETRANS_BITS 3 unsigned int retrans :TCP_RETRANS_BITS; -#define TCP_MAX_RETRANS ((1U << TCP_RETRANS_BITS) - 1) +#define TCP_MAX_RETRANS MAX_FROM_BITS(TCP_RETRANS_BITS) #define TCP_WS_BITS 4 /* RFC 7323 */ #define TCP_WS_MAX 14 diff --git a/util.h b/util.h index 6303c17..570094c 100644 --- a/util.h +++ b/util.h @@ -40,6 +40,8 @@ #define ROUND_DOWN(x, y) ((x) & ~((y) - 1)) #define ROUND_UP(x, y) (((x) + (y) - 1) & ~((y) - 1)) +#define MAX_FROM_BITS(n) ((int)((1U << (n)) - 1)) + #define BIT(n) (1UL << (n)) #define BITMAP_BIT(n) (BIT((n) % (sizeof(long) * 8))) #define BITMAP_WORD(n) (n / (sizeof(long) * 8)) -- 2.39.1

2 1

[PATCH 0/3] Avoid some warnings reported by Coverity
by Stefano Brivio 27 Feb '23

27 Feb '23

Stefano Brivio (3): tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls() tcp: Avoid false (but convoluted) positive Coverity CWE-476 warning tcp: Avoid (theoretical) resource leak (CWE-772) Coverity warning tcp.c | 17 ++++++++++++----- tcp_splice.c | 24 ++++++++++++++++-------- 2 files changed, 28 insertions(+), 13 deletions(-) -- 2.39.1

2 6

[PATCH] treewide: Disable gcc strict aliasing rules as needed, drop workarounds
by Stefano Brivio 27 Feb '23

27 Feb '23

Recently, commit 4ddbcb9c0c55 ("tcp: Disable optimisations for tcp_hash()") worked around yet another issue we hit with gcc 12 and '-flto -O2': some stores affecting the input data to siphash_20b() were omitted altogether, and tcp_hash() wouldn't get the correct hash for incoming connections. Digging further into this revealed that, at least according to gcc's interpretation of C99 aliasing rules, passing pointers to functions with different types compared to the effective type of the object (for example, a uint8_t pointer to an anonymous struct, as it happens in tcp_hash()), doesn't guarantee that stores are not reordered across the function call. This means that, in general, our checksum and hash functions might not see parts of input data that was intended to be provided by callers. Not even switching from uint8_t to character types, which should be appropriate here, according to C99 (ISO/IEC 9899, TC3, draft N1256), section 6.5, "Expressions", paragraph 7: An object shall have its stored value accessed only by an lvalue expression that has one of the following types: [...] — a character type. does the trick. I guess this is also subject to interpretation: casting passed pointers to character types, and then using those as different types, might still violate (dubious) aliasing rules. Disable gcc strict aliasing rules for potentially affected functions, which, in turn, disables gcc's Type-Based Alias Analysis (TBAA) optimisations based on those function arguments. Drop the existing workarounds. Also the (seemingly?) bogus 'maybe-uninitialized' warning on the tcp_tap_handler() > tcp_hash() > siphash_20b() path goes away with -fno-strict-aliasing on siphash_20b(). Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Makefile | 21 --------------------- checksum.c | 13 ++++++++++--- siphash.c | 22 +++++++++++++++++++--- tcp.c | 6 ------ 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/Makefile b/Makefile index 080c748..667ddfb 100644 --- a/Makefile +++ b/Makefile @@ -56,27 +56,6 @@ PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \ tcp_splice.h udp.h util.h HEADERS = $(PASST_HEADERS) seccomp.h -# On gcc 11 and 12, with -O2 and -flto, tcp_hash() and siphash_20b(), if -# inlined, seem to be hitting something similar to: -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993 -# from the pointer arithmetic used from the tcp_tap_handler() path to get the -# remote connection address. -# -# TODO: With the same combination, in ndp(), gcc optimises away the store of -# hop_limit in the IPv6 header (temporarily set to the protocol number for -# convenience, to mimic the ICMPv6 checksum pseudo-header) before the call to -# csum_unaligned(). Mark csum_unaligned() as "noipa" as a quick work-around, -# while we figure out if a corresponding gcc issue has already been reported. -ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion))) -ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS) $(CPPFLAGS))) -ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS) $(CPPFLAGS))) - FLAGS += -DTCP_HASH_NOINLINE - FLAGS += -DSIPHASH_20B_NOINLINE - FLAGS += -DCSUM_UNALIGNED_NO_IPA -endif -endif -endif - C := \#include <linux/tcp.h>\nstruct tcp_info x = { .tcpi_snd_wnd = 0 }; ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) FLAGS += -DHAS_SND_WND diff --git a/checksum.c b/checksum.c index 29769d9..9631f91 100644 --- a/checksum.c +++ b/checksum.c @@ -69,6 +69,8 @@ * * Return: 32-bit sum of 16-bit words */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ uint32_t sum_16b(const void *buf, size_t len) { const uint16_t *p = buf; @@ -107,9 +109,8 @@ uint16_t csum_fold(uint32_t sum) * * Return: 16-bit IPv4-style checksum */ -#if CSUM_UNALIGNED_NO_IPA -__attribute__((__noipa__)) /* See comment in Makefile */ -#endif +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init) { return (uint16_t)~csum_fold(sum_16b(buf, len) + init); @@ -245,6 +246,8 @@ void csum_icmp6(struct icmp6hdr *icmp6hr, * - sum_a/sum_b unpacking is interleaved and not sequential to reduce stalls * - coding style adaptation */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init) { __m256i a, b, sum256, sum_a_hi, sum_a_lo, sum_b_hi, sum_b_lo, c, d; @@ -391,6 +394,8 @@ less_than_128_bytes: * * Return: 16-bit folded, complemented checksum sum */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ uint16_t csum(const void *buf, size_t len, uint32_t init) { return (uint16_t)~csum_fold(csum_avx2(buf, len, init)); @@ -406,6 +411,8 @@ uint16_t csum(const void *buf, size_t len, uint32_t init) * * Return: 16-bit folded, complemented checksum */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ uint16_t csum(const void *buf, size_t len, uint32_t init) { return csum_unaligned(buf, len, init); diff --git a/siphash.c b/siphash.c index 811918b..e8b144d 100644 --- a/siphash.c +++ b/siphash.c @@ -104,6 +104,17 @@ * * Return: the 64-bit hash output */ +/* Type-Based Alias Analysis (TBAA) optimisation in gcc 11 and 12 (-flto -O2) + * makes these functions essentially useless by allowing reordering of stores of + * input data across function calls. Not even declaring @in as char pointer is + * enough: disable gcc's interpretation of strict aliasing altogether. See also: + * + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106706 + * https://stackoverflow.com/questions/2958633/gcc-strict-aliasing-and-horror-… + * https://lore.kernel.org/all/alpine.LFD.2.00.0901121128080.6528__33422.53280… + */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* cppcheck-suppress unusedFunction */ uint64_t siphash_8b(const uint8_t *in, const uint64_t *k) { @@ -123,6 +134,8 @@ uint64_t siphash_8b(const uint8_t *in, const uint64_t *k) * * Return: 32 bits obtained by XORing the two halves of the 64-bit hash output */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ /* cppcheck-suppress unusedFunction */ uint32_t siphash_12b(const uint8_t *in, const uint64_t *k) { @@ -148,9 +161,8 @@ uint32_t siphash_12b(const uint8_t *in, const uint64_t *k) * * Return: the 64-bit hash output */ -#if SIPHASH_20B_NOINLINE -__attribute__((__noinline__)) /* See comment in Makefile */ -#endif +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ uint64_t siphash_20b(const uint8_t *in, const uint64_t *k) { uint32_t *in32 = (uint32_t *)in; @@ -179,6 +191,8 @@ uint64_t siphash_20b(const uint8_t *in, const uint64_t *k) * * Return: the 64-bit hash output */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ /* cppcheck-suppress unusedFunction */ uint32_t siphash_32b(const uint8_t *in, const uint64_t *k) { @@ -205,6 +219,8 @@ uint32_t siphash_32b(const uint8_t *in, const uint64_t *k) * * Return: 32 bits obtained by XORing the two halves of the 64-bit hash output */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */ uint32_t siphash_36b(const uint8_t *in, const uint64_t *k) { uint32_t *in32 = (uint32_t *)in; diff --git a/tcp.c b/tcp.c index 21c319d..cbd537e 100644 --- a/tcp.c +++ b/tcp.c @@ -1182,12 +1182,6 @@ static int tcp_hash_match(const struct tcp_tap_conn *conn, * * Return: hash value, already modulo size of the hash table */ -#if TCP_HASH_NOINLINE -__attribute__((__noinline__)) /* See comment in Makefile */ -#endif -__attribute__((optimize("O0"))) /* TODO: with -O2 and -flto on gcc 12.2, - * siphash_20b() doesn't see 'addr', why? - */ static unsigned int tcp_hash(const struct ctx *c, const union inany_addr *addr, in_port_t tap_port, in_port_t sock_port) { -- 2.39.1

2 1

[PATCH] tcp: Suppress knownConditionTrueFalse cppcheck false positive
by Stefano Brivio 25 Feb '23

25 Feb '23

cppcheck 2.10 reports: tcp.c:1815:12: style: Condition 'wnd>prev_scaled' is always false [knownConditionTrueFalse] if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || ^ tcp.c:1808:8: note: Assignment 'wnd=((1<<(16+8))<(wnd))?(1<<(16+8)):(wnd)', assigned value is less than 1 wnd = MIN(MAX_WINDOW, wnd); ^ tcp.c:1811:19: note: Assuming condition is false if (prev_scaled == wnd) ^ tcp.c:1815:12: note: Condition 'wnd>prev_scaled' is always false if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || ^ but this is not actually the case: wnd is typically greater than 1, and might very well be greater than prev_scaled as well. I bisected this down to cppcheck commit b4d455df487c ("Fix 11349: FP negativeIndex for clamped array index (#4627)") and reported findings at https://github.com/danmar/cppcheck/pull/4627. Suppress the warning for the moment being. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tcp.c b/tcp.c index 803c2c4..21c319d 100644 --- a/tcp.c +++ b/tcp.c @@ -1812,6 +1812,15 @@ static void tcp_clamp_window(const struct ctx *c, struct tcp_tap_conn *conn, return; /* Discard +/- 1% updates to spare some syscalls. */ + /* TODO: cppcheck, starting from commit b4d455df487c ("Fix + * 11349: FP negativeIndex for clamped array index (#4627)"), + * reports wnd > prev_scaled as always being true, see also: + * + * https://github.com/danmar/cppcheck/pull/4627 + * + * drop this suppression once that's resolved. + */ + /* cppcheck-suppress knownConditionTrueFalse */ if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || (wnd < prev_scaled && wnd * 101 / 100 > prev_scaled)) return; -- 2.39.1

1 0

[PATCH] qrap: Pass PCI device numbers to qemu in base 10, not in base 16
by Stefano Brivio 25 Feb '23

25 Feb '23

Alona reports that when libvirt starts qrap (KubeVirt integration) and the domain description leads to more than 10 devices, indices of PCI device descriptors are formatted as hexadecimal, so we end up with things like "pci.a" instead of "pci.10". Reported-by: Alona Paz <alkaplan(a)redhat.com> Fixes: 5307faa05997 ("qrap: Strip network devices from command line, set them up according to machine") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- qrap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qrap.c b/qrap.c index 287198e..ff99c89 100644 --- a/qrap.c +++ b/qrap.c @@ -281,11 +281,11 @@ int main(int argc, char **argv) qemu_argv[qemu_argc++] = "-device"; if (!has_json) { snprintf(dev_str, ARG_MAX, - "%s,%s%x%s,netdev=hostnet0,x-txburst=4096", + "%s,%s%i%s,netdev=hostnet0,x-txburst=4096", dev->name, dev->template, i, dev->template_post); } else { snprintf(dev_str, ARG_MAX, - "{\"driver\":\"%s\",%s%x\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}", + "{\"driver\":\"%s\",%s%i\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}", dev->name, dev->template_json, i, dev->template_json_post); } qemu_argv[qemu_argc++] = dev_str; -- 2.39.1

3 11

[PATCH] log: Send identifier string in log messages, openlog() won't work for us
by Stefano Brivio 24 Feb '23

24 Feb '23

openlog() can be used to set "ident" and have all the log messages prefixed by it, but only if we call syslog() -- this is implemented by C libraries. We don't log messages with syslog(), though, as we have a custom implementation to ensure we don't need dynamic memory allocation. This means that it's perfectly useless to call openlog(), and that we have to prefix every message we log by the identifier on our own. Reported-by: Andrea Bolognani <abologna(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- log.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/log.c b/log.c index f23e8ef..bfde6ce 100644 --- a/log.c +++ b/log.c @@ -106,7 +106,7 @@ void trace_init(int enable) } /** - * __openlog() - Non-optional openlog() wrapper, to allow custom vsyslog() + * __openlog() - Non-optional openlog() implementation, for custom vsyslog() * @ident: openlog() identity (program name) * @option: openlog() options * @facility: openlog() facility (LOG_DAEMON) @@ -136,8 +136,6 @@ void __openlog(const char *ident, int option, int facility) log_mask |= facility; strncpy(log_ident, ident, sizeof(log_ident) - 1); log_opt = option; - - openlog(ident, option, facility); } /** @@ -158,11 +156,11 @@ void __setlogmask(int mask) */ void passt_vsyslog(int pri, const char *format, va_list ap) { + int prefix_len, n; char buf[BUFSIZ]; - int n; - /* Send without name and timestamp, the system logger should add them */ - n = snprintf(buf, BUFSIZ, "<%i> ", pri); + /* Send without timestamp, the system logger should add it */ + n = prefix_len = snprintf(buf, BUFSIZ, "<%i> %s: ", pri, log_ident); n += vsnprintf(buf + n, BUFSIZ - n, format, ap); @@ -170,7 +168,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap) n += snprintf(buf + n, BUFSIZ - n, "\n"); if (log_opt & LOG_PERROR) - fprintf(stderr, "%s", buf + sizeof("<0>")); + fprintf(stderr, "%s", buf + prefix_len); if (send(log_sock, buf, n, 0) != n) fprintf(stderr, "Failed to send %i bytes to syslog\n", n); -- 2.39.1

3 4

[libvirt PATCH 0/3] Support for restarting passt backend
by Laine Stump 22 Feb '23

22 Feb '23

If the passt process that implements the backend of a QEMU emulated network device terminates, QEMU itself is incapable of restarting a new process so that the networking can begin... working again. However, what QEMU can and does do is: 1) send the NETDEV_STREAM_DISCONNECTED event to libvirt when it sees that the socket has been closed (this event has been in QEMU for as long as support for "-netdev stream", which is used for connecting to passt) 2) as of QEMU 8.0.0, the qemu commandline for -netdev stream accepts the "reconnect" option, which tells QEMU to attempt reconnecting to the same socket it previously used, repeating the attempt every "n" seconds (the only argument to reconnect) until it is successful. If libvirt adds the reconnect option to the qemu commandline, and then responds to a NETDEV_STREAM_DISCONNECTED event by re-running the same passt command that it ran when the device was originally connected, then a guest will be able to recover in the (very unlikely, according to Stefano!) event that the original passt process unexpectedly exits, or is killed by some external entity. Patch 2/3 handles (2) above, while patch 3/3 handles (1). (patch 1/3 is a short guest appearance by pkrempa. Thanks pkrempa!). This resolves https://bugzilla.redhat.com/2172098 Along with Stefano's series fixing up selinux issues related to running the passt process, they make the passt backend very usable. Laine Stump (2): qemu: add reconnect=5 to passt qemu commandline options when available qemu: respond to NETDEV_STREAM_DISCONNECTED event Peter Krempa (1): qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 82 +++++++++++++++++++ src/qemu/qemu_monitor.c | 11 +++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 16 ++++ src/qemu/qemu_passt.c | 11 +++ src/qemu/qemu_process.c | 18 ++++ .../caps_8.0.0.x86_64.xml | 1 + .../net-user-passt.x86_64-7.2.0.args | 37 +++++++++ .../net-user-passt.x86_64-latest.args | 2 +- tests/qemuxml2argvtest.c | 1 + 14 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args -- 2.39.2

2 6