[PATCH] tcp_splice: avoid delay on certain transfers

[PATCH] tcp_splice: avoid delay on certain transfers

[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

Re: [PATCH] tcp_splice: avoid delay on certain transfers

[Stefano Brivio]

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

Re: [PATCH] tcp_splice: avoid delay on certain transfers

[Bernhard M. Wiedemann]

[-- 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 --]

Re: [PATCH] tcp_splice: avoid delay on certain transfers

[Stefano Brivio]

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>
> ---

Applied, thanks, sorry for the delay, and welcome to the git log!

> Notes:
>   The change was slightly tested and benchmarked. Results look decent.

I also played a bit with it and the scripts you supplied in different
setups, and couldn't find any relevant issue either.

It might be worth having them (with license, usage and all) under, say,
doc/performance/ or something like that. I don't have a big preference
either way, though, because after all we already have the same tests
using iperf3 and tcp_rr.

>   Not sure if we actually need the flow_trace for error handling there.

I think yes, that's actually useful should it ever happen.

-- 
Stefano

Re: [PATCH] tcp_splice: avoid delay on certain transfers

[Bernhard M. Wiedemann]

[-- Attachment #1.1: Type: text/plain, Size: 301 bytes --]



On 14/10/2026 13.28, Stefano Brivio wrote:
> It might be worth having them (with license, usage and all) under, say,
> doc/performance/ or something like that.

I declare them CC0 licensed (quasi public-domain), so feel free to add 
them with whatever license you like.

Ciao
Bernhard M.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]