- dev - passt

[PATCH] log: Don't define logging function 4 times
by David Gibson 13 Oct '23

13 Oct '23

In log.c we use a macro to define logging functions for each of 4 priority levels. The only difference between these is the priority we pass to vsyslog() and similar functions. Because it's done as a macro, however, the entire functions code is included in the binary 4 times. Rearrange this to take the priority level as a parameter to a regular function, then just use macros to define trivial wrappers which pass the priority level. This saves about 600 bytes of text in the executable (x86, non-AVX2). Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- log.c | 65 +++++++++++++++++++++++++++-------------------------------- log.h | 12 +++++++---- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/log.c b/log.c index c57bc8d..fd33f64 100644 --- a/log.c +++ b/log.c @@ -47,36 +47,36 @@ int log_to_stdout; /* Print to stdout instead of stderr */ #define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) -#define logfn(name, level) \ -void name(const char *format, ...) { \ - FILE *out = log_to_stdout ? stdout : stderr; \ - struct timespec tp; \ - va_list args; \ - \ - if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { \ - clock_gettime(CLOCK_REALTIME, &tp); \ - fprintf(out, "%lli.%04lli: ", \ - (long long int)tp.tv_sec - log_start, \ - (long long int)tp.tv_nsec / (100L * 1000)); \ - } \ - \ - if ((LOG_MASK(LOG_PRI(level)) & log_mask) || BEFORE_DAEMON) { \ - va_start(args, format); \ - if (log_file != -1) \ - logfile_write(level, format, args); \ - else if (!(setlogmask(0) & LOG_MASK(LOG_DEBUG))) \ - passt_vsyslog(level, format, args); \ - va_end(args); \ - } \ - \ - if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || \ - (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { \ - va_start(args, format); \ - (void)vfprintf(out, format, args); \ - va_end(args); \ - if (format[strlen(format)] != '\n') \ - fprintf(out, "\n"); \ - } \ +void logmsg(int pri, const char *format, ...) +{ + FILE *out = log_to_stdout ? stdout : stderr; + struct timespec tp; + va_list args; + + if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { + clock_gettime(CLOCK_REALTIME, &tp); + fprintf(out, "%lli.%04lli: ", + (long long int)tp.tv_sec - log_start, + (long long int)tp.tv_nsec / (100L * 1000)); + } + + if ((LOG_MASK(LOG_PRI(pri)) & log_mask) || BEFORE_DAEMON) { + va_start(args, format); + if (log_file != -1) + logfile_write(pri, format, args); + else if (!(setlogmask(0) & LOG_MASK(LOG_DEBUG))) + passt_vsyslog(pri, format, args); + va_end(args); + } + + if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || + (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { + va_start(args, format); + (void)vfprintf(out, format, args); + va_end(args); + if (format[strlen(format)] != '\n') + fprintf(out, "\n"); + } } /* Prefixes for log file messages, indexed by priority */ @@ -89,11 +89,6 @@ const char *logfile_prefix[] = { " ", /* LOG_DEBUG */ }; -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 * @enable: Tracing debug mode enabled if non-zero diff --git a/log.h b/log.h index a17171a..b4fb7e9 100644 --- a/log.h +++ b/log.h @@ -6,14 +6,18 @@ #ifndef LOG_H #define LOG_H +#include <syslog.h> + #define LOGFILE_SIZE_DEFAULT (1024 * 1024UL) #define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */ #define LOGFILE_SIZE_MIN (5UL * MAX(BUFSIZ, PAGE_SIZE)) -void err(const char *format, ...); -void warn(const char *format, ...); -void info(const char *format, ...); -void debug(const char *format, ...); +void logmsg(int pri, const char *format, ...); + +#define err(...) logmsg(LOG_ERR, __VA_ARGS__) +#define warn(...) logmsg(LOG_WARNING, __VA_ARGS__) +#define info(...) logmsg(LOG_INFO, __VA_ARGS__) +#define debug(...) logmsg(LOG_DEBUG, __VA_ARGS__) #define die(...) \ do { \ -- 2.41.0

1 1

[PATCH] arp: only send ARP replies for --gateway address
by Nikolay Edigaryev 12 Oct '23

12 Oct '23

Problem: when passt/pasta are working in a broadcast domain with more than one host machine, it will answer for all of these machines, except for the one having --address. This is akin to ARP spoofing and breaks connection with these machines if passt/pasta ARP reply arrives before the original one. Solution: only be responsible and send ARP replies for the --gateway's address. --- arp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arp.c b/arp.c index a35c1b6..f873491 100644 --- a/arp.c +++ b/arp.c @@ -67,8 +67,8 @@ int arp(const struct ctx *c, const struct pool *p) !memcmp(am->sip, am->tip, sizeof(am->sip))) return 1; - /* Don't resolve our own address, either. */ - if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) + /* Don't resolve anything but gateway address. */ + if (memcmp(am->tip, &c->ip4.gw, sizeof(am->tip)) != 0) return 1; ah->ar_op = htons(ARPOP_REPLY); -- 2.39.2 (Apple Git-144)

3 5

passt: new version 2023_10_04.f851084 available
by Stefano Brivio 10 Oct '23

10 Oct '23

The new version with tag 2023_10_04.f851084 includes the following changes: f851084 dhcp: put option 53 at the beginning a469fc3 tcp, tap: Don't increase tap-side sequence counter for dropped frames d3192f6 tcp: Force TCP_WINDOW_CLAMP before resetting STALLED flag feaeb49 tcp: Fix comment to tcp_sock_consume() 117b474 cppcheck: Work around bug in cppcheck 2.12.0 8640d62 cppcheck: Use "exhaustive" level checking when available c0efa4e conf: Remove overly cryptic selection of forward table 6471c7d cppcheck: Make many pointers const fc8f0f8 siphash: Use incremental rather than all-at-once siphash functions 04b10a8 siphash, checksum: Move TBAA explanation to checksum.c ceae842 siphash: Make internal helpers public cbc84df siphash: Use specific structure for internal state fcec3f6 siphash: Use more hygienic state initialiser 5cc8435 siphash: Fix bug in state initialisation 831067f siphash: Clean up hash finalisation with posthash_final() function 7a3153c siphash: Add siphash_feed() helper f7b2be2 siphash: Make sip round calculations an inline function rather than macro ca6e947 siphash: Make siphash functions consistently return 64-bit results c1d2a07 util: Consolidate and improve workarounds for clang-tidy issue 58992 5b6c68c Avoid shadowing index(3) 9178a9e tcp: Always send an ACK segment once the handshake is completed c090692 dhcp: Actually note down the length of options received by the client 8b8537d dhcpv6: Properly separate domain names in search list https://passt.top/passt/log/?qt=range&q=2023_09_08.05627dc..2023_10_04.f851… Packages: - Arch Linux: https://www.archlinux.org/packages/extra/x86_64/passt/ https://archlinuxarm.org/packages/aarch64/passt https://archlinuxarm.org/packages/armv7h/passt - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/6497040/ permanent mirror: https://passt.top/builds/copr/0^20231004.gf851084/ - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Ubuntu tracker: https://launchpad.net/ubuntu/+source/passt - Void Linux: https://voidlinux.org/packages/?q=passt - Static builds: - Package for other RPM-based distributions, x86_64 only: https://passt.top/builds/latest/x86_64/passt-gf851084-1.x86_64.rpm - x86_64 static binaries: https://passt.top/builds/latest/x86_64/ - Debian package, from x86_64 static build: https://passt.top/builds/latest/x86_64/passt_0f851084-1_all.deb -- Stefano

1 0

[PATCH RFT 0/5] Fixes and a workaround for TCP stalls with small buffers
by Stefano Brivio 05 Oct '23

05 Oct '23

The fundamental patch here is 3/5, which is a workaround for a rather surprising kernel behaviour we seem to be hitting. This all comes from the investigation around https://bugs.passt.top/show_bug.cgi?id=74. I can't hit stalls anymore and throughput looks finally good to me (~3.5gbps with 208 KiB rmem_max and wmem_max), but... please test. Stefano Brivio (5): tcp: Fix comment to tcp_sock_consume() tcp: Reset STALLED flag on ACK only, check for pending socket data tcp: Force TCP_WINDOW_CLAMP before resetting STALLED flag tcp, tap: Don't increase tap-side sequence counter for dropped frames passt.1: Add note about tuning rmem_max and wmem_max for throughput passt.1 | 33 +++++++++++++++++++++++++ tap.c | 10 +++++--- tap.h | 2 +- tcp.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++---------- 4 files changed, 102 insertions(+), 17 deletions(-) -- 2.39.2

2 31

[PATCH] dhcp: put option 53 at the beginning
by Stas Sergeev 05 Oct '23

05 Oct '23

... unless it is listed in 55. Many clients expect option 53 at the beginning. mTCP has this code: if ( resp->options[0] != 53 ) { TRACE_WARN(( "Dhcp: first option was not a Dhcp msg type\n" )); return; } wattcp32 has this: static int DHCP_is_ack (void) { const BYTE *opt = (const BYTE*) &dhcp_in.dh_opt[4]; return (opt[0] == DHCP_OPT_MSG_TYPE && opt[1] == 1 && opt[2] == DHCP_ACK); } static int DHCP_is_nack (void) { const BYTE *opt = (const BYTE*) &dhcp_in.dh_opt[4]; return (opt[0] == DHCP_OPT_MSG_TYPE && opt[1] == 1 && opt[2] == DHCP_NAK); } Link: https://bugs.passt.top/show_bug.cgi?id=77 Signed-off-by: Stas Sergeev <stsp2(a)yandex.ru> --- dhcp.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dhcp.c b/dhcp.c index 46c258e..eed1b4a 100644 --- a/dhcp.c +++ b/dhcp.c @@ -149,6 +149,12 @@ static int fill(struct msg *m) for (o = 0; o < 255; o++) opts[o].sent = 0; + /* Some clients (wattcp32, mTCP, maybe some other) expect + * options 53 at the beginning of the list. + * Put it there explicitly, unless requested via option 55. + */ + if (!memchr(opts[55].c, 53, opts[55].clen)) + fill_one(m, 53, &offset); for (i = 0; i < opts[55].clen; i++) { o = opts[55].c[i]; if (opts[o].slen) -- 2.40.1

2 1

[PATCH 0/4] RFC: Updates for cppcheck-2.12 warnings
by David Gibson 05 Oct '23

05 Oct '23

cppcheck 2.12 (which Fedora 38 has updated, for one) introduces a number of new warnings. Unfortunately, at least one of these is a clear bug in cppcheck. This series fixes a number of the new warnings reported in passt (patches 1..3) and works around the remaining cppcheck bug (patch 4). I'm pretty confident that patches 1 & 2 are safe and beneficial to apply regardless of which cppcheck we're using. Patch 3 is a little more dubious, because it potentially increases the cppcheck runtime. On my system it doesn't seem to make a significant difference, but that might not always stay true. Patch 4 is a tricky one. It applies a specific suppression to work around the cppcheck bug. That's necessary to get a pass with the currently available cppcheck. However, it's ugly and we'd like to remove it once the bug is fixed, but have no obvious way to remind us to do that. What we want to do here kind of depends how long it takes the bug to be fixed, which isn't clear at the moment. David Gibson (4): cppcheck: Make many pointers const conf: Remove overly cryptic selection of forward table cppcheck: Use "exhaustive" level checking when available cppcheck: Work around bug in cppcheck 2.12.0 Makefile | 6 ++++++ conf.c | 25 +++++++++++-------------- isolation.c | 2 +- isolation.h | 2 +- log.c | 6 +++--- netlink.c | 6 +++--- netlink.h | 6 +++--- pasta.c | 4 ++-- pasta.h | 2 +- tap.c | 14 +++++++------- tap.h | 6 +++--- tcp.c | 25 ++++++++++++++++--------- tcp_conn.h | 2 +- tcp_splice.c | 5 +++-- tcp_splice.h | 3 ++- udp.c | 4 ++-- udp.h | 2 +- util.c | 5 +++-- util.h | 4 ++-- 19 files changed, 71 insertions(+), 58 deletions(-) -- 2.41.0

2 8

[PATCH v2 0/3] Fixes and a workaround for TCP stalls with small buffers
by Stefano Brivio 03 Oct '23

03 Oct '23

The fundamental patch here is 2/3, which is a workaround for a rather surprising kernel behaviour we seem to be hitting. This all comes from the investigation around https://bugs.passt.top/show_bug.cgi?id=74. I can't hit stalls anymore and throughput looks finally good to me (~3.5gbps with 208 KiB rmem_max and wmem_max), but... please test (again). v2: - Drop 1/5 (checking for ACK before resetting STALLED and calling tcp_data_from_sock() directly from tcp_tap_handler()) - Moving reset of STALLED flag is now done in 2/3 (was 3/5) - Don't pass unnecessary argument to tcp_data_to_tap() in 3/3 (was 4/5) - Drop 5/5 as long as we're not sure what kind of buffer clamping is actually beneficial Stefano Brivio (3): tcp: Fix comment to tcp_sock_consume() tcp: Force TCP_WINDOW_CLAMP before resetting STALLED flag tcp, tap: Don't increase tap-side sequence counter for dropped frames tap.c | 10 ++++++--- tap.h | 2 +- tcp.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 67 insertions(+), 16 deletions(-) -- 2.39.2

2 6

[PATCH v2 00/10] siphash: cleanups and fixes
by David Gibson 30 Sep '23

30 Sep '23

While working on unifying the hashing for the flow table, I noticed some awkwardness in the siphash functions. While looking into that I noticed some bugs. So.. here we are. Changes since v1: * Don't accidentally increase the alignment of union inany_addr David Gibson (10): siphash: Make siphash functions consistently return 64-bit results siphash: Make sip round calculations an inline function rather than macro siphash: Add siphash_feed() helper siphash: Clean up hash finalisation with posthash_final() function siphash: Fix bug in state initialisation siphash: Use more hygienic state initialiser siphash: Use specific structure for internal state siphash: Make internal helpers public siphash, checksum: Move TBAA explanation to checksum.c siphash: Use incremental rather than all-at-once siphash functions Makefile | 2 +- checksum.c | 19 ++-- inany.h | 17 +++- siphash.c | 243 --------------------------------------------------- siphash.h | 119 +++++++++++++++++++++++-- tcp.c | 37 +++----- tcp_splice.c | 1 + 7 files changed, 159 insertions(+), 279 deletions(-) delete mode 100644 siphash.c -- 2.41.0

2 11

[PATCH 00/10] siphash: cleanups and fixes
by David Gibson 29 Sep '23

29 Sep '23

While working on unifying the hashing for the flow table, I noticed some awkwardness in the siphash functions. While looking into that I noticed some bugs. So.. here we are. David Gibson (10): siphash: Make siphash functions consistently return 64-bit results siphash: Make sip round calculations an inline function rather than macro siphash: Add siphash_feed() helper siphash: Clean up hash finalisation with posthash_final() function siphash: Fix bug in state initialisation siphash: Use more hygienic state initialiser siphash: Use specific structure for internal state siphash: Make internal helpers public siphash, checksum: Move TBAA explanation to checksum.c siphash: Use incremental rather than all-at-once siphash functions Makefile | 2 +- checksum.c | 19 ++-- inany.h | 16 +++- siphash.c | 243 --------------------------------------------------- siphash.h | 119 +++++++++++++++++++++++-- tcp.c | 37 +++----- tcp_splice.c | 1 + 7 files changed, 158 insertions(+), 279 deletions(-) delete mode 100644 siphash.c -- 2.41.0

2 16

[PATCH v2 0/2] Some static checker fixes
by David Gibson 27 Sep '23

27 Sep '23

We already had a couple of places we were working around clang-tidy issue 58992, and the flow table series adds more. I got sick of ugly inlines every time we used a syscall which returns a socket address, so wrote a patch to consolidate the workarounds in one place. However, that patch added an include of <string.h> to util.h which exposed a classic C library gotcha in packet.c, so I fixed that too. Changes since v1: * Updated missed comment to match code changes in 1/2 * Fixed more places which shadowed index(3) David Gibson (2): Avoid shadowing index(3) util: Consolidate and improve workarounds for clang-tidy issue 58992 Makefile | 2 +- icmp.c | 5 ----- packet.c | 30 +++++++++++++++--------------- packet.h | 10 +++++----- tcp.c | 22 ++++++++-------------- tcp_splice.c | 2 +- util.c | 12 ++++++------ util.h | 43 ++++++++++++++++++++++++++++++++++++++++++- 8 files changed, 78 insertions(+), 48 deletions(-) -- 2.41.0

2 3