* [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).