* Re: [PATCH mptcp-next] selftests: mptcp: sockopt: test passive Fast Open early write push
2026-10-08 11:10 [PATCH mptcp-next] selftests: mptcp: sockopt: test passive Fast Open early write push T S Rameshkumar
@ 2026-10-08 11:19 ` sashiko-bot
2026-10-08 11:31 ` Matthieu Baerts
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-08 11:19 UTC (permalink / raw)
To: T S Rameshkumar; +Cc: mptcp
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests: mptcp: Fast Open test falls back to 3WHS due to missing TFO cookie
- [Medium] selftests/mptcp: Fast Open tests fail CI on older kernels missing support
--
commit b13d72847e34610069932e37b3730c9df5c74745
Author: T S Rameshkumar <rameshsv06@gmail.com>
selftests: mptcp: sockopt: test passive Fast Open early write push
This commit adds automated test coverage for passive Fast Open early write
data push in MPTCP. The server enables TCP_FASTOPEN on an MPTCP listening
socket, and the client connects using TCP_FASTOPEN_CONNECT.
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> index d68515b7903b8..861de909abbd8 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
[ ... ]
> @@ -254,6 +255,14 @@ static int sock_connect_mptcp(const char * const remoteaddr,
> if (sock < 0)
> continue;
>
> + if (do_tfo) {
> + int one = 1;
> +
> + if (setsockopt(sock, IPPROTO_TCP, TCP_FASTOPEN_CONNECT,
> + &one, sizeof(one)) < 0)
> + die_perror("setsockopt TCP_FASTOPEN_CONNECT");
> + }
> +
[Severity: Medium]
Since MPTCP Fast Open was introduced in Linux 6.1+, will unconditionally
calling setsockopt() cause CI failures on older LTS kernels?
On older kernels, setsockopt() returns -EOPNOTSUPP. By using die_perror(),
the test terminates with a hard failure instead of gracefully skipping.
Could this be updated to catch EOPNOTSUPP and exit with a designated SKIP
code?
> if (connect(sock, a->ai_addr, a->ai_addrlen) == 0)
> break; /* success */
>
[ ... ]
> @@ -726,13 +738,36 @@ static int server(int pipefd)
> break;
> }
>
> + if (do_tfo) {
> + int qlen = 5;
> +
> + if (setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) < 0)
> + die_perror("setsockopt TCP_FASTOPEN");
> + }
> +
[Severity: Medium]
Similarly, will this cause CI failures on older kernels without MPTCP
Fast Open support?
A failure here returns -EOPNOTSUPP, but die_perror() will cause a hard
failure instead of gracefully bypassing the test.
> r = write(pipefd, "conn", 4);
> assert(r == 4);
>
[ ... ]
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index b4e3eddc9cf21..e88a8bbeb6e36 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
[ ... ]
> @@ -359,6 +359,39 @@ do_tcpinq_tests()
> return $?
> }
>
> +do_fastopen_tests()
> +{
> + local lret=0
> +
> + ip netns exec "$ns_sbox" sysctl -q net.ipv4.tcp_fastopen=3
> +
> + ip netns exec "$ns_sbox" ./mptcp_sockopt -f
> + lret=$?
> +
[Severity: Medium]
Will this sequence actually exercise the Fast Open early write feature?
Configuring net.ipv4.tcp_fastopen=3 requires a valid TFO cookie to accept
data within the initial SYN packet. Because the script executes
./mptcp_sockopt -f exactly once per address family, the client has an empty
cookie cache.
When connect() is called with TCP_FASTOPEN_CONNECT without a cookie, the
kernel falls back to a normal blocking 3-way handshake that requests a
cookie but sends no data in the SYN packet. The server's accept() will
block until the 3-way handshake finishes, bypassing the TFO early write
(TCP_SYN_RECV) state entirely.
Could the test be executed twice (first to get the cookie, second to use it)
or should tcp_fastopen be configured to allow cookieless Fast Open (e.g.,
adding TFO_SERVER_COOKIE_NOT_REQD)?
> + print_title "Fast Open early write v4"
> + if [ $lret -ne 0 ]; then
> + mptcp_lib_pr_fail
> + mptcp_lib_result_fail "Fast Open early write v4"
> + ret=$lret
> + return
> + fi
> + mptcp_lib_pr_ok
> + mptcp_lib_result_pass "Fast Open early write v4"
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261008111003.136143-1-rameshkumar.t@phytecembedded.in?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH mptcp-next] selftests: mptcp: sockopt: test passive Fast Open early write push
2026-10-08 11:10 [PATCH mptcp-next] selftests: mptcp: sockopt: test passive Fast Open early write push T S Rameshkumar
` (2 preceding siblings ...)
2026-10-08 13:16 ` MPTCP CI
@ 2026-10-12 11:43 ` netdev-bot+sashiko
3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-12 11:43 UTC (permalink / raw)
To: rameshsv06
Cc: matttbe, martineau, geliang, netdev, mptcp, linux-kernel,
rameshkumar.t, kuba
Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 3 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The new test never runs the code path it was written to cover.
- [Medium] do_fastopen_tests() runs the TFO subtests without first
checking that the kernel supports MPTCP Fast Open.
- [Medium] The TFO branches of client() and server() in mptcp_sockopt.c
skip every MPTCP-level check.
- [Medium] Even if passive TFO were actually triggered (see the
sysctl/cookie finding), the test has nothing that keeps the handshake…
- [Low] The commit author is 'T S Rameshkumar <rameshsv06@gmail.com>',
but the only Signed-off-by is 'T S Rameshkumar…
- [Low] parse_opts() in mptcp_sockopt.c now accepts '-f' (the getopt
string changed from "h6" to "h6f"), but die_usage(), which both -h and…
- [Low] Result checking in the TFO path is fragile and weaker than in the
non-TFO path.
--
selftests: mptcp: sockopt: test passive Fast Open early write push
This adds a -f option to mptcp_sockopt. It enables TCP_FASTOPEN on the
MPTCP listener and TCP_FASTOPEN_CONNECT on the client, then exchanges a
short request and reply. mptcp_sockopt.sh gains do_fastopen_tests(),
which sets net.ipv4.tcp_fastopen=3 in ns_sbox and runs the helper over
IPv4 and IPv6.
> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
[Severity: Low]
The author is T S Rameshkumar <rameshsv06@gmail.com>, but the only
Signed-off-by uses rameshkumar.t@phytecembedded.in. checkpatch reports
this as FROM_SIGN_OFF_MISMATCH.
Could the Signed-off-by, the author, or an explicit From: line be
updated so they match?
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> index d68515b7903b8..861de909abbd8 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
[ ... ]
> @@ -271,7 +280,7 @@ static void parse_opts(int argc, char **argv)
> {
> int c;
>
> - while ((c = getopt(argc, argv, "h6")) != -1) {
> + while ((c = getopt(argc, argv, "h6f")) != -1) {
[Severity: Low]
This isn't a bug, but die_usage() still prints:
fprintf(stderr, "Usage: mptcp_sockopt [-6]\n");
Should the usage string also list the new -f option?
[ ... ]
> @@ -726,13 +738,36 @@ static int server(int pipefd)
[ ... ]
> alarm(15);
> r = xaccept(fd);
>
> - process_one_client(r, pipefd);
> + if (do_tfo) {
> + char buf[64] = {0};
> + ssize_t n;
> +
> + n = read(r, buf, sizeof(buf) - 1);
> + if (n <= 0)
> + die_perror("read tfo syn data");
> +
> + n = write(r, "PONG_REPLY", 10);
> + if (n != 10)
> + die_perror("write tfo reply");
[Severity: Medium]
Even if passive TFO were triggered, what keeps the handshake incomplete
while this write runs? server() does xaccept() -> read() ->
write("PONG_REPLY").
On loopback, the server stack is likely to process the client's final
ACK in softirq before the process returns from accept(), or between the
read() and the write().
Nothing in the test holds that ACK back (for example, a netfilter rule
that drops the final ACK). Nothing checks that the socket is still in
SYN_RECV when the write happens either.
Wouldn't a normal write on an established socket still be reported as a
"Fast Open early write" pass?
> +
> + n = read(r, buf, 1);
> + close(r);
> + } else {
> + process_one_client(r, pipefd);
> + }
[Severity: Medium]
This TFO branch, and the one added to client(), skip every MPTCP-level
check. In the non-TFO path, process_one_client() and
connect_one_server() call do_getsockopts() (MPTCP_INFO, MPTCP_TCPINFO,
...). They also check mptcpi_rcv_delta and the bytes_sent, received and
acked counters.
The new branches only do a plain write/read exchange and never call
getsockopt(SOL_MPTCP, ...).
If MPTCP with TFO silently fell back to plain TCP, wouldn't this still
pass?
> @@ -844,6 +879,26 @@ static int client(int pipefd)
>
> alarm(15);
>
> + if (do_tfo) {
[ ... ]
> + setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
> + n = read(fd, buf, sizeof(buf) - 1);
> + if (n <= 0 || strcmp(buf, "PONG_REPLY") != 0)
> + xerror("Fastopen early write failed: received %zd bytes '%s'", n, buf);
[Severity: Low]
On a stream socket, read() can return fewer than the 10 bytes the
server wrote. Could a short read here cause a spurious failure, since
the result is compared exactly against "PONG_REPLY" with strcmp()?
On the server side, the return value of the final read in server() is
stored but never checked:
n = read(r, buf, 1);
close(r);
process_one_client() checks for EOF instead:
ret3 = read(fd, buf, 1);
if (ret3 != 0)
xerror("expected EOF, got %lu", ret3);
Should the TFO path do the same, so that read errors or unexpected extra
data are not silently ignored?
[ ... ]
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index b4e3eddc9cf21..e88a8bbeb6e36 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -359,6 +359,39 @@ do_tcpinq_tests()
> return $?
> }
>
> +do_fastopen_tests()
> +{
> + local lret=0
> +
[Severity: Medium]
Should this function check for MPTCP Fast Open support before it runs?
On a kernel without that support, one of two setsockopt() calls fails:
TCP_FASTOPEN in server() or TCP_FASTOPEN_CONNECT in
sock_connect_mptcp(). die_perror() then exits with 1. The script
records "Fast Open early write v4" as a failure and sets ret, where a
skip would be expected.
do_tcpinq_tests() in this script already handles this case:
if ! mptcp_lib_kallsyms_has "mptcp_ioctl$"; then
mptcp_lib_pr_skip "TCP_INQ not supported"
mptcp_lib_result_skip "TCP_INQ"
return
fi
run_tests_mptfo() in mptcp_connect.sh does the same with
mptcp_lib_kallsyms_has "mptcp_fastopen_".
> + ip netns exec "$ns_sbox" sysctl -q net.ipv4.tcp_fastopen=3
[Severity: High]
Does this test ever reach the passive Fast Open path it is meant to
cover?
The value 3 is TFO_CLIENT_ENABLE | TFO_SERVER_ENABLE. It does not set
TFO_CLIENT_NO_COOKIE (0x4) or TFO_SERVER_COOKIE_NOT_REQD (0x200). Also,
sock_connect_mptcp() does not set TCP_FASTOPEN_NO_COOKIE.
Each mptcp_sockopt -f run makes a single connection, first to 127.0.0.1
and then to ::1. So no cookie is cached for either destination in
ns_sbox. On the client, tcp_fastopen_defer_connect() then gets false
from here:
net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_check() {
...
if (cookie->len > 0)
return true;
tcp_sk(sk)->fastopen_client_fail = TFO_COOKIE_UNAVAILABLE;
return false;
}
As a result, connect() does a normal handshake and sends a cookie-request
SYN with no data.
On the server, tcp_try_fastopen() only generates a cookie:
if (foc->len == 0) {
/* Client requests a cookie. */
tcp_fastopen_cookie_gen(sk, req, skb, &valid_foc);
No TFO child socket is created. accept() returns an ESTABLISHED socket,
PING_DATA arrives after the handshake, and PONG_REPLY is an ordinary
write.
Neither side checks that TFO actually happened. There is no
TCPI_OPT_SYN_DATA check on tcpi_options and no TcpExtTCPFastOpenPassive
check, and the server doesn't compare the payload it reads.
Wouldn't this pass whether or not the MPTCP passive TFO early write path
works?
This also seems to contradict the commit message, which says the server:
"accepts an incoming connection with SYN payload data, and immediately
writes reply bytes before the 3-way handshake fully completes"
For comparison, run_tests_mptfo() in mptcp_connect.sh connects to each
address twice, so the second connection has a cookie:
run_tests_lo "$ns1" "$ns2" 10.0.1.1 0 "-o MPTFO"
run_tests_lo "$ns1" "$ns2" 10.0.1.1 0 "-o MPTFO"
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008111003.136143-1-rameshkumar.t%40phytecembedded.in
^ permalink raw reply [flat|nested] 5+ messages in thread