- dev - passt

[PATCH] tcp_splice: fcntl(2) returns the size of the pipe, if F_SETPIPE_SZ succeeds
by Stefano Brivio 25 Oct '24

25 Oct '24

Don't report bogus failures (with --trace) just because the return value is not zero. Link: https://github.com/containers/podman/issues/24219 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp_splice.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tcp_splice.c b/tcp_splice.c index f112cfe..93f8bce 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -320,7 +320,7 @@ static int tcp_splice_connect_finish(const struct ctx *c, } if (fcntl(conn->pipe[sidei][0], F_SETPIPE_SZ, - c->tcp.pipe_size)) { + c->tcp.pipe_size) != (int)c->tcp.pipe_size) { flow_trace(conn, "cannot set %d->%d pipe size to %zu", sidei, !sidei, c->tcp.pipe_size); @@ -672,7 +672,7 @@ static void tcp_splice_pipe_refill(const struct ctx *c) continue; if (fcntl(splice_pipe_pool[i][0], F_SETPIPE_SZ, - c->tcp.pipe_size)) { + c->tcp.pipe_size) != (int)c->tcp.pipe_size) { trace("TCP (spliced): cannot set pool pipe size to %zu", c->tcp.pipe_size); } -- 2.43.0

2 2

[PATCH] tcp_splice: splice() all we have to the writing side, not what we just read
by Stefano Brivio 25 Oct '24

25 Oct '24

In tcp_splice_sock_handler(), we try to calculate how much we can move from the pipe to the writing socket: if we just read some bytes, we'll use that amount, but if we haven't, we just try to empty the pipe. However, if we just read something, that doesn't mean that that's all the data we have on the pipe, as it's obvious from this sequence, where: pasta: epoll event on connected spliced TCP socket 54 (events: 0x00000001) Flow 0 (TCP connection (spliced)): 98304 from read-side call Flow 0 (TCP connection (spliced)): 33615 from write-side call (passed 98304) Flow 0 (TCP connection (spliced)): -1 from read-side call Flow 0 (TCP connection (spliced)): -1 from write-side call (passed 524288) Flow 0 (TCP connection (spliced)): event at tcp_splice_sock_handler:580 Flow 0 (TCP connection (spliced)): OUT_WAIT_0 we first pile up 98304 - 33615 = 64689 pending bytes, that we read but couldn't write, as the receiver buffer is full, and we set the corresponding OUT_WAIT flag. Then: pasta: epoll event on connected spliced TCP socket 54 (events: 0x00000001) Flow 0 (TCP connection (spliced)): 32768 from read-side call Flow 0 (TCP connection (spliced)): -1 from write-side call (passed 32768) Flow 0 (TCP connection (spliced)): event at tcp_splice_sock_handler:580 we splice() 32768 more bytes from our receiving side to the pipe. At some point: pasta: epoll event on connected spliced TCP socket 49 (events: 0x00000004) Flow 0 (TCP connection (spliced)): event at tcp_splice_sock_handler:489 Flow 0 (TCP connection (spliced)): ~OUT_WAIT_0 Flow 0 (TCP connection (spliced)): 1320 from read-side call Flow 0 (TCP connection (spliced)): 1320 from write-side call (passed 1320) the receiver is signalling to us that it's ready for more data (EPOLLOUT). We reset the OUT_WAIT flag, read 1320 more bytes from our receiving socket into the pipe, and that's what we write to the receiver, forgetting about the pending 97457 bytes we had, which the receiver might never get (not the same 97547 bytes: we'll actually send 1320 of those). This condition is rather hard to reproduce, and it was observed with Podman pulling container images via HTTPS. In the traces above, the client is side 0 (the initiating peer), and the server is sending the whole data. Instead of splicing from pipe to socket the amount of data we just read, we need to splice all the pending data we piled up until that point. We could do that using 'read' and 'written' counters, but there's actually no need, as the kernel also keeps track of how much data is available on the pipe. So, to make this simple and more robust, just give the whole pipe size as length to splice(). The kernel knows what to do with it. Later in the function, we used 'to_write' for an optimisation meant to reduce wakeups which retries right away to splice() in both directions if we couldn't write to the receiver the whole amount of pending data. Calculate a 'pending' value instead, only if we reach that point. Now that we check for the actual amount of pending data in that optimisation, we need to make sure we don't compare a zero or negative 'written' value: if we met that, it means that the receiver signalled end-of-file, an error, or to try again later. In those three cases, the optimisation doesn't make any sense, so skip it. Reported-by: Ed Santiago <santiago(a)redhat.com> Reported-by: Paul Holzinger <pholzing(a)redhat.com> Analysed-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/issues/24219 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp_splice.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tcp_splice.c b/tcp_splice.c index 9f5cc27..f112cfe 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -503,7 +503,7 @@ swap: lowat_act_flag = RCVLOWAT_ACT(fromsidei); while (1) { - ssize_t readlen, to_write = 0, written; + ssize_t readlen, written, pending; int more = 0; retry: @@ -518,14 +518,11 @@ retry: if (errno != EAGAIN) goto close; - - to_write = c->tcp.pipe_size; } else if (!readlen) { eof = 1; - to_write = c->tcp.pipe_size; } else { never_read = 0; - to_write += readlen; + if (readlen >= (long)c->tcp.pipe_size * 90 / 100) more = SPLICE_F_MORE; @@ -535,10 +532,10 @@ retry: eintr: written = splice(conn->pipe[fromsidei][0], NULL, - conn->s[!fromsidei], NULL, to_write, + conn->s[!fromsidei], NULL, c->tcp.pipe_size, SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK); flow_trace(conn, "%zi from write-side call (passed %zi)", - written, to_write); + written, c->tcp.pipe_size); /* Most common case: skip updating counters. */ if (readlen > 0 && readlen == written) { @@ -584,10 +581,9 @@ eintr: if (never_read && written == (long)(c->tcp.pipe_size)) goto retry; - if (!never_read && written < to_write) { - to_write -= written; + pending = conn->read[fromsidei] - conn->written[fromsidei]; + if (!never_read && written > 0 && written < pending) goto retry; - } if (eof) break; -- 2.43.0

2 1

[PATCH 0/2] tcp: Runtime checks for availability of TCP_INFO fields
by David Gibson 24 Oct '24

24 Oct '24

We have a number of Makefile tests for the presence of various fields in tcp_info. However that only tells us if they're present in the kernel headers against which we're compiling, not whether they're available in the kernel we're running on. For anything outside the earliest forms of TCP_INFO (as described in netinet/tcp.h and shared with BSD), we should verify at runtime if the kernel is returning that information. This was supposed to be leading into improved handling of lazily calling TCP_INFO, but I hit additional complications there, so I've postponed it. Might as well get these bits merged, though. David Gibson (2): tcp: Generalise probing for tcpi_snd_wnd field tcp: Use runtime tests for TCP_INFO fields tcp.c | 97 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 40 deletions(-) -- 2.47.0

2 4

[PATCH v4 0/2] tcp: unify IPv4 and IPv6 tap queues
by Jon Maloy 21 Oct '24

21 Oct '24

This should save us some memory and code. --- v2: - Setting pointers to pre-set IP and MAC headers on the fly instead of copying them. - Merged patch #2 and #3 from v1 v3: - Changes based on feedback from team v4: - Rebased to latest version of master branch Jon Maloy (2): tcp: set ip and eth headers in l2 tap queues on the fly tcp: unify l2 TCPv4 and TCPv6 queues and structures tcp.c | 6 +- tcp_buf.c | 272 ++++++++++++++++++------------------------------------ tcp_buf.h | 3 +- 3 files changed, 91 insertions(+), 190 deletions(-) -- 2.45.2

3 5

[PATCH] tcp: Use structures to construct initial TCP options
by David Gibson 21 Oct '24

21 Oct '24

As a rule, we prefer constructing packets with matching C structures, rather than building them byte by byte. However, one case we still build byte by byte is the TCP options we include in SYN packets (in fact the only time we generate TCP options on the tap interface). Rework this to use a structure and initialisers which make it a bit clearer what's going on. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 17 +++---------- tcp_buf.c | 2 +- tcp_internal.h | 68 ++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 67 insertions(+), 20 deletions(-) diff --git a/tcp.c b/tcp.c index b2155ab..0d22e07 100644 --- a/tcp.c +++ b/tcp.c @@ -1232,7 +1232,7 @@ static void tcp_update_seqack_from_tap(const struct ctx *c, * 1 otherwise */ int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, - int flags, struct tcphdr *th, char *data, + int flags, struct tcphdr *th, struct tcp_syn_opts *opts, size_t *optlen) { struct tcp_info tinfo = { 0 }; @@ -1258,12 +1258,6 @@ int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, if (flags & SYN) { int mss; - /* Options: MSS, NOP and window scale (8 bytes) */ - *optlen = OPT_MSS_LEN + 1 + OPT_WS_LEN; - - *data++ = OPT_MSS; - *data++ = OPT_MSS_LEN; - if (c->mtu == -1) { mss = tinfo.tcpi_snd_mss; } else { @@ -1279,16 +1273,11 @@ int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, else if (mss > PAGE_SIZE) mss = ROUND_DOWN(mss, PAGE_SIZE); } - *(uint16_t *)data = htons(MIN(USHRT_MAX, mss)); - - data += OPT_MSS_LEN - 2; conn->ws_to_tap = MIN(MAX_WS, tinfo.tcpi_snd_wscale); - *data++ = OPT_NOP; - *data++ = OPT_WS; - *data++ = OPT_WS_LEN; - *data++ = conn->ws_to_tap; + *opts = TCP_SYN_OPTS(mss, conn->ws_to_tap); + *optlen = sizeof(*opts); } else if (!(flags & RST)) { flags |= ACK; } diff --git a/tcp_buf.c b/tcp_buf.c index 238827b..44df0e4 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -282,7 +282,7 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) seq = conn->seq_to_tap; ret = tcp_prepare_flags(c, conn, flags, &payload->th, - payload->opts, &optlen); + &payload->opts, &optlen); if (ret <= 0) { if (CONN_V4(conn)) tcp4_flags_used--; diff --git a/tcp_internal.h b/tcp_internal.h index 2f74ffe..1ab8ce2 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -33,9 +33,7 @@ #define OPT_EOL 0 #define OPT_NOP 1 #define OPT_MSS 2 -#define OPT_MSS_LEN 4 #define OPT_WS 3 -#define OPT_WS_LEN 3 #define OPT_SACKP 4 #define OPT_SACK 5 #define OPT_TS 8 @@ -77,6 +75,65 @@ struct tcp_payload_t { } __attribute__ ((packed, aligned(__alignof__(unsigned int)))); #endif +/** struct tcp_opt_nop - TCP NOP option + * @kind: Option kind (OPT_NOP = 1) + */ +struct tcp_opt_nop { + uint8_t kind; +} __attribute__ ((packed)); +#define TCP_OPT_NOP ((struct tcp_opt_nop){ .kind = OPT_NOP, }) + +/** struct tcp_opt_mss - TCP MSS option + * @kind: Option kind (OPT_MSS == 2) + * @len: Option length (4) + * @mss: Maximum Segment Size + */ +struct tcp_opt_mss { + uint8_t kind; + uint8_t len; + uint16_t mss; +} __attribute__ ((packed)); +#define TCP_OPT_MSS(mss_) \ + ((struct tcp_opt_mss) { \ + .kind = OPT_MSS, \ + .len = sizeof(struct tcp_opt_mss), \ + .mss = htons(mss_), \ + }) + +/** struct tcp_opt_ws - TCP Window Scaling option + * @kind: Option kind (OPT_WS == 3) + * @len: Option length (3) + * @shift: Window scaling shift + */ +struct tcp_opt_ws { + uint8_t kind; + uint8_t len; + uint8_t shift; +} __attribute__ ((packed)); +#define TCP_OPT_WS(shift_) \ + ((struct tcp_opt_ws) { \ + .kind = OPT_WS, \ + .len = sizeof(struct tcp_opt_ws), \ + .shift = (shift_), \ + }) + +/** struct tcp_syn_opts - TCP options we apply to SYN packets + * @mss: Maximum Segment Size (MSS) option + * @nop: NOP opt (for alignment) + * @ws: Window Scaling (WS) option + */ +struct tcp_syn_opts { + struct tcp_opt_mss mss; + struct tcp_opt_nop nop; + struct tcp_opt_ws ws; +} __attribute__ ((packed)); +#define TCP_SYN_OPTS(mss_, ws_) \ + ((struct tcp_syn_opts){ \ + .mss = TCP_OPT_MSS(mss_), \ + .nop = TCP_OPT_NOP, \ + .ws = TCP_OPT_WS(ws_), \ + }) + /** * struct tcp_flags_t - TCP header and data to send zero-length * segments (flags) @@ -85,7 +142,7 @@ struct tcp_payload_t { */ struct tcp_flags_t { struct tcphdr th; - char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; + struct tcp_syn_opts opts; #ifdef __AVX2__ } __attribute__ ((packed, aligned(32))); #else @@ -124,7 +181,8 @@ size_t tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn, bool no_tcp_csum); int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, bool force_seq, struct tcp_info *tinfo); -int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, int flags, - struct tcphdr *th, char *data, size_t *optlen); +int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, + int flags, struct tcphdr *th, struct tcp_syn_opts *opts, + size_t *optlen); #endif /* TCP_INTERNAL_H */ -- 2.47.0

2 1

[PATCH v3 0/4] Don't expose container loopback services to the host
by David Gibson 21 Oct '24

21 Oct '24

podman issue #24045 pointed out that pasta's spliced forwarding logic can expose services within the namespace bound only to 127.0.0.1 or ::1 to the host. However, the namespace probably expects those to only be accessible to itself, so that's probably not what we want. This changes our forwarding logic so as not to do this. Note that the podman tests will currently fail with this series, I've submitted podman PR https://github.com/containers/podman/pull/24064 to fix that. Link: https://github.com/containers/podman/issues/24045 Changes since v2: * Add new field do structure comment Changes since v1: * Add --host-lo-to-ns-lo option to preserve the old behaviour * Clarify the new behaviour in the man page * Add some extra patches making some other corrections to the man page David Gibson (4): passt.1: Mark --stderr as deprecated more prominently passt.1: Clarify and update "Handling of local addresses" section test: Clarify test for spliced inbound transfers fwd: Direct inbound spliced forwards to the guest's external address conf.c | 9 ++++++ fwd.c | 31 +++++++++++++----- passt.1 | 75 +++++++++++++++++++++++++++----------------- passt.h | 2 ++ test/passt_in_ns/tcp | 8 ++--- test/passt_in_ns/udp | 4 +-- test/pasta/tcp | 16 +++++----- test/pasta/udp | 8 ++--- 8 files changed, 98 insertions(+), 55 deletions(-) -- 2.46.2

2 15

[PATCH v5 0/7] Don't expose container loopback services to the host
by David Gibson 18 Oct '24

18 Oct '24

podman issue #24045 pointed out that pasta's spliced forwarding logic can expose services within the namespace bound only to 127.0.0.1 or ::1 to the host. However, the namespace probably expects those to only be accessible to itself, so that's probably not what we want. This changes our forwarding logic so as not to do this. Note that the podman tests will currently fail with this series, I've submitted podman PR https://github.com/containers/podman/pull/24064 to fix that. Link: https://github.com/containers/podman/issues/24045 Changes since v4: * Handled a few cases where we need to wait for DAD which were missed in v4. * Fix a grammar error in comment Changes since v3: * Added several extra patches working around failures on Debian, because its dhclient-script doesn't wait for DAD to complete Changes since v2: * Add new field do structure comment Changes since v1: * Add --host-lo-to-ns-lo option to preserve the old behaviour * Clarify the new behaviour in the man page * Add some extra patches making some other corrections to the man page David Gibson (7): arp: Fix a handful of small warts test: Explicitly wait for DAD to complete on SLAAC addresses test: Wait for DAD on DHCPv6 addresses passt.1: Mark --stderr as deprecated more prominently passt.1: Clarify and update "Handling of local addresses" section test: Clarify test for spliced inbound transfers fwd: Direct inbound spliced forwards to the guest's external address arp.c | 8 ++--- conf.c | 9 ++++++ fwd.c | 31 +++++++++++++----- passt.1 | 75 ++++++++++++++++++++++++++----------------- passt.h | 2 ++ test/passt/dhcp | 2 ++ test/passt/ndp | 4 ++- test/passt_in_ns/dhcp | 2 ++ test/passt_in_ns/tcp | 8 ++--- test/passt_in_ns/udp | 4 +-- test/pasta/dhcp | 2 ++ test/pasta/ndp | 3 +- test/pasta/tcp | 16 ++++----- test/pasta/udp | 8 ++--- test/perf/passt_tcp | 2 ++ test/two_guests/basic | 6 +++- 16 files changed, 119 insertions(+), 63 deletions(-) -- 2.47.0

2 8

[PATCH v4 0/7] Don't expose container loopback services to the host
by David Gibson 18 Oct '24

18 Oct '24

podman issue #24045 pointed out that pasta's spliced forwarding logic can expose services within the namespace bound only to 127.0.0.1 or ::1 to the host. However, the namespace probably expects those to only be accessible to itself, so that's probably not what we want. This changes our forwarding logic so as not to do this. Note that the podman tests will currently fail with this series, I've submitted podman PR https://github.com/containers/podman/pull/24064 to fix that. Link: https://github.com/containers/podman/issues/24045 Changes since v3: * Added several extra patches working around failures on Debian, because its dhclient-script doesn't wait for DAD to complete Changes since v2: * Add new field do structure comment Changes since v1: * Add --host-lo-to-ns-lo option to preserve the old behaviour * Clarify the new behaviour in the man page * Add some extra patches making some other corrections to the man page David Gibson (7): arp: Fix a handful of small warts test: Explicitly wait for DAD to complete on SLAAC addresses test: Wait for DAD on DHCPv6 addresses passt.1: Mark --stderr as deprecated more prominently passt.1: Clarify and update "Handling of local addresses" section test: Clarify test for spliced inbound transfers fwd: Direct inbound spliced forwards to the guest's external address arp.c | 8 ++--- conf.c | 9 ++++++ fwd.c | 31 +++++++++++++----- passt.1 | 75 ++++++++++++++++++++++++++----------------- passt.h | 2 ++ test/passt/dhcp | 2 ++ test/passt/ndp | 4 ++- test/passt_in_ns/tcp | 8 ++--- test/passt_in_ns/udp | 4 +-- test/pasta/dhcp | 2 ++ test/pasta/ndp | 3 +- test/pasta/tcp | 16 ++++----- test/pasta/udp | 8 ++--- test/two_guests/basic | 6 +++- 14 files changed, 115 insertions(+), 63 deletions(-) -- 2.47.0

2 11

[PATCH v7 0/8] Add vhost-user support to passt. (part 3)
by Laurent Vivier 17 Oct '24

17 Oct '24

This series of patches adds vhost-user support to passt and then allows passt to connect to QEMU network backend using virtqueue rather than a socket. With QEMU, rather than using to connect: -netdev stream,id=s,server=off,addr.type=unix,addr.path=/tmp/passt_1.socket we will use: -chardev socket,id=chr0,path=/tmp/passt_1.socket -netdev vhost-user,id=netdev0,chardev=chr0 -device virtio-net,netdev=netdev0 -object memory-backend-memfd,id=memfd0,share=on,size=$RAMSIZE -numa node,memdev=memfd0 The memory backend is needed to share data between passt and QEMU. Performance comparison between "-netdev stream" and "-netdev vhost-user": $ iperf3 -c localhost -p 10001 -t 60 -6 -u -b 50G socket: [ 5] 0.00-60.05 sec 95.6 GBytes 13.7 Gbits/sec 0.017 ms 6998988/10132413 (69%) receiver vhost-user: [ 5] 0.00-60.04 sec 237 GBytes 33.9 Gbits/sec 0.006 ms 53673/7813770 (0.69%) receiver $ iperf3 -c localhost -p 10001 -t 60 -4 -u -b 50G socket: [ 5] 0.00-60.05 sec 98.9 GBytes 14.1 Gbits/sec 0.018 ms 6260735/9501832 (66%) receiver vhost-user: [ 5] 0.00-60.05 sec 235 GBytes 33.7 Gbits/sec 0.008 ms 37581/7752699 (0.48%) receiver $ iperf3 -c localhost -p 10001 -t 60 -6 socket: [ 5] 0.00-60.00 sec 17.3 GBytes 2.48 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 17.3 GBytes 2.48 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 191 GBytes 27.4 Gbits/sec 0 sender [ 5] 0.00-60.05 sec 191 GBytes 27.3 Gbits/sec receiver $ iperf3 -c localhost -p 10001 -t 60 -4 socket: [ 5] 0.00-60.00 sec 15.6 GBytes 2.24 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 15.6 GBytes 2.24 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 189 GBytes 27.1 Gbits/sec 0 sender [ 5] 0.00-60.04 sec 189 GBytes 27.0 Gbits/sec receiver 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 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 | 15 +- packet.c | 91 ++-- packet.h | 22 +- passt.1 | 10 +- passt.c | 11 +- passt.h | 6 + pcap.c | 1 - tap.c | 128 +++-- tap.h | 7 +- tcp.c | 37 +- tcp_internal.h | 15 + tcp_vu.c | 476 +++++++++++++++++++ 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 | 332 +++++++++++++ udp_vu.h | 13 + util.h | 8 + vhost_user.c | 1005 ++++++++++++++++++++++++++++++++++++++++ vhost_user.h | 206 ++++++++ virtio.c | 660 ++++++++++++++++++++++++++ virtio.h | 184 ++++++++ vu_common.c | 358 ++++++++++++++ vu_common.h | 47 ++ 37 files changed, 4135 insertions(+), 154 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.46.2

2 17

[PATCH] tcp: Send "empty" handshake ACK before first data segment
by Stefano Brivio 15 Oct '24

15 Oct '24

Starting from commit 9178a9e3462d ("tcp: Always send an ACK segment once the handshake is completed"), we always send an ACK segment, without any payload, to complete the three-way handshake while establishing a connection started from a socket. We queue that segment after checking if we already have data to send to the tap, which means that its sequence number is higher than any segment with data we're sending in the same iteration, if any data is available on the socket. However, in tcp_defer_handler(), we first flush "flags" buffers, that is, we send out segments without any data first, and then segments with data, which means that our "empty" ACK is sent before the ACK segment with data (if any), which has a lower sequence number. This appears to be harmless as the guest or container will generally reorder segments, but it looks rather weird and we can't exclude it's actually causing problems. Queue the empty ACK first, so that it gets a lower sequence number, before checking for any data from the socket. Reported-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tcp.c b/tcp.c index 9617b7a..b2155ab 100644 --- a/tcp.c +++ b/tcp.c @@ -1957,11 +1957,12 @@ static void tcp_conn_from_sock_finish(const struct ctx *c, return; } + tcp_send_flag(c, conn, ACK); + /* The client might have sent data already, which we didn't * dequeue waiting for SYN,ACK from tap -- check now. */ tcp_data_from_sock(c, conn); - tcp_send_flag(c, conn, ACK); } /** -- 2.43.0

2 1

2024

2023

2022

2021

dev