- dev - passt

[PATCH 00/12] Minor fixups for or inspired by clangd and related tools
by David Gibson 07 Nov '24

07 Nov '24

I've been experimenting with Zed and clangd recently. Currently it generates an enormous number of largely spurious errors and warnings on the passt code base. Mostly that's due to its default configurations not suiting us. This series adds some configuration that addresses a number of those warnings, though there remain many more for now. Some of the warnings also look reasonable, so I have a grab bag of fixes or workarounds for some of those two. David Gibson (12): clang: Add .clang-format file Makefile: Simplify exclusion of qrap from static checks clang: Move clang-tidy configuration from Makefile to .clang-tidy arch: Avoid explicit access to 'environ' flow: Correct type of flowside_at_sidx() netlink: RTA_PAYLOAD() returns int, not size_t Makefile: Move NETNS_RUN_DIR definition to C code seccomp: Simplify handling of AUDIT_ARCH Makefile: Use -DARCH for qrap only Makefile: Don't attempt to auto-detect stack size clang: Add rudimentary clangd configuration util: Remove unused ffsl() function .clang-format | 126 ++++++++++++++++++++++++++++++++++++++++++++++ .clang-tidy | 93 ++++++++++++++++++++++++++++++++++ .clangd | 3 ++ Makefile | 136 +++----------------------------------------------- arch.c | 2 +- conf.c | 2 + flow_table.h | 2 +- netlink.c | 4 +- seccomp.sh | 14 +++++- util.h | 5 +- 10 files changed, 247 insertions(+), 140 deletions(-) create mode 100644 .clang-format create mode 100644 .clang-tidy create mode 100644 .clangd -- 2.47.0

2 16

[PATCH v3] tcp: unify payload and flags l2 frames array
by Jon Maloy 07 Nov '24

07 Nov '24

In order to reduce static memory and code footprint, we merge the array for l2 flag frames into the one for payload frames. This change also ensures that no flag message will be sent out over the l2 media bypassing already queued payload messages. Performance measurements with iperf3, where we force all traffic via the tap queue, show no significant difference: Dual traffic both directions sinmultaneously, with patch: ======================================================== host->ns: -------- [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 36.3 GBytes 3.12 Gbits/sec 4759 sender [ 5] 0.00-100.04 sec 36.3 GBytes 3.11 Gbits/sec receiver ns->host: --------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 321 GBytes 27.6 Gbits/sec receiver Dual traffic both directions sinmultaneously, without patch: ============================================================ host->ns: -------- [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 35.0 GBytes 3.01 Gbits/sec 6001 sender [ 5] 0.00-100.04 sec 34.8 GBytes 2.99 Gbits/sec receiver ns->host -------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 345 GBytes 29.6 Gbits/sec receiver Single connection, with patch: ============================== host->ns: --------- [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 138 GBytes 11.8 Gbits/sec 922 sender [ 5] 0.00-100.04 sec 138 GBytes 11.8 Gbits/sec receiver ns->host: ----------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 430 GBytes 36.9 Gbits/sec receiver Single connection, without patch: ================================= host->ns: ------------ [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 139 GBytes 11.9 Gbits/sec 900 sender [ 5] 0.00-100.04 sec 139 GBytes 11.9 Gbits/sec receiver ns->host: --------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 440 GBytes 37.8 Gbits/sec receiver Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> ---- v2: - Adapted to and rebased on latest release. - Removed redundant tcp_flags_push() function. - Added measurement results. v3: - Rebased on latest release. - Fixes based on comments from D. Gibson --- tcp.c | 1 - tcp_buf.c | 70 ++++++++++++-------------------------------------- tcp_buf.h | 1 - tcp_internal.h | 15 ----------- 4 files changed, 17 insertions(+), 70 deletions(-) diff --git a/tcp.c b/tcp.c index 4e0a17e..b652900 100644 --- a/tcp.c +++ b/tcp.c @@ -937,7 +937,6 @@ bool tcp_flow_defer(const struct tcp_tap_conn *conn) /* cppcheck-suppress [constParameterPointer, unmatchedSuppression] */ void tcp_defer_handler(struct ctx *c) { - tcp_flags_flush(c); tcp_payload_flush(c); } diff --git a/tcp_buf.c b/tcp_buf.c index 274e313..d29c1a9 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -20,7 +20,7 @@ #include <netinet/ip.h> -#include <linux/tcp.h> +#include <netinet/tcp.h> #include "util.h" #include "ip.h" @@ -59,22 +59,10 @@ static_assert(MSS6 <= sizeof(tcp_payload[0].data), "MSS6 is greater than 65516") static struct tcp_tap_conn *tcp_frame_conns[TCP_FRAMES_MEM]; static unsigned int tcp_payload_used; -static struct tap_hdr tcp_flags_tap_hdr[TCP_FRAMES_MEM]; -/* IPv4 headers for TCP segment without payload */ -static struct iphdr tcp4_flags_ip[TCP_FRAMES_MEM]; -/* TCP segments without payload for IPv4 frames */ -static struct tcp_flags_t tcp_flags[TCP_FRAMES_MEM]; - -static unsigned int tcp_flags_used; - -/* IPv6 headers for TCP segment without payload */ -static struct ipv6hdr tcp6_flags_ip[TCP_FRAMES_MEM]; - /* recvmsg()/sendmsg() data for tap */ static struct iovec iov_sock [TCP_FRAMES_MEM + 1]; static struct iovec tcp_l2_iov[TCP_FRAMES_MEM][TCP_NUM_IOVS]; -static struct iovec tcp_l2_flags_iov[TCP_FRAMES_MEM][TCP_NUM_IOVS]; /** * tcp_update_l2_buf() - Update Ethernet header buffers with addresses @@ -103,15 +91,6 @@ void tcp_sock_iov_init(const struct ctx *c) for (i = 0; i < ARRAY_SIZE(tcp_payload); i++) { tcp6_payload_ip[i] = ip6; tcp4_payload_ip[i] = iph; - tcp_payload[i].th.doff = sizeof(struct tcphdr) / 4; - tcp_payload[i].th.ack = 1; - } - - for (i = 0; i < ARRAY_SIZE(tcp_flags); i++) { - tcp6_flags_ip[i] = ip6; - tcp4_flags_ip[i] = iph; - tcp_flags[i].th.doff = sizeof(struct tcphdr) / 4; - tcp_flags[i].th.ack = 1; } for (i = 0; i < TCP_FRAMES_MEM; i++) { @@ -121,25 +100,6 @@ void tcp_sock_iov_init(const struct ctx *c) iov[TCP_IOV_ETH].iov_len = sizeof(struct ethhdr); iov[TCP_IOV_PAYLOAD].iov_base = &tcp_payload[i]; } - - for (i = 0; i < TCP_FRAMES_MEM; i++) { - struct iovec *iov = tcp_l2_flags_iov[i]; - - iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp_flags_tap_hdr[i]); - iov[TCP_IOV_ETH].iov_len = sizeof(struct ethhdr); - iov[TCP_IOV_PAYLOAD].iov_base = &tcp_flags[i]; - } -} - -/** - * tcp_flags_flush() - Send out buffers for segments with no data (flags) - * @c: Execution context - */ -void tcp_flags_flush(const struct ctx *c) -{ - tap_send_frames(c, &tcp_l2_flags_iov[0][0], TCP_NUM_IOVS, - tcp_flags_used); - tcp_flags_used = 0; } /** @@ -171,7 +131,7 @@ static void tcp_revert_seq(const struct ctx *c, struct tcp_tap_conn **conns, } /** - * tcp_payload_flush() - Send out buffers for segments with data + * tcp_payload_flush() - Send out buffers for segments with data or flags * @c: Execution context */ void tcp_payload_flush(const struct ctx *c) @@ -197,37 +157,35 @@ void tcp_payload_flush(const struct ctx *c) */ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) { - struct tcp_flags_t *payload; + struct tcp_payload_t *payload; struct iovec *iov; size_t optlen; size_t l4len; uint32_t seq; int ret; - iov = tcp_l2_flags_iov[tcp_flags_used]; + iov = tcp_l2_iov[tcp_payload_used]; if (CONN_V4(conn)) { - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_flags_ip[tcp_flags_used]); + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src; } else { - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_flags_ip[tcp_flags_used]); + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; } payload = iov[TCP_IOV_PAYLOAD].iov_base; seq = conn->seq_to_tap; ret = tcp_prepare_flags(c, conn, flags, &payload->th, - &payload->opts, &optlen); + (struct tcp_syn_opts *)&payload->data, &optlen); if (ret <= 0) return ret; - tcp_flags_used++; + tcp_payload_used++; l4len = tcp_l2_buf_fill_headers(conn, iov, optlen, NULL, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len; - if (flags & DUP_ACK) { - struct iovec *dup_iov; + struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++]; - dup_iov = tcp_l2_flags_iov[tcp_flags_used++]; memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_len); dup_iov[TCP_IOV_ETH].iov_base = iov[TCP_IOV_ETH].iov_base; @@ -237,8 +195,8 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) dup_iov[TCP_IOV_PAYLOAD].iov_len = l4len; } - if (tcp_flags_used > TCP_FRAMES_MEM - 2) - tcp_flags_flush(c); + if (tcp_payload_used > TCP_FRAMES_MEM - 2) + tcp_payload_flush(c); return 0; } @@ -254,6 +212,7 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, ssize_t dlen, int no_csum, uint32_t seq) { + struct tcp_payload_t *payload; const uint16_t *check = NULL; struct iovec *iov; size_t l4len; @@ -274,6 +233,11 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; } + payload = iov[TCP_IOV_PAYLOAD].iov_base; + payload->th.th_off = sizeof(struct tcphdr) / 4; + payload->th.th_x2 = 0; + payload->th.th_flags = 0; + payload->th.ack = 1; l4len = tcp_l2_buf_fill_headers(conn, iov, dlen, check, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len; if (++tcp_payload_used > TCP_FRAMES_MEM - 1) diff --git a/tcp_buf.h b/tcp_buf.h index 49c04d4..54f5e53 100644 --- a/tcp_buf.h +++ b/tcp_buf.h @@ -7,7 +7,6 @@ #define TCP_BUF_H void tcp_sock_iov_init(const struct ctx *c); -void tcp_flags_flush(const struct ctx *c); void tcp_payload_flush(const struct ctx *c); int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn); int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags); diff --git a/tcp_internal.h b/tcp_internal.h index a5a47df..c846f60 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -134,21 +134,6 @@ struct tcp_syn_opts { .ws = TCP_OPT_WS(ws_), \ }) -/** - * struct tcp_flags_t - TCP header and data to send zero-length - * segments (flags) - * @th: TCP header - * @opts TCP options - */ -struct tcp_flags_t { - struct tcphdr th; - struct tcp_syn_opts opts; -#ifdef __AVX2__ -} __attribute__ ((packed, aligned(32))); -#else -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); -#endif - extern char tcp_buf_discard [MAX_WINDOW]; void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn, -- 2.45.2

3 2

spliced performance
by Jon Maloy 06 Nov '24

06 Nov '24

I re-ran the splice ns->host test using splice, and it works flawlessly. It seems like Stefano's fix solved the problem. /jon [ 5] 90.00-91.00 sec 4.06 GBytes 34.9 Gbits/sec [ 5] 91.00-92.00 sec 4.05 GBytes 34.8 Gbits/sec [ 5] 92.00-93.00 sec 4.04 GBytes 34.7 Gbits/sec [ 5] 93.00-94.00 sec 3.93 GBytes 33.8 Gbits/sec [ 5] 94.00-95.00 sec 3.92 GBytes 33.6 Gbits/sec [ 5] 95.00-96.00 sec 3.95 GBytes 33.9 Gbits/sec [ 5] 96.00-97.00 sec 3.91 GBytes 33.6 Gbits/sec [ 5] 97.00-98.00 sec 3.92 GBytes 33.7 Gbits/sec [ 5] 98.00-99.00 sec 3.67 GBytes 31.5 Gbits/sec [ 5] 99.00-100.00 sec 3.64 GBytes 31.3 Gbits/sec [ 5] 100.00-100.00 sec 1.88 MBytes 35.0 Gbits/sec - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 389 GBytes 33.4 Gbits/sec receiver ----------------------------------------------------------- Server listening on 5201 (test #2) -----------------------------------------------------------

2 1

[PATCH] fwd: Squash different-signedness comparison warning
by David Gibson 06 Nov '24

06 Nov '24

On certain architectures we get a warning about comparison between different signedness integers in fwd_probe_ephemeral(). This is because NUM_PORTS evaluates to an unsigned integer. It's a fixed value, though and we know it will fit in a signed long on anything reasonable, so add a cast to suppress the warning. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- fwd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fwd.c b/fwd.c index c71f5e1..0b7f8b1 100644 --- a/fwd.c +++ b/fwd.c @@ -75,8 +75,8 @@ void fwd_probe_ephemeral(void) if (*end || errno) goto parse_err; - if (min < 0 || min >= NUM_PORTS || - max < 0 || max >= NUM_PORTS) + if (min < 0 || min >= (long)NUM_PORTS || + max < 0 || max >= (long)NUM_PORTS) goto parse_err; fwd_ephemeral_min = min; -- 2.47.0

1 0

[PATCH] test: Adjust misplaced sleeps in two_guests code
by David Gibson 06 Nov '24

06 Nov '24

Most of our transfer tests using socat use 'sleep' waaiting for the server side to be ready before starting the client. However in two_guests/basic the sleep is in the wrong place: rather than being between starting the server and starting the client, it's after waiting for the server to complete. This causes occasional hangs when the client runs before the server is ready - in that case the receiving guest sends an RST, which we don't (currently) propagate back to the sender. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- test/two_guests/basic | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/two_guests/basic b/test/two_guests/basic index 9ba5efe..e2338ff 100644 --- a/test/two_guests/basic +++ b/test/two_guests/basic @@ -52,33 +52,33 @@ check [ "__ADDR2_6__" = "__HOST_ADDR6__" ] test TCP/IPv4: guest 1 > guest 2 g1out GW1 ip -j -4 route show|jq -rM '.[] | select(.dst == "default").gateway' guest2b socat -u TCP4-LISTEN:10004 OPEN:msg,create,trunc +sleep 1 guest1 echo "Hello_from_guest_1" | socat -u STDIN TCP4:__GW1__:10004 guest2w -sleep 1 g2out MSG2 cat msg check [ "__MSG2__" = "Hello_from_guest_1" ] test TCP/IPv6: guest 2 > guest 1 g2out GW2_6 ip -j -6 route show|jq -rM '.[] | select(.dst == "default").gateway' guest1b socat -u TCP6-LISTEN:10001 OPEN:msg,create,trunc +sleep 1 guest2 echo "Hello_from_guest_2" | socat -u STDIN TCP6:[__GW2_6__%__IFNAME2__]:10001 guest1w -sleep 1 g1out MSG1 cat msg check [ "__MSG1__" = "Hello_from_guest_2" ] test UDP/IPv4: guest 1 > guest 2 guest2b socat -u TCP4-LISTEN:10004 OPEN:msg,create,trunc +sleep 1 guest1 echo "Hello_from_guest_1" | socat -u STDIN TCP4:__GW1__:10004 guest2w -sleep 1 g2out MSG2 cat msg check [ "__MSG2__" = "Hello_from_guest_1" ] test UDP/IPv6: guest 2 > guest 1 guest1b socat -u TCP6-LISTEN:10001 OPEN:msg,create,trunc +sleep 1 guest2 echo "Hello_from_guest_2" | socat -u STDIN TCP6:[__GW2_6__%__IFNAME2__]:10001 guest1w -sleep 1 g1out MSG1 cat msg check [ "__MSG1__" = "Hello_from_guest_2" ] -- 2.47.0

2 1

[PATCH v9 00/20] (RFC) Add vhost-user support to passt. (part 3)
by David Gibson 05 Nov '24

05 Nov '24

This is a rebased and revised version of Laurent's vhost-user patches. I had been working on some revisions to the IOV and buffer handling in the existing code with the idea that the vhost-user code could be simplified on top of it. However, it turned out to be clearer to implement these changes on top of vhost-user rather than the other way around. This is often hanging in the two guests tests, and I haven't had a chance to debug it yet. Laurent, For clarity, I only made the absolute minimum changes to your patch in order to get it to apply and compile. Other changes I've made as patches on top of that. Feel free to fold the simpler revisions into the main vhost-user patch if you want to. v9: [David Gibson] - Rebased on current main - Conflicts with v4/v6 buffer merge addressed - Conflicts with TCP options construction rework addressed - Added several cleanup patches on top - Added a number of IOV and buffer cleanups on top - The aim is that these should allow more sharing of logic between the vhost-user and non-vhost-user pathes, although they've only minimally accomplished that so far. v8: - remove iov_size() from vu_collect_one_frame() - move vu_packet_check_range() to vu_common.c - fix UDP when dlen is 0. v7: - rebase - use vu_collect_one_frame() to do vu_collect() (collect multiple frame) - add vhost-user tests from Stefano v6: - rebase - extract 3 patches from "vhost-user: add vhost-user": passt: rename tap_sock_init() to tap_backend_init() tcp: Export headers functions udp: Prepare udp.c to be shared with vhost-user - introduce new functions vu_collect_one_frame(), vu_collect(), vu_set_vnethdr(), vu_flush(), vu_send_single() to be called from tcp_vu.c, udp_vu.c and ICMP/DHCP where vhost-user code was duplicated. v5: - rebase on top of 2024_09_06.6b38f07 - rework udp_vu.c as ref.udp.v6 has been removed and we need to know if we receive IPv4 or IPv6 frame when we prepare the guest buffers for recvmsg() - remove vnet->hdrlen as the size is always the same with virtio-net v1 - address comments from David and Stefano v4: - rebase on top of 2024_08_21.1d6142f (rebasing on top of 620e19a1b48a ("udp: Merge udp[46]_mh_recv arrays") introduces a regression in the measure of the latency with UDP because I think I don't replace correctly ref.udp.v6 that is removed by this commit) - Addressed most of the comments from David and Stefano (I didn't want to postpone this version to next week, so I'll address the remaining comments in the next version). v3: - rebase on top of flow table - update tcp_vu.c to look like udp_vu.c (recv()/prepare()/send_frame()) - address comments from Stefano and David on version 2 v2: - remove PATCH 4 - rewrite PATCH 2 and 3 to follow passt coding style - move some code from PATCH 3 to PATCH 4 (previously PATCH 5) - partially addressed David's comment on PATCH 5 David Gibson (12): vhost-user: Fix some trivial errors in comments. isolation: Abort if mode is not set vhost-user: Consistent sense when encoding IP version as boolean test: Fix trivial errors in two_guests_vu tests tcp: Use only netinet/tcp.h instead of linux/tcp.h tcp_vu: Share more header construction between IPv4 and IPv6 paths tcp: Move tcp_l2_buf_fill_headers() to tcp_buf.c tcp: Adjust iov_len before filling headers tcp: Pass TCP header and payload separately to tcp_update_check_tcp[46]() tcp: Pass TCP header and payload separately to tcp_fill_headers[46]() tcp: Merge tcp_update_check_tcp[46]() tcp: Merge tcp_fill_headers[46]() with each other Laurent Vivier (7): packet: replace struct desc by struct iovec vhost-user: introduce virtio API vhost-user: introduce vhost-user API udp: Prepare udp.c to be shared with vhost-user tcp: Export headers functions passt: rename tap_sock_init() to tap_backend_init() vhost-user: add vhost-user Stefano Brivio (1): test: Add tests for passt in vhost-user mode Makefile | 9 +- conf.c | 21 +- epoll_type.h | 4 + iov.c | 1 - isolation.c | 17 +- packet.c | 91 ++-- packet.h | 22 +- passt.1 | 10 +- passt.c | 11 +- passt.h | 6 + pcap.c | 1 - tap.c | 129 ++++-- tap.h | 7 +- tcp.c | 283 ++++-------- tcp_buf.c | 40 +- tcp_internal.h | 14 +- tcp_vu.c | 465 ++++++++++++++++++++ tcp_vu.h | 12 + test/lib/perf_report | 15 + test/lib/setup | 77 +++- test/lib/setup_ugly | 2 +- test/passt_vu | 1 + test/passt_vu_in_ns | 1 + test/perf/passt_vu_tcp | 211 +++++++++ test/perf/passt_vu_udp | 159 +++++++ test/run | 25 ++ test/two_guests_vu | 1 + udp.c | 84 ++-- udp_internal.h | 34 ++ udp_vu.c | 336 ++++++++++++++ udp_vu.h | 13 + util.h | 8 + vhost_user.c | 977 +++++++++++++++++++++++++++++++++++++++++ vhost_user.h | 206 +++++++++ virtio.c | 660 ++++++++++++++++++++++++++++ virtio.h | 184 ++++++++ vu_common.c | 385 ++++++++++++++++ vu_common.h | 47 ++ 38 files changed, 4213 insertions(+), 356 deletions(-) create mode 100644 tcp_vu.c create mode 100644 tcp_vu.h create mode 120000 test/passt_vu create mode 120000 test/passt_vu_in_ns create mode 100644 test/perf/passt_vu_tcp create mode 100644 test/perf/passt_vu_udp create mode 120000 test/two_guests_vu create mode 100644 udp_internal.h create mode 100644 udp_vu.c create mode 100644 udp_vu.h create mode 100644 vhost_user.c create mode 100644 vhost_user.h create mode 100644 virtio.c create mode 100644 virtio.h create mode 100644 vu_common.c create mode 100644 vu_common.h -- 2.47.0

2 23

[PATCH v2] tcp: unify payload and flags l2 frames array
by Jon Maloy 04 Nov '24

04 Nov '24

In order to reduce static memory and code footprint, we merge the array for l2 flag frames into the one for payload frames. This change also ensures that no flag message will be sent out over the l2 media bypassing already queued payload messages. Performance measurements with iperf3, where we force all traffic via the tap queue, show no significant difference: Dual traffic both directions sinmultaneously, with patch: ======================================================== host->ns: -------- [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 36.3 GBytes 3.12 Gbits/sec 4759 sender [ 5] 0.00-100.04 sec 36.3 GBytes 3.11 Gbits/sec receiver ns->host: --------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 321 GBytes 27.6 Gbits/sec receiver Dual traffic both directions sinmultaneously, without patch: ============================================================ host->ns: -------- [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 35.0 GBytes 3.01 Gbits/sec 6001 sender [ 5] 0.00-100.04 sec 34.8 GBytes 2.99 Gbits/sec receiver ns->host -------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 345 GBytes 29.6 Gbits/sec receiver Single connection, with patch: ============================== host->ns: --------- [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 138 GBytes 11.8 Gbits/sec 922 sender [ 5] 0.00-100.04 sec 138 GBytes 11.8 Gbits/sec receiver ns->host: ----------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 430 GBytes 36.9 Gbits/sec receiver Single connection, without patch: ================================= host->ns: ------------ [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-100.00 sec 139 GBytes 11.9 Gbits/sec 900 sender [ 5] 0.00-100.04 sec 139 GBytes 11.9 Gbits/sec receiver ns->host: --------- [ ID] Interval Transfer Bitrate [ 5] 0.00-100.00 sec 440 GBytes 37.8 Gbits/sec receiver Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> ---- v2: - Adapted to and rebased on latest release. - Removed redundant tcp_flags_push() function. - Added measurement results. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tcp.c | 1 - tcp_buf.c | 66 ++++++++++++-------------------------------------- tcp_buf.h | 1 - tcp_internal.h | 15 ------------ 4 files changed, 16 insertions(+), 67 deletions(-) diff --git a/tcp.c b/tcp.c index 10ad06a..b17d5fe 100644 --- a/tcp.c +++ b/tcp.c @@ -936,7 +936,6 @@ bool tcp_flow_defer(const struct tcp_tap_conn *conn) /* cppcheck-suppress [constParameterPointer, unmatchedSuppression] */ void tcp_defer_handler(struct ctx *c) { - tcp_flags_flush(c); tcp_payload_flush(c); } diff --git a/tcp_buf.c b/tcp_buf.c index 274e313..58df4cc 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -33,6 +33,7 @@ #include "tcp_internal.h" #include "tcp_buf.h" +#define PAYLOAD_FLAGS htons(0x5010) /* doff = 5, ack = 1 */ #define TCP_FRAMES_MEM 128 #define TCP_FRAMES \ (c->mode == MODE_PASTA ? 1 : TCP_FRAMES_MEM) @@ -59,22 +60,10 @@ static_assert(MSS6 <= sizeof(tcp_payload[0].data), "MSS6 is greater than 65516") static struct tcp_tap_conn *tcp_frame_conns[TCP_FRAMES_MEM]; static unsigned int tcp_payload_used; -static struct tap_hdr tcp_flags_tap_hdr[TCP_FRAMES_MEM]; -/* IPv4 headers for TCP segment without payload */ -static struct iphdr tcp4_flags_ip[TCP_FRAMES_MEM]; -/* TCP segments without payload for IPv4 frames */ -static struct tcp_flags_t tcp_flags[TCP_FRAMES_MEM]; - -static unsigned int tcp_flags_used; - -/* IPv6 headers for TCP segment without payload */ -static struct ipv6hdr tcp6_flags_ip[TCP_FRAMES_MEM]; - /* recvmsg()/sendmsg() data for tap */ static struct iovec iov_sock [TCP_FRAMES_MEM + 1]; static struct iovec tcp_l2_iov[TCP_FRAMES_MEM][TCP_NUM_IOVS]; -static struct iovec tcp_l2_flags_iov[TCP_FRAMES_MEM][TCP_NUM_IOVS]; /** * tcp_update_l2_buf() - Update Ethernet header buffers with addresses @@ -107,13 +96,6 @@ void tcp_sock_iov_init(const struct ctx *c) tcp_payload[i].th.ack = 1; } - for (i = 0; i < ARRAY_SIZE(tcp_flags); i++) { - tcp6_flags_ip[i] = ip6; - tcp4_flags_ip[i] = iph; - tcp_flags[i].th.doff = sizeof(struct tcphdr) / 4; - tcp_flags[i].th.ack = 1; - } - for (i = 0; i < TCP_FRAMES_MEM; i++) { struct iovec *iov = tcp_l2_iov[i]; @@ -121,25 +103,6 @@ void tcp_sock_iov_init(const struct ctx *c) iov[TCP_IOV_ETH].iov_len = sizeof(struct ethhdr); iov[TCP_IOV_PAYLOAD].iov_base = &tcp_payload[i]; } - - for (i = 0; i < TCP_FRAMES_MEM; i++) { - struct iovec *iov = tcp_l2_flags_iov[i]; - - iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp_flags_tap_hdr[i]); - iov[TCP_IOV_ETH].iov_len = sizeof(struct ethhdr); - iov[TCP_IOV_PAYLOAD].iov_base = &tcp_flags[i]; - } -} - -/** - * tcp_flags_flush() - Send out buffers for segments with no data (flags) - * @c: Execution context - */ -void tcp_flags_flush(const struct ctx *c) -{ - tap_send_frames(c, &tcp_l2_flags_iov[0][0], TCP_NUM_IOVS, - tcp_flags_used); - tcp_flags_used = 0; } /** @@ -171,7 +134,7 @@ static void tcp_revert_seq(const struct ctx *c, struct tcp_tap_conn **conns, } /** - * tcp_payload_flush() - Send out buffers for segments with data + * tcp_payload_flush() - Send out buffers for segments with data or flags * @c: Execution context */ void tcp_payload_flush(const struct ctx *c) @@ -197,37 +160,35 @@ void tcp_payload_flush(const struct ctx *c) */ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) { - struct tcp_flags_t *payload; + struct tcp_payload_t *payload; struct iovec *iov; size_t optlen; size_t l4len; uint32_t seq; int ret; - iov = tcp_l2_flags_iov[tcp_flags_used]; + iov = tcp_l2_iov[tcp_payload_used]; if (CONN_V4(conn)) { - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_flags_ip[tcp_flags_used]); + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src; } else { - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_flags_ip[tcp_flags_used]); + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; } payload = iov[TCP_IOV_PAYLOAD].iov_base; seq = conn->seq_to_tap; ret = tcp_prepare_flags(c, conn, flags, &payload->th, - &payload->opts, &optlen); + (struct tcp_syn_opts *)&payload->data, &optlen); if (ret <= 0) return ret; - tcp_flags_used++; + tcp_payload_used++; l4len = tcp_l2_buf_fill_headers(conn, iov, optlen, NULL, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len; - if (flags & DUP_ACK) { - struct iovec *dup_iov; + struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++]; - dup_iov = tcp_l2_flags_iov[tcp_flags_used++]; memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_len); dup_iov[TCP_IOV_ETH].iov_base = iov[TCP_IOV_ETH].iov_base; @@ -237,8 +198,8 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) dup_iov[TCP_IOV_PAYLOAD].iov_len = l4len; } - if (tcp_flags_used > TCP_FRAMES_MEM - 2) - tcp_flags_flush(c); + if (tcp_payload_used > TCP_FRAMES_MEM - 2) + tcp_payload_flush(c); return 0; } @@ -254,8 +215,10 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, ssize_t dlen, int no_csum, uint32_t seq) { + struct tcp_payload_t *payload; const uint16_t *check = NULL; struct iovec *iov; + uint16_t *flags; size_t l4len; conn->seq_to_tap = seq + dlen; @@ -274,6 +237,9 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; } + payload = iov[TCP_IOV_PAYLOAD].iov_base; + flags = &payload->th.window - 1; + *(flags) = PAYLOAD_FLAGS; l4len = tcp_l2_buf_fill_headers(conn, iov, dlen, check, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len; if (++tcp_payload_used > TCP_FRAMES_MEM - 1) diff --git a/tcp_buf.h b/tcp_buf.h index 49c04d4..54f5e53 100644 --- a/tcp_buf.h +++ b/tcp_buf.h @@ -7,7 +7,6 @@ #define TCP_BUF_H void tcp_sock_iov_init(const struct ctx *c); -void tcp_flags_flush(const struct ctx *c); void tcp_payload_flush(const struct ctx *c); int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn); int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags); diff --git a/tcp_internal.h b/tcp_internal.h index a5a47df..c846f60 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -134,21 +134,6 @@ struct tcp_syn_opts { .ws = TCP_OPT_WS(ws_), \ }) -/** - * struct tcp_flags_t - TCP header and data to send zero-length - * segments (flags) - * @th: TCP header - * @opts TCP options - */ -struct tcp_flags_t { - struct tcphdr th; - struct tcp_syn_opts opts; -#ifdef __AVX2__ -} __attribute__ ((packed, aligned(32))); -#else -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); -#endif - extern char tcp_buf_discard [MAX_WINDOW]; void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn, -- 2.45.2

2 2

[PATCH v2] tcp: Fix build against musl, __sum16 comes from linux/types.h
by Stefano Brivio 02 Nov '24

02 Nov '24

Use a plain uint16_t instead and avoid including one extra header: the 'bitwise' attribute of __sum16 is just used by sparse(1). Reported-by: omni <omni+alpine(a)hack.org> Fixes: 3d484aa37090 ("tcp: Update TCP checksum using an iovec array") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Use uint16_t instead of including linux/types.h tcp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tcp.c b/tcp.c index 4e0a17e..56ceba6 100644 --- a/tcp.c +++ b/tcp.c @@ -766,7 +766,7 @@ static void tcp_update_check_tcp4(const struct iphdr *iph, struct in_addr saddr = { .s_addr = iph->saddr }; struct in_addr daddr = { .s_addr = iph->daddr }; size_t check_ofs; - __sum16 *check; + uint16_t *check; int check_idx; uint32_t sum; char *ptr; @@ -797,7 +797,7 @@ static void tcp_update_check_tcp4(const struct iphdr *iph, return; } - check = (__sum16 *)ptr; + check = (uint16_t *)ptr; *check = 0; *check = csum_iov(iov, iov_cnt, l4offset, sum); @@ -816,7 +816,7 @@ static void tcp_update_check_tcp6(const struct ipv6hdr *ip6h, { uint16_t l4len = ntohs(ip6h->payload_len); size_t check_ofs; - __sum16 *check; + uint16_t *check; int check_idx; uint32_t sum; char *ptr; @@ -848,7 +848,7 @@ static void tcp_update_check_tcp6(const struct ipv6hdr *ip6h, return; } - check = (__sum16 *)ptr; + check = (uint16_t *)ptr; *check = 0; *check = csum_iov(iov, iov_cnt, l4offset, sum); -- 2.43.0

2 1

[PATCH] tcp: Fix build against musl, __sum16 comes from linux/types.h
by Stefano Brivio 31 Oct '24

31 Oct '24

...so we need to include that header explicitly. For some reason, it works without it when building against glibc. Reported-by: omni <omni+alpine(a)hack.org> Fixes: 3d484aa37090 ("tcp: Update TCP checksum using an iovec array") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tcp.c b/tcp.c index 4e0a17e..429634a 100644 --- a/tcp.c +++ b/tcp.c @@ -287,6 +287,8 @@ #include <time.h> #include <arpa/inet.h> +#include <linux/types.h> /* For __sum16 */ + #include "checksum.h" #include "util.h" #include "iov.h" -- 2.43.0

2 2

[PATCH v3 0/9] Take care of clang-tidy warnings with LLVM >= 16
by Stefano Brivio 31 Oct '24

31 Oct '24

So I started hitting some clang-tidy warnings with LLVM 16, some looked bogus, so I upgraded to LLVM 19, and... I got even more. This series takes care of them in different ways. v3: - split 5/8 into 5/9 and 6/9: in the first, drop O_APPEND so that we can have a helper to open any output file we need, and in the second one, always use O_CLOEXEC for pcap file (and use the new helper, now that we can) v2: - make snprintf_check() return and set errno on failure, in 2/8 - add missing err_perror() calls on clock_gettime() failures in 6/8 - drop all explicit integer assignments in enum udp_iov_idx in 7/8 Stefano Brivio (9): Makefile: Exclude qrap.c from clang-tidy checks treewide: Comply with CERT C rule ERR33-C for snprintf() treewide: Silence cert-err33-c clang-tidy warnings for fprintf() Makefile: Disable readability-math-missing-parentheses clang-tidy check log: Don't use O_APPEND at all treewide: Suppress clang-tidy warning if we already use O_CLOEXEC or if we can't treewide: Address cert-err33-c clang-tidy warnings for clock and timer functions udp: Take care of cert-int09-c clang-tidy warning for enum udp_iov_idx util: Don't use errno after a successful call in __daemon() Makefile | 13 ++++++++--- arch.c | 6 ++++- conf.c | 62 +++++++++++++++++++++++++++---------------------- log.c | 15 ++++-------- passt.c | 9 ++++--- pasta.c | 11 ++++++--- pcap.c | 24 ++++++++++--------- tap.c | 5 ++-- tcp.c | 12 +++++++--- udp.c | 10 ++++---- util.c | 71 +++++++++++++++++++++++++++++++++++--------------------- util.h | 7 +++++- 12 files changed, 148 insertions(+), 97 deletions(-) -- 2.43.0

2 18