* [PATCH] tcp_splice: avoid delay on certain transfers
@ 2026-10-02 16:39 Bernhard M. Wiedemann
2026-10-05 19:37 ` Stefano Brivio
0 siblings, 1 reply; 3+ messages in thread
From: Bernhard M. Wiedemann @ 2026-10-02 16:39 UTC (permalink / raw)
To: passt-dev; +Cc: Bernhard M. Wiedemann
without this patch, sending buffers of certain sizes
caused a delay of 200ms because tcp_splice_forward()
passes SPLICE_F_MORE to the writer
when a read filled at least 90% of the pipe.
This is easy to hit when pipes are small: once a user exceeds
fs.pipe-user-pages-soft (64 MiB by default, which a few long-running
pasta instances with large pipes reach on their own),
tcp_set_pipe_size() settles on the 8 KiB minimum, and every message
whose length is 7372 to 8192 bytes (90% of the pipe and more) past a
multiple of 8 KiB stalls.
This change leaves bulk throughput unchanged.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>
---
Notes:
The change was slightly tested and benchmarked. Results look decent.
Not sure if we actually need the flow_trace for error handling there.
This issue was accidentally found by running PostgreSQL in a rootless
podman container for benchmarking.
tcp_splice.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/tcp_splice.c b/tcp_splice.c
index 4b01f1a..f7a3913 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -488,6 +488,7 @@ static int tcp_splice_forward(struct ctx *c,
{
uint8_t lowat_set_flag = RCVLOWAT_SET(fromsidei);
uint8_t lowat_act_flag = RCVLOWAT_ACT(fromsidei);
+ bool corked = false;
while (1) {
ssize_t readlen, written;
@@ -517,8 +518,17 @@ static int tcp_splice_forward(struct ctx *c,
* there's nothing in the pipe so there's nothing to do
* write side either.
*/
- if (!conn->pending[fromsidei])
+ if (!conn->pending[fromsidei]) {
+ /* Setting TCP_NODELAY again flushes data held
+ * back by SPLICE_F_MORE
+ */
+ if (corked &&
+ setsockopt(conn->s[!fromsidei], SOL_TCP,
+ TCP_NODELAY, &((int){ 1 }),
+ sizeof(int)))
+ flow_trace(conn, "failed to push data");
break;
+ }
} else {
conn->pending[fromsidei] += readlen;
@@ -549,6 +559,9 @@ static int tcp_splice_forward(struct ctx *c,
if (written < 0)
break;
+ if (written > 0)
+ corked = more;
+
conn->pending[fromsidei] -= written;
if (!conn->pending[fromsidei] && readlen <= 0) {
--
2.55.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] tcp_splice: avoid delay on certain transfers 2026-10-02 16:39 [PATCH] tcp_splice: avoid delay on certain transfers Bernhard M. Wiedemann @ 2026-10-05 19:37 ` Stefano Brivio 2026-10-06 9:41 ` Bernhard M. Wiedemann 0 siblings, 1 reply; 3+ messages in thread From: Stefano Brivio @ 2026-10-05 19:37 UTC (permalink / raw) To: Bernhard M. Wiedemann; +Cc: passt-dev Bernard, thanks for the investigation and for the patch. Just one doubt: On Fri, 2 Oct 2026 18:39:25 +0200 "Bernhard M. Wiedemann" <bwiedemann@suse.de> wrote: > without this patch, sending buffers of certain sizes > caused a delay of 200ms because tcp_splice_forward() > passes SPLICE_F_MORE to the writer > when a read filled at least 90% of the pipe. > > This is easy to hit when pipes are small: once a user exceeds > fs.pipe-user-pages-soft (64 MiB by default, which a few long-running > pasta instances with large pipes reach on their own), > tcp_set_pipe_size() settles on the 8 KiB minimum, and every message > whose length is 7372 to 8192 bytes (90% of the pipe and more) past a > multiple of 8 KiB stalls. > > This change leaves bulk throughput unchanged. > > Assisted-by: Claude:claude-opus-5-5 > Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de> > --- > Notes: > The change was slightly tested and benchmarked. Results look decent. > Not sure if we actually need the flow_trace for error handling there. > This issue was accidentally found by running PostgreSQL in a rootless > podman container for benchmarking. > > tcp_splice.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/tcp_splice.c b/tcp_splice.c > index 4b01f1a..f7a3913 100644 > --- a/tcp_splice.c > +++ b/tcp_splice.c > @@ -488,6 +488,7 @@ static int tcp_splice_forward(struct ctx *c, > { > uint8_t lowat_set_flag = RCVLOWAT_SET(fromsidei); > uint8_t lowat_act_flag = RCVLOWAT_ACT(fromsidei); > + bool corked = false; > > while (1) { > ssize_t readlen, written; > @@ -517,8 +518,17 @@ static int tcp_splice_forward(struct ctx *c, > * there's nothing in the pipe so there's nothing to do > * write side either. > */ > - if (!conn->pending[fromsidei]) > + if (!conn->pending[fromsidei]) { > + /* Setting TCP_NODELAY again flushes data held > + * back by SPLICE_F_MORE ...nice, I didn't know about that trick. But wouldn't it be more natural to not use SPLICE_F_MORE if the pipe is small enough? Do you have a stand-alone reproducer that could help figuring this out? I'm a bit worried we might cause unnecessary setsockopt() calls in some corner cases if we go this way, even though it's a rather minor concern (we just called splice(), and that setsockopt() is not _that_ expensive in comparison). > + */ > + if (corked && > + setsockopt(conn->s[!fromsidei], SOL_TCP, > + TCP_NODELAY, &((int){ 1 }), > + sizeof(int))) > + flow_trace(conn, "failed to push data"); > break; > + } > } else { > conn->pending[fromsidei] += readlen; > > @@ -549,6 +559,9 @@ static int tcp_splice_forward(struct ctx *c, > if (written < 0) > break; > > + if (written > 0) > + corked = more; > + > conn->pending[fromsidei] -= written; > > if (!conn->pending[fromsidei] && readlen <= 0) { -- Stefano ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tcp_splice: avoid delay on certain transfers 2026-10-05 19:37 ` Stefano Brivio @ 2026-10-06 9:41 ` Bernhard M. Wiedemann 0 siblings, 0 replies; 3+ messages in thread From: Bernhard M. Wiedemann @ 2026-10-06 9:41 UTC (permalink / raw) To: Stefano Brivio; +Cc: passt-dev [-- Attachment #1.1.1: Type: text/plain, Size: 3265 bytes --] On 05/10/2026 21.37, Stefano Brivio wrote: > Bernard, thanks for the investigation and for the patch. Just one doubt: [...] > I'm a bit worried we might cause unnecessary setsockopt() calls in some > corner cases if we go this way, even though it's a rather minor concern > (we just called splice(), and that setsockopt() is not _that_ expensive > in comparison). The 'corked' variable should ensure that we only add the setsockopt when it would otherwise add the unneccesary 200ms delay. I attached the reproducers and benchmarking tools. The finding from running it was that SPLICE_F_MORE gives plenty extra performance even for 8K pipes and setsockopt hardly costs anything. To test: # maybe with adjustment to /proc/sys/fs/pipe-user-pages-soft pasta --config-net -t 127.0.0.1/25432:15432 -- python3 rr.py serve 15432 python3 rr.py 25432 ┌─────────┬─────────┬───────────────┬─────────────────┬────────────────┐ │ Build │ Pipes │ Worst latency │ Upload (Gbit/s) │ Download/Gbit │ ├─────────┼─────────┼───────────────┼─────────────────┼────────────────┤ │ base │ 8 KiB │ 208 ms │ 89–91 │ 87–89 │ ├─────────┼─────────┼───────────────┼─────────────────┼────────────────┤ │ patched │ 8 KiB │ 0.1 ms │ 88–89 │ 86–89 │ ├─────────┼─────────┼───────────────┼─────────────────┼────────────────┤ │ nomore │ 8 KiB │ 0.1 ms │ 42–43 │ 40 │ ├─────────┼─────────┼───────────────┼─────────────────┼────────────────┤ │ base │ default │ 0.3 ms │ 203–204 │ 189–198 │ ├─────────┼─────────┼───────────────┼─────────────────┼────────────────┤ │ patched │ default │ 0.1 ms │ 203–205 │ 185–190 │ ├─────────┼─────────┼───────────────┼─────────────────┼────────────────┤ │ nomore │ default │ 0.1 ms │ 193–203 │ 177–190 │ └─────────┴─────────┴───────────────┴─────────────────┴────────────────┘ Ciao Bernhard M. [-- Attachment #1.1.2: bulk.py --] [-- Type: text/x-python, Size: 1891 bytes --] """Bulk throughput through a TCP port. server: bulk.py serve PORT; client: bulk.py PORT MiB RUNS""" import socket, statistics, sys, time CH = 1 << 20 def sink(s): buf = bytearray(CH); n = 0 while (r := s.recv_into(buf)): n += r return n def source(s, total): mv = memoryview(bytes(CH)); sent = 0 while sent < total: s.sendall(mv); sent += CH s.shutdown(socket.SHUT_WR) if sys.argv[1] == "serve": l = socket.socket(); l.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) l.bind(("0.0.0.0", int(sys.argv[2]))); l.listen() while True: s, _ = l.accept(); f = s.makefile("rb"); cmd = f.readline().split() if not cmd: # readiness probe f.close(); s.close(); continue if cmd[0] == b"up": # client sends, we count and reply with byte count buf = bytearray(CH); n = 0 while (r := f.readinto(buf)): n += r s.sendall(str(n).encode()); f.close(); s.close() else: # down: we send N bytes source(s, int(cmd[1])); sink(s); f.close(); s.close() else: port, mib, runs = int(sys.argv[1]), int(sys.argv[2]), int(sys.argv[3]); total = mib * CH for d in (sys.argv[4:] or ["up", "down"]): rates = [] for _ in range(runs): s = socket.create_connection(("127.0.0.1", port)); t0 = time.perf_counter() s.sendall(f"{d} {total}\n".encode()) if d == "up": source(s, total) got = b"" while (c := s.recv(64)): got += c assert int(got) == total, got else: assert sink(s) == total rates.append(total / (time.perf_counter() - t0) / 1e9 * 8) s.close() print(f"{d:4}: median {statistics.median(rates):6.2f} Gbit/s (" + " ".join(f"{r:.2f}" for r in rates) + ")", flush=True) [-- Attachment #1.1.3: rr.py --] [-- Type: text/x-python, Size: 1410 bytes --] """Request/response latency through a TCP port. server: rr.py serve PORT; client: rr.py PORT""" import socket, struct, sys, time def recvn(s, n): b = bytearray() while len(b) < n: c = s.recv(n - len(b)) if not c: raise EOFError b += c return bytes(b) if sys.argv[1] == "serve": l = socket.socket(); l.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) l.bind(("0.0.0.0", int(sys.argv[2]))); l.listen() while True: s, _ = l.accept(); s.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) try: while True: req, resp = struct.unpack("!II", recvn(s, 8)); recvn(s, req); s.sendall(b"r" * resp) except EOFError: s.close() else: s = socket.create_connection(("127.0.0.1", int(sys.argv[1]))); s.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) for label, req, resp in [("request", n, 16) for n in (4000, 7372, 7380, 8000, 9000, 16000, 20000)] + [("response", 16, n) for n in (4000, 7380, 8000, 16000, 64000)]: ts = [] for _ in range(3): t0 = time.perf_counter(); s.sendall(struct.pack("!II", req, resp) + b"q" * req); recvn(s, resp); ts.append((time.perf_counter() - t0) * 1000) print(f"{label:8} {max(req, resp) if label == 'request' and False else (req if label=='request' else resp):6d} B: " + " ".join(f"{t:6.1f}" for t in ts) + " ms", flush=True) [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-06 9:41 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-02 16:39 [PATCH] tcp_splice: avoid delay on certain transfers Bernhard M. Wiedemann 2026-10-05 19:37 ` Stefano Brivio 2026-10-06 9:41 ` Bernhard M. Wiedemann
Code repositories for project(s) associated with this public inbox https://passt.top/passt This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for IMAP folder(s).