[PATCH mptcp-next 00/11] mptcp: avoid dup events + error + misc.

[PATCH mptcp-next 00/11] mptcp: avoid dup events + error + misc.

[Matthieu Baerts (NGI0)]

Here is my last series of the year :)

There are two fixes with their tests:

- a subflow closed event can be visible multiple times, see issue #603

- the subflow closed event didn't contain the error as expected.

The 4 first patches are for -net, the rest are small fixes and
improvements for net-next. Some tests should be quicker now.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (11):
      mptcp: avoid dup SUB_CLOSED events after disconnect
      selftests: mptcp: check no dup close events after error
      mptcp: only reset subflow errors when propagated
      selftests: mptcp: check subflow errors in close events
      selftests: mptcp: join: wait for estab event instead of MPJ
      selftests: mptcp: join: fix wait_mpj helper
      selftests: mptcp: join: userspace: wait for new events
      selftests: mptcp: join chk_stale_nr: avoid dup stats
      selftests: mptcp: join: avoid declaring i if not used
      selftests: mptcp: connect: fix maybe-uninitialize warn
      selftests: mptcp: connect cleanup TFO setup

 net/mptcp/protocol.c                              |   9 +-
 tools/testing/selftests/net/mptcp/mptcp_connect.c |  19 ++-
 tools/testing/selftests/net/mptcp/mptcp_join.sh   | 146 +++++++++++++++-------
 3 files changed, 116 insertions(+), 58 deletions(-)
---
base-commit: b3ae75d680ea6ef281101b0f5029056c4f87ff02
change-id: 20251222-mptcp-issue-603-d85d967f7c0a

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-next 01/11] mptcp: avoid dup SUB_CLOSED events after disconnect

[Matthieu Baerts (NGI0)]

In case of subflow disconnect(), which can also happen with the first
subflow in case of errors like timeout or reset, mptcp_subflow_ctx_reset
will reset most fields from the mptcp_subflow_context structure,
including close_event_done. Then, when another subflow is closed, yet
another SUB_CLOSED event for the disconnected initial subflow is sent.
Because of the previous reset, there are no source address and
destination port.

A solution is then to also check the subflow's local id: it shouldn't be
negative anyway.

Another solution would be not to reset subflow->close_event_done at
disconnect time, but when reused. But then, probably the whole reset
could be done when being reused. Let's not change this logic, similar
to TCP with tcp_disconnect().

Fixes: d82809b6c5f2 ("mptcp: avoid duplicated SUB_CLOSED events")
Reported-by: Marco Angaroni <marco.angaroni@italtel.com>
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/603
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 900f26e21acd..1d68648b5194 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2622,8 +2622,8 @@ void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct sk_buff *skb;
 
-	/* The first subflow can already be closed and still in the list */
-	if (subflow->close_event_done)
+	/* The 1st subflow can already be closed/disco and still in the list */
+	if (subflow->close_event_done || READ_ONCE(subflow->local_id) < 0)
 		return;
 
 	subflow->close_event_done = true;

-- 
2.51.0

[PATCH mptcp-next 02/11] selftests: mptcp: check no dup close events after error

[Matthieu Baerts (NGI0)]

This validates the previous commit: subflow closed events are re-sent
with less info when the initial subflow is disconnected after an error
and each time a subflow is closed after that.

In this new test, the userspace PM is involved because that's how it was
discovered, but it is not specific to it. The initial subflow is
terminated with a RESET, and that will cause the subflow disconnect.
Then, a new subflow is initiated, but also got rejected, which cause a
second subflow closed event, but not a third one.

While at it, in case of failure to get the expected amount of events,
the events are printed.

The 'Fixes' tag here below is the same as the one from the previous
commit: this patch here is not fixing anything wrong in the selftests,
but it validates the previous fix for an issue introduced by this commit
ID.

Fixes: d82809b6c5f2 ("mptcp: avoid duplicated SUB_CLOSED events")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 49 +++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index b2e6e548f796..468ffb97293b 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3872,11 +3872,32 @@ chk_evt_nr()
 	count=$(grep -cw "type:${evt}" "${evts}")
 	if [ "${count}" != "${exp}" ]; then
 		fail_test "got ${count} events, expected ${exp}"
+		cat "${evts}"
 	else
 		print_ok
 	fi
 }
 
+# $1: ns ; $2: event type ; $3: expected count
+wait_event()
+{
+	local ns="${1}"
+	local evt_name="${2}"
+	local exp="${3}"
+
+	local evt="${!evt_name}"
+	local evts="${evts_ns1}"
+	local count
+
+	[ "${ns}" == "ns2" ] && evts="${evts_ns2}"
+
+	for _ in $(seq 100); do
+		count=$(grep -cw "type:${evt}" "${evts}")
+		[ "${count}" -ge "${exp}" ] && break
+		sleep 0.1
+	done
+}
+
 userspace_tests()
 {
 	# userspace pm type prevents add_addr
@@ -4085,6 +4106,34 @@ userspace_tests()
 		kill_events_pids
 		mptcp_lib_kill_group_wait $tests_pid
 	fi
+
+	# userspace pm no duplicated spurious close events after an error
+	if reset_with_events "userspace pm no dup close events after error" &&
+	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
+		set_userspace_pm $ns2
+		pm_nl_set_limits $ns1 0 2
+		{ timeout_test=120 test_linkfail=128 speed=slow \
+			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+		local tests_pid=$!
+		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
+		userspace_pm_add_sf $ns2 10.0.3.2 20
+		chk_mptcp_info subflows 1 subflows 1
+		chk_subflows_total 2 2
+
+		# force quick loss
+		ip netns exec $ns2 sysctl -q net.ipv4.tcp_syn_retries=1
+		if ip netns exec "${ns1}" ${iptables} -A INPUT -s "10.0.1.2" -p tcp --tcp-option 30 -j REJECT --reject-with tcp-reset &&
+		   ip netns exec "${ns2}" ${iptables} -A INPUT -d "10.0.1.2" -p tcp --tcp-option 30 -j REJECT --reject-with tcp-reset; then
+			wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1
+			wait_event ns1 MPTCP_LIB_EVENT_SUB_CLOSED 1
+			chk_subflows_total 1 1
+			userspace_pm_add_sf $ns2 10.0.1.2 0
+			wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2
+			chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2
+		fi
+		kill_events_pids
+		mptcp_lib_kill_group_wait $tests_pid
+	fi
 }
 
 endpoint_tests()

-- 
2.51.0

[PATCH mptcp-next 03/11] mptcp: only reset subflow errors when propagated

[Matthieu Baerts (NGI0)]

Some subflow socket errors need to be reported to the MPTCP socket: the
initial subflow connect (MP_CAPABLE), and the ones from the fallback
sockets. The others are not propagated.

The issue is that sock_error() was used to retrieve the error, which was
also resetting the sk_err field. Because of that, when notifying the
userspace about subflow close events later on from the MPTCP worker, the
ssk->sk_err field was always 0.

Now, the error (sk_err) is only reset when propagating it to the msk.

Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Note: I guess we could also duplicate the error in mptcp_subflow_context
struct before scheduling the worker, and use this field in
mptcp_event_put_token_and_ssk(), but I don't see why we should always
reset ssk->sk_err in __mptcp_subflow_error_report() in all cases.
---
 net/mptcp/protocol.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1d68648b5194..5d6c987089c6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -817,10 +817,9 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
 
 static bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk)
 {
-	int err = sock_error(ssk);
 	int ssk_state;
 
-	if (!err)
+	if (!READ_ONCE(ssk->sk_err))
 		return false;
 
 	/* only propagate errors on fallen-back sockets or
@@ -837,7 +836,7 @@ static bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk)
 	ssk_state = inet_sk_state_load(ssk);
 	if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD))
 		mptcp_set_state(sk, ssk_state);
-	WRITE_ONCE(sk->sk_err, -err);
+	WRITE_ONCE(sk->sk_err, -sock_error(ssk));
 
 	/* This barrier is coupled with smp_rmb() in mptcp_poll() */
 	smp_wmb();

-- 
2.51.0

[PATCH mptcp-next 04/11] selftests: mptcp: check subflow errors in close events

[Matthieu Baerts (NGI0)]

This validates the previous commit: subflow closed events should contain
an error field when a subflow got closed with an error, e.g. reset or
timeout.

For this test, the chk_evt_nr helper has been extended to check
attributes in the matched events.

In this test, the 2 subflow closed events should have an error.

The 'Fixes' tag here below is the same as the one from the previous
commit: this patch here is not fixing anything wrong in the selftests,
but it validates the previous fix for an issue introduced by this commit
ID.

Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 468ffb97293b..bf0cc59b5210 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3847,21 +3847,28 @@ userspace_pm_chk_get_addr()
 	fi
 }
 
-# $1: ns ; $2: event type ; $3: count
+# $1: ns ; $2: event type ; $3: count ; [ $4: attr ; $5: attr count ]
 chk_evt_nr()
 {
 	local ns=${1}
 	local evt_name="${2}"
 	local exp="${3}"
+	local attr="${4}"
+	local attr_exp="${5}"
 
 	local evts="${evts_ns1}"
 	local evt="${!evt_name}"
+	local attr_name
 	local count
 
+	if [ -n "${attr}" ]; then
+		attr_name=", ${attr}: ${attr_exp}"
+	fi
+
 	evt_name="${evt_name:16}" # without MPTCP_LIB_EVENT_
 	[ "${ns}" == "ns2" ] && evts="${evts_ns2}"
 
-	print_check "event ${ns} ${evt_name} (${exp})"
+	print_check "event ${ns} ${evt_name} (${exp}${attr_name})"
 
 	if [[ "${evt_name}" = "LISTENER_"* ]] &&
 	   ! mptcp_lib_kallsyms_has "mptcp_event_pm_listener$"; then
@@ -3873,6 +3880,16 @@ chk_evt_nr()
 	if [ "${count}" != "${exp}" ]; then
 		fail_test "got ${count} events, expected ${exp}"
 		cat "${evts}"
+		return
+	elif [ -z "${attr}" ]; then
+		print_ok
+		return
+	fi
+
+	count=$(grep -w "type:${evt}" "${evts}" | grep -c ",${attr}:")
+	if [ "${count}" != "${attr_exp}" ]; then
+		fail_test "got ${count} event attributes, expected ${attr_exp}"
+		grep -w "type:${evt}" "${evts}"
 	else
 		print_ok
 	fi
@@ -4129,7 +4146,7 @@ userspace_tests()
 			chk_subflows_total 1 1
 			userspace_pm_add_sf $ns2 10.0.1.2 0
 			wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2
-			chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2
+			chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2 error 2
 		fi
 		kill_events_pids
 		mptcp_lib_kill_group_wait $tests_pid

-- 
2.51.0

[PATCH mptcp-next 05/11] selftests: mptcp: join: wait for estab event instead of MPJ

[Matthieu Baerts (NGI0)]

'wait_mpj' was used just after having created a background connection,
but before creating new subflows. So no MPJ were sent. The intention was
to wait for the connection to be established, which was the same as
doing a simple sleep with a "random" value.

Instead, wait for an "established" event. With this, the tests can
finish quicker.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index bf0cc59b5210..9b58d7567c18 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -4000,7 +4000,7 @@ userspace_tests()
 		{ timeout_test=120 test_linkfail=128 speed=5 \
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
-		wait_mpj $ns1
+		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_addr $ns1 10.0.2.1 10
 		userspace_pm_add_addr $ns1 10.0.3.1 20
 		chk_join_nr 2 2 2
@@ -4033,7 +4033,7 @@ userspace_tests()
 		{ timeout_test=120 test_linkfail=128 speed=5 \
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
-		wait_mpj $ns2
+		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_sf $ns2 10.0.3.2 20
 		chk_join_nr 1 1 1
 		chk_mptcp_info subflows 1 subflows 1
@@ -4061,7 +4061,7 @@ userspace_tests()
 		{ timeout_test=120 test_linkfail=128 speed=5 \
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
-		wait_mpj $ns2
+		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
 		userspace_pm_add_sf $ns2 10.0.3.2 0
@@ -4082,7 +4082,7 @@ userspace_tests()
 		{ timeout_test=120 test_linkfail=128 speed=5 \
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
-		wait_mpj $ns2
+		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_sf $ns2 10.0.3.2 20
 		chk_join_nr 1 1 1
 		chk_mptcp_info subflows 1 subflows 1
@@ -4106,7 +4106,7 @@ userspace_tests()
 		{ timeout_test=120 test_linkfail=128 speed=5 \
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
-		wait_mpj $ns1
+		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_addr $ns1 10.0.2.1 10
 		chk_join_nr 1 1 1
 		chk_add_nr 1 1
@@ -4159,6 +4159,7 @@ endpoint_tests()
 	# userspace pm type prevents add_addr
 	if reset "implicit EP" &&
 	   continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
+		start_events
 		pm_nl_set_limits $ns1 2 2
 		pm_nl_set_limits $ns2 2 2
 		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
@@ -4166,7 +4167,7 @@ endpoint_tests()
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
 
-		wait_mpj $ns1
+		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		pm_nl_check_endpoint "creation" \
 			$ns2 10.0.2.2 id 1 flags implicit
 		chk_mptcp_info subflows 1 subflows 1
@@ -4180,6 +4181,7 @@ endpoint_tests()
 		pm_nl_check_endpoint "modif is allowed" \
 			$ns2 10.0.2.2 id 1 flags signal
 		mptcp_lib_kill_group_wait $tests_pid
+		kill_events_pids
 	fi
 
 	if reset_with_tcp_filter "delete and re-add" ns2 10.0.3.2 REJECT OUTPUT &&
@@ -4193,7 +4195,7 @@ endpoint_tests()
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
 
-		wait_mpj $ns2
+		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		pm_nl_check_endpoint "creation" \
 			$ns2 10.0.2.2 id 2 flags subflow dev ns2eth2
 		chk_subflow_nr "before delete id 2" 2
@@ -4271,7 +4273,7 @@ endpoint_tests()
 			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
 		local tests_pid=$!
 
-		wait_mpj $ns2
+		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		pm_nl_check_endpoint "creation" \
 			$ns1 10.0.2.1 id 1 flags signal
 		chk_subflow_nr "before delete" 2

-- 
2.51.0

[PATCH mptcp-next 06/11] selftests: mptcp: join: fix wait_mpj helper

[Matthieu Baerts (NGI0)]

It looks like most of the time, this helper was simply waiting a bit
more than one second: the previous MPJoin counter was often already at
the expected value. So at the end, it was just checking 10 times for
the MPJoin counter to change, but it was not happening. For the tests,
that was time, it was just waiting longer for nothing.

Instead, use 'wait_mpj' with the expected counter: in the tests, the MPJ
counter can easily be predicted. While at it, stop passing the netns as
argument: here the received MPJoin ACK is checked, which happens on the
server side. If later on, this needs to be checked on the client side,
the helper can be adapted for this case, but better avoid confusions now
if it is not needed.

While at it, stop using 'i' for the variable if it is not used.

With this, the tests can finish quicker.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 30 ++++++++++++-------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 9b58d7567c18..cf28537b71f7 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -631,17 +631,15 @@ wait_rm_sf()
 	done
 }
 
+# $1: expected MPJ ACK Rx counter in $ns1
 wait_mpj()
 {
-	local ns="${1}"
-	local cnt old_cnt
+	local exp_cnt="${1}"
+	local cnt
 
-	old_cnt=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPJoinAckRx")
-
-	local i
-	for i in $(seq 10); do
-		cnt=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPJoinAckRx")
-		[ "$cnt" = "${old_cnt}" ] || break
+	for _ in $(seq 10); do
+		cnt=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinAckRx")
+		[ "${cnt}" = "${exp_cnt}" ] && break
 		sleep 0.1
 	done
 }
@@ -4207,7 +4205,7 @@ endpoint_tests()
 		chk_mptcp_info subflows 0 subflows 0
 
 		pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow
-		wait_mpj $ns2
+		wait_mpj 2
 		chk_subflow_nr "after re-add id 2" 2
 		chk_mptcp_info subflows 1 subflows 1
 
@@ -4219,7 +4217,7 @@ endpoint_tests()
 		ip netns exec "${ns2}" ${iptables} -D OUTPUT -s "10.0.3.2" -p tcp -j REJECT
 		pm_nl_del_endpoint $ns2 3 10.0.3.2
 		pm_nl_add_endpoint $ns2 10.0.3.2 id 3 flags subflow
-		wait_mpj $ns2
+		wait_mpj 3
 		chk_subflow_nr "after no reject" 3
 		chk_mptcp_info subflows 2 subflows 2
 
@@ -4231,7 +4229,7 @@ endpoint_tests()
 			chk_mptcp_info subflows 2 subflows 2 # only decr for additional sf
 
 			pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow
-			wait_mpj $ns2
+			wait_mpj $((3 + i))
 			chk_subflow_nr "after re-add id 0 ($i)" 3
 			chk_mptcp_info subflows 3 subflows 3
 		done
@@ -4289,7 +4287,7 @@ endpoint_tests()
 
 		pm_nl_add_endpoint $ns1 10.0.2.1 id 1 flags signal
 		pm_nl_add_endpoint $ns1 10.0.3.1 id 2 flags signal
-		wait_mpj $ns2
+		wait_mpj 3
 		chk_subflow_nr "after re-add" 3
 		chk_mptcp_info subflows 2 subflows 2
 		chk_mptcp_info add_addr_signal 2 add_addr_accepted 2
@@ -4301,7 +4299,7 @@ endpoint_tests()
 		chk_mptcp_info add_addr_signal 2 add_addr_accepted 2
 
 		pm_nl_add_endpoint $ns1 10.0.1.1 id 99 flags signal
-		wait_mpj $ns2
+		wait_mpj 4
 		chk_subflow_nr "after re-add ID 0" 3
 		chk_mptcp_info subflows 3 subflows 3
 		chk_mptcp_info add_addr_signal 3 add_addr_accepted 2
@@ -4313,7 +4311,7 @@ endpoint_tests()
 		chk_mptcp_info add_addr_signal 2 add_addr_accepted 2
 
 		pm_nl_add_endpoint $ns1 10.0.1.1 id 88 flags signal
-		wait_mpj $ns2
+		wait_mpj 5
 		chk_subflow_nr "after re-re-add ID 0" 3
 		chk_mptcp_info subflows 3 subflows 3
 		chk_mptcp_info add_addr_signal 3 add_addr_accepted 2
@@ -4362,9 +4360,9 @@ endpoint_tests()
 		wait_rm_addr $ns2 0
 		ip netns exec "${ns2}" ${iptables} -D OUTPUT -s "10.0.3.2" -p tcp -j REJECT
 		pm_nl_add_endpoint $ns2 10.0.3.2 id 3 flags subflow
-		wait_mpj $ns2
+		wait_mpj 1
 		pm_nl_add_endpoint $ns1 10.0.3.1 id 2 flags signal
-		wait_mpj $ns2
+		wait_mpj 2
 		mptcp_lib_kill_group_wait $tests_pid
 
 		join_syn_tx=3 join_connect_err=1 \

-- 
2.51.0

[PATCH mptcp-next 07/11] selftests: mptcp: join: userspace: wait for new events

[Matthieu Baerts (NGI0)]

Instead of waiting for a random amount of time (1 second), wait for an
event to be received on the other side.

To do that, when an address is announced (userspace_pm_add_addr), the
ANNOUNCED is expected. When a new subflow is created
(userspace_pm_add_sf), the SUB_ESTABLISHED event is expected.

With this, the tests can finish quicker.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index cf28537b71f7..63a84a4d43a6 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3717,7 +3717,6 @@ userspace_pm_add_addr()
 	tk=$(mptcp_lib_evts_get_info token "$evts")
 
 	ip netns exec $1 ./pm_nl_ctl ann $2 token $tk id $3
-	sleep 1
 }
 
 # $1: ns ; $2: id
@@ -3748,7 +3747,6 @@ userspace_pm_add_sf()
 
 	ip netns exec $1 ./pm_nl_ctl csf lip $2 lid $3 \
 				rip $da rport $dp token $tk
-	sleep 1
 }
 
 # $1: ns ; $2: addr $3: event type
@@ -4000,7 +3998,9 @@ userspace_tests()
 		local tests_pid=$!
 		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_addr $ns1 10.0.2.1 10
+		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 1
 		userspace_pm_add_addr $ns1 10.0.3.1 20
+		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 2
 		chk_join_nr 2 2 2
 		chk_add_nr 2 2
 		chk_mptcp_info subflows 2 subflows 2
@@ -4033,6 +4033,7 @@ userspace_tests()
 		local tests_pid=$!
 		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_sf $ns2 10.0.3.2 20
+		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
 		chk_join_nr 1 1 1
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 2 2
@@ -4063,6 +4064,7 @@ userspace_tests()
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
 		userspace_pm_add_sf $ns2 10.0.3.2 0
+		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
 		userspace_pm_chk_dump_addr "${ns2}" \
 			"id 0 flags subflow 10.0.3.2" "id 0 subflow"
 		chk_join_nr 1 1 1
@@ -4082,6 +4084,7 @@ userspace_tests()
 		local tests_pid=$!
 		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_sf $ns2 10.0.3.2 20
+		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
 		chk_join_nr 1 1 1
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 2 2
@@ -4106,6 +4109,7 @@ userspace_tests()
 		local tests_pid=$!
 		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_addr $ns1 10.0.2.1 10
+		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 1
 		chk_join_nr 1 1 1
 		chk_add_nr 1 1
 		chk_mptcp_info subflows 1 subflows 1
@@ -4132,6 +4136,7 @@ userspace_tests()
 		local tests_pid=$!
 		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		userspace_pm_add_sf $ns2 10.0.3.2 20
+		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 2 2
 

-- 
2.51.0

[PATCH mptcp-next 08/11] selftests: mptcp: join chk_stale_nr: avoid dup stats

[Matthieu Baerts (NGI0)]

nstat outputs are already printed when calling 'fail_test', no need to
do it again.

While at it, no need to use the dump_stats variable, print the extra
stats directly. And use 'ip -n $ns' instead of 'ip netns exec $ns',
shorter and clearer.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 63a84a4d43a6..80e03168b419 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1648,7 +1648,6 @@ chk_stale_nr()
 	local stale_min=$2
 	local stale_max=$3
 	local stale_delta=$4
-	local dump_stats
 	local stale_nr
 	local recover_nr
 
@@ -1664,16 +1663,11 @@ chk_stale_nr()
 		fail_test "got $stale_nr stale[s] $recover_nr recover[s], " \
 		     " expected stale in range [$stale_min..$stale_max]," \
 		     " stale-recover delta $stale_delta"
-		dump_stats=1
+		echo $ns stats
+		ip -n $ns -s link show
 	else
 		print_ok
 	fi
-
-	if [ "${dump_stats}" = 1 ]; then
-		echo $ns stats
-		ip netns exec $ns ip -s link show
-		ip netns exec $ns nstat -as | grep MPTcp
-	fi
 }
 
 chk_add_nr()

-- 
2.51.0

[PATCH mptcp-next 09/11] selftests: mptcp: join: avoid declaring i if not used

[Matthieu Baerts (NGI0)]

A few loops were declaring 'i', but this variable was not used.

To avoid confusions, use '_' instead: it is more explicit to mark that
this variable is not needed.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 80e03168b419..4ca4bdfea246 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -603,8 +603,7 @@ wait_rm_addr()
 	local old_cnt="${2}"
 	local cnt
 
-	local i
-	for i in $(seq 10); do
+	for _ in $(seq 10); do
 		cnt=$(rm_addr_count ${ns})
 		[ "$cnt" = "${old_cnt}" ] || break
 		sleep 0.1
@@ -623,8 +622,7 @@ wait_rm_sf()
 	local old_cnt="${2}"
 	local cnt
 
-	local i
-	for i in $(seq 10); do
+	for _ in $(seq 10); do
 		cnt=$(rm_sf_count ${ns})
 		[ "$cnt" = "${old_cnt}" ] || break
 		sleep 0.1
@@ -648,8 +646,7 @@ wait_ll_ready()
 {
 	local ns="${1}"
 
-	local i
-	for i in $(seq 50); do
+	for _ in $(seq 50); do
 		ip -n "${ns}" -6 addr show scope link | grep "inet6 fe80" |
 			grep -qw "tentative" || break
 		sleep 0.1

-- 
2.51.0

[PATCH mptcp-next 10/11] selftests: mptcp: connect: fix maybe-uninitialize warn

[Matthieu Baerts (NGI0)]

This warning can be seen with GCC 15.2:

  mptcp_connect.c: In function ‘main_loop’:
  mptcp_connect.c:1422:37: warning: ‘peer’ may be used uninitialized [-Wmaybe-uninitialized]
   1422 |                 if (connect(fd, peer->ai_addr, peer->ai_addrlen))
        |                                 ~~~~^~~~~~~~~
  mptcp_connect.c:1377:26: note: ‘peer’ was declared here
   1377 |         struct addrinfo *peer;
        |                          ^~~~

This variable is set in sock_connect_mptcp() in some conditions. If not,
this helper returns an error, and the program stops. So this is a false
positive, but better remove it by initialising peer to NULL.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index b82df82e0594..27a93a25b142 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -1353,8 +1353,8 @@ void xdisconnect(int fd)
 
 int main_loop(void)
 {
+	struct addrinfo *peer = NULL;
 	int fd = 0, ret, fd_in = 0;
-	struct addrinfo *peer;
 	struct wstate winfo;
 
 	if (cfg_input && cfg_sockopt_types.mptfo) {

-- 
2.51.0

[PATCH mptcp-next 11/11] selftests: mptcp: connect cleanup TFO setup

[Matthieu Baerts (NGI0)]

To the TFO, only the file descriptor is needed, the family is not.

Also, the error can be handled the same way when 'sendto()' or
'connect()' are used. Only the printed error message is different.

This avoids a bit of confusions.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 27a93a25b142..6d2eb5cf8539 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -258,7 +258,7 @@ static void set_transparent(int fd, int pf)
 	}
 }
 
-static void set_mptfo(int fd, int pf)
+static void set_mptfo(int fd)
 {
 	int qlen = 25;
 
@@ -335,7 +335,7 @@ static int sock_listen_mptcp(const char * const listenaddr,
 			set_transparent(sock, pf);
 
 		if (cfg_sockopt_types.mptfo)
-			set_mptfo(sock, pf);
+			set_mptfo(sock);
 
 		if (bind(sock, a->ai_addr, a->ai_addrlen) == 0)
 			break; /* success */
@@ -406,21 +406,18 @@ static int sock_connect_mptcp(const char * const remoteaddr,
 				*peer = a;
 				break; /* success */
 			}
+			perror("sendto()");
 		} else {
 			if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
 				*peer = a;
 				break; /* success */
 			}
-		}
-		if (cfg_sockopt_types.mptfo) {
-			perror("sendto()");
-			close(sock);
-			sock = -1;
-		} else {
 			perror("connect()");
-			close(sock);
-			sock = -1;
 		}
+
+		/* error */
+		close(sock);
+		sock = -1;
 	}
 
 	freeaddrinfo(addr);

-- 
2.51.0

Re: [PATCH mptcp-next 00/11] mptcp: avoid dup events + error + misc.

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/20517913570

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/3f8a1954c920
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1036645


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next 08/11] selftests: mptcp: join chk_stale_nr: avoid dup stats

[Geliang Tang]

Hi Matt,

Thanks for this patch.

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> nstat outputs are already printed when calling 'fail_test', no need
> to
> do it again.
> 
> While at it, no need to use the dump_stats variable, print the extra
> stats directly. And use 'ip -n $ns' instead of 'ip netns exec $ns',
> shorter and clearer.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 63a84a4d43a6..80e03168b419 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1648,7 +1648,6 @@ chk_stale_nr()
>  	local stale_min=$2
>  	local stale_max=$3
>  	local stale_delta=$4
> -	local dump_stats
>  	local stale_nr
>  	local recover_nr
>  
> @@ -1664,16 +1663,11 @@ chk_stale_nr()
>  		fail_test "got $stale_nr stale[s] $recover_nr
> recover[s], " \
>  		     " expected stale in range
> [$stale_min..$stale_max]," \
>  		     " stale-recover delta $stale_delta"
> -		dump_stats=1
> +		echo $ns stats
> +		ip -n $ns -s link show
>  	else
>  		print_ok
>  	fi
> -
> -	if [ "${dump_stats}" = 1 ]; then
> -		echo $ns stats
> -		ip netns exec $ns ip -s link show
> -		ip netns exec $ns nstat -as | grep MPTcp

mptcp_lib_pr_nstat() is called in fail_test(), it's slightly different
from the above line:

        ip netns exec "${ns}" nstat -as | grep Tcp

So, is it necessary for us to keep the above line?

Thanks,
-Geliang

> -	fi
>  }
>  
>  chk_add_nr()

Re: [PATCH mptcp-next 05/11] selftests: mptcp: join: wait for estab event instead of MPJ

[Geliang Tang]

Hi Matt,

Thanks for this patch.

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> 'wait_mpj' was used just after having created a background
> connection,
> but before creating new subflows. So no MPJ were sent. The intention
> was
> to wait for the connection to be established, which was the same as
> doing a simple sleep with a "random" value.
> 
> Instead, wait for an "established" event. With this, the tests can
> finish quicker.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 18 ++++++++++-----
> ---
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index bf0cc59b5210..9b58d7567c18 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -4000,7 +4000,7 @@ userspace_tests()
>  		{ timeout_test=120 test_linkfail=128 speed=5 \
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
> -		wait_mpj $ns1
> +		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_addr $ns1 10.0.2.1 10
>  		userspace_pm_add_addr $ns1 10.0.3.1 20
>  		chk_join_nr 2 2 2
> @@ -4033,7 +4033,7 @@ userspace_tests()
>  		{ timeout_test=120 test_linkfail=128 speed=5 \
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
> -		wait_mpj $ns2
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_sf $ns2 10.0.3.2 20
>  		chk_join_nr 1 1 1
>  		chk_mptcp_info subflows 1 subflows 1
> @@ -4061,7 +4061,7 @@ userspace_tests()
>  		{ timeout_test=120 test_linkfail=128 speed=5 \
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
> -		wait_mpj $ns2
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		chk_mptcp_info subflows 0 subflows 0
>  		chk_subflows_total 1 1
>  		userspace_pm_add_sf $ns2 10.0.3.2 0
> @@ -4082,7 +4082,7 @@ userspace_tests()
>  		{ timeout_test=120 test_linkfail=128 speed=5 \
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
> -		wait_mpj $ns2
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_sf $ns2 10.0.3.2 20
>  		chk_join_nr 1 1 1
>  		chk_mptcp_info subflows 1 subflows 1
> @@ -4106,7 +4106,7 @@ userspace_tests()
>  		{ timeout_test=120 test_linkfail=128 speed=5 \
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
> -		wait_mpj $ns1
> +		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_addr $ns1 10.0.2.1 10
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1
> @@ -4159,6 +4159,7 @@ endpoint_tests()
>  	# userspace pm type prevents add_addr
>  	if reset "implicit EP" &&
>  	   continue_if mptcp_lib_kallsyms_has
> "subflow_rebuild_header$"; then
> +		start_events

What do you think about using reset_with_events here?

Thanks,
-Geliang

>  		pm_nl_set_limits $ns1 2 2
>  		pm_nl_set_limits $ns2 2 2
>  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
> @@ -4166,7 +4167,7 @@ endpoint_tests()
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
>  
> -		wait_mpj $ns1
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		pm_nl_check_endpoint "creation" \
>  			$ns2 10.0.2.2 id 1 flags implicit
>  		chk_mptcp_info subflows 1 subflows 1
> @@ -4180,6 +4181,7 @@ endpoint_tests()
>  		pm_nl_check_endpoint "modif is allowed" \
>  			$ns2 10.0.2.2 id 1 flags signal
>  		mptcp_lib_kill_group_wait $tests_pid
> +		kill_events_pids
>  	fi
>  
>  	if reset_with_tcp_filter "delete and re-add" ns2 10.0.3.2
> REJECT OUTPUT &&
> @@ -4193,7 +4195,7 @@ endpoint_tests()
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
>  
> -		wait_mpj $ns2
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		pm_nl_check_endpoint "creation" \
>  			$ns2 10.0.2.2 id 2 flags subflow dev ns2eth2
>  		chk_subflow_nr "before delete id 2" 2
> @@ -4271,7 +4273,7 @@ endpoint_tests()
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
>  		local tests_pid=$!
>  
> -		wait_mpj $ns2
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		pm_nl_check_endpoint "creation" \
>  			$ns1 10.0.2.1 id 1 flags signal
>  		chk_subflow_nr "before delete" 2

Re: [PATCH mptcp-next 07/11] selftests: mptcp: join: userspace: wait for new events

[Geliang Tang]

Hi Matt,

Thanks for this patch.

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> Instead of waiting for a random amount of time (1 second), wait for
> an
> event to be received on the other side.
> 
> To do that, when an address is announced (userspace_pm_add_addr), the
> ANNOUNCED is expected. When a new subflow is created
> (userspace_pm_add_sf), the SUB_ESTABLISHED event is expected.
> 
> With this, the tests can finish quicker.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index cf28537b71f7..63a84a4d43a6 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3717,7 +3717,6 @@ userspace_pm_add_addr()
>  	tk=$(mptcp_lib_evts_get_info token "$evts")
>  
>  	ip netns exec $1 ./pm_nl_ctl ann $2 token $tk id $3
> -	sleep 1
>  }
>  
>  # $1: ns ; $2: id
> @@ -3748,7 +3747,6 @@ userspace_pm_add_sf()
>  
>  	ip netns exec $1 ./pm_nl_ctl csf lip $2 lid $3 \
>  				rip $da rport $dp token $tk
> -	sleep 1
>  }
>  
>  # $1: ns ; $2: addr $3: event type
> @@ -4000,7 +3998,9 @@ userspace_tests()
>  		local tests_pid=$!
>  		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_addr $ns1 10.0.2.1 10
> +		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 1
>  		userspace_pm_add_addr $ns1 10.0.3.1 20
> +		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 2

This has been changed to wait for 2 seconds. Can it revert to waiting
for only 1 second like before?

Thanks,
-Geliang

>  		chk_join_nr 2 2 2
>  		chk_add_nr 2 2
>  		chk_mptcp_info subflows 2 subflows 2
> @@ -4033,6 +4033,7 @@ userspace_tests()
>  		local tests_pid=$!
>  		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_sf $ns2 10.0.3.2 20
> +		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
>  		chk_join_nr 1 1 1
>  		chk_mptcp_info subflows 1 subflows 1
>  		chk_subflows_total 2 2
> @@ -4063,6 +4064,7 @@ userspace_tests()
>  		chk_mptcp_info subflows 0 subflows 0
>  		chk_subflows_total 1 1
>  		userspace_pm_add_sf $ns2 10.0.3.2 0
> +		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
>  		userspace_pm_chk_dump_addr "${ns2}" \
>  			"id 0 flags subflow 10.0.3.2" "id 0 subflow"
>  		chk_join_nr 1 1 1
> @@ -4082,6 +4084,7 @@ userspace_tests()
>  		local tests_pid=$!
>  		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_sf $ns2 10.0.3.2 20
> +		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
>  		chk_join_nr 1 1 1
>  		chk_mptcp_info subflows 1 subflows 1
>  		chk_subflows_total 2 2
> @@ -4106,6 +4109,7 @@ userspace_tests()
>  		local tests_pid=$!
>  		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_addr $ns1 10.0.2.1 10
> +		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 1
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1
>  		chk_mptcp_info subflows 1 subflows 1
> @@ -4132,6 +4136,7 @@ userspace_tests()
>  		local tests_pid=$!
>  		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		userspace_pm_add_sf $ns2 10.0.3.2 20
> +		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
>  		chk_mptcp_info subflows 1 subflows 1
>  		chk_subflows_total 2 2
>  

Re: [PATCH mptcp-next 09/11] selftests: mptcp: join: avoid declaring i if not used

[Geliang Tang]

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> A few loops were declaring 'i', but this variable was not used.
> 
> To avoid confusions, use '_' instead: it is more explicit to mark
> that
> this variable is not needed.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 80e03168b419..4ca4bdfea246 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -603,8 +603,7 @@ wait_rm_addr()
>  	local old_cnt="${2}"
>  	local cnt
>  
> -	local i
> -	for i in $(seq 10); do
> +	for _ in $(seq 10); do
>  		cnt=$(rm_addr_count ${ns})
>  		[ "$cnt" = "${old_cnt}" ] || break
>  		sleep 0.1
> @@ -623,8 +622,7 @@ wait_rm_sf()
>  	local old_cnt="${2}"
>  	local cnt
>  
> -	local i
> -	for i in $(seq 10); do
> +	for _ in $(seq 10); do
>  		cnt=$(rm_sf_count ${ns})
>  		[ "$cnt" = "${old_cnt}" ] || break
>  		sleep 0.1
> @@ -648,8 +646,7 @@ wait_ll_ready()
>  {
>  	local ns="${1}"
>  
> -	local i
> -	for i in $(seq 50); do
> +	for _ in $(seq 50); do
>  		ip -n "${ns}" -6 addr show scope link | grep "inet6
> fe80" |
>  			grep -qw "tentative" || break
>  		sleep 0.1

Re: [PATCH mptcp-next 10/11] selftests: mptcp: connect: fix maybe-uninitialize warn

[Geliang Tang]

Hi Matt,

Thanks for this fix. In my previous commit "selftests: mptcp: connect:
fix uninitialized peer warning" [1], I also fixed this.

[1]
https://patchwork.kernel.org/project/mptcp/patch/43931d38bed6e47b1b68d3bdf0a13916a926d190.1763546473.git.tanggeliang@kylinos.cn/

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> This warning can be seen with GCC 15.2:
> 
>   mptcp_connect.c: In function ‘main_loop’:
>   mptcp_connect.c:1422:37: warning: ‘peer’ may be used uninitialized
> [-Wmaybe-uninitialized]
>    1422 |                 if (connect(fd, peer->ai_addr, peer-
> >ai_addrlen))
>         |                                 ~~~~^~~~~~~~~
>   mptcp_connect.c:1377:26: note: ‘peer’ was declared here
>    1377 |         struct addrinfo *peer;
>         |                          ^~~~
> 
> This variable is set in sock_connect_mptcp() in some conditions. If
> not,
> this helper returns an error, and the program stops. So this is a
> false
> positive, but better remove it by initialising peer to NULL.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

    Reviewed-by: Geliang Tang <geliang@kernel.org>

-Geliang

> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index b82df82e0594..27a93a25b142 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -1353,8 +1353,8 @@ void xdisconnect(int fd)
>  
>  int main_loop(void)
>  {
> +	struct addrinfo *peer = NULL;
>  	int fd = 0, ret, fd_in = 0;
> -	struct addrinfo *peer;
>  	struct wstate winfo;
>  
>  	if (cfg_input && cfg_sockopt_types.mptfo) {

Re: [PATCH mptcp-next 11/11] selftests: mptcp: connect cleanup TFO setup

[Geliang Tang]

Hi Matt,

Thanks for this patch.

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> To the TFO, only the file descriptor is needed, the family is not.
> 
> Also, the error can be handled the same way when 'sendto()' or
> 'connect()' are used. Only the printed error message is different.
> 
> This avoids a bit of confusions.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

LGTM!

    Reviewed-by: Geliang Tang <geliang@kernel.org>

-Geliang

> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.c | 17 +++++++------
> ----
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 27a93a25b142..6d2eb5cf8539 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -258,7 +258,7 @@ static void set_transparent(int fd, int pf)
>  	}
>  }
>  
> -static void set_mptfo(int fd, int pf)
> +static void set_mptfo(int fd)
>  {
>  	int qlen = 25;
>  
> @@ -335,7 +335,7 @@ static int sock_listen_mptcp(const char * const
> listenaddr,
>  			set_transparent(sock, pf);
>  
>  		if (cfg_sockopt_types.mptfo)
> -			set_mptfo(sock, pf);
> +			set_mptfo(sock);
>  
>  		if (bind(sock, a->ai_addr, a->ai_addrlen) == 0)
>  			break; /* success */
> @@ -406,21 +406,18 @@ static int sock_connect_mptcp(const char *
> const remoteaddr,
>  				*peer = a;
>  				break; /* success */
>  			}
> +			perror("sendto()");
>  		} else {
>  			if (connect(sock, a->ai_addr, a->ai_addrlen)
> == 0) {
>  				*peer = a;
>  				break; /* success */
>  			}
> -		}
> -		if (cfg_sockopt_types.mptfo) {
> -			perror("sendto()");
> -			close(sock);
> -			sock = -1;
> -		} else {
>  			perror("connect()");
> -			close(sock);
> -			sock = -1;
>  		}
> +
> +		/* error */
> +		close(sock);
> +		sock = -1;
>  	}
>  
>  	freeaddrinfo(addr);

Re: [PATCH mptcp-next 08/11] selftests: mptcp: join chk_stale_nr: avoid dup stats

[Geliang Tang]

On Sun, 2025-12-28 at 10:25 +0800, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for this patch.
> 
> On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> > nstat outputs are already printed when calling 'fail_test', no need
> > to
> > do it again.
> > 
> > While at it, no need to use the dump_stats variable, print the
> > extra
> > stats directly. And use 'ip -n $ns' instead of 'ip netns exec $ns',
> > shorter and clearer.
> > 
> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > ---
> >  tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index 63a84a4d43a6..80e03168b419 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -1648,7 +1648,6 @@ chk_stale_nr()
> >  	local stale_min=$2
> >  	local stale_max=$3
> >  	local stale_delta=$4
> > -	local dump_stats
> >  	local stale_nr
> >  	local recover_nr
> >  
> > @@ -1664,16 +1663,11 @@ chk_stale_nr()
> >  		fail_test "got $stale_nr stale[s] $recover_nr
> > recover[s], " \
> >  		     " expected stale in range
> > [$stale_min..$stale_max]," \
> >  		     " stale-recover delta $stale_delta"
> > -		dump_stats=1
> > +		echo $ns stats
> > +		ip -n $ns -s link show
> >  	else
> >  		print_ok
> >  	fi
> > -
> > -	if [ "${dump_stats}" = 1 ]; then
> > -		echo $ns stats
> > -		ip netns exec $ns ip -s link show
> > -		ip netns exec $ns nstat -as | grep MPTcp
> 
> mptcp_lib_pr_nstat() is called in fail_test(), it's slightly
> different
> from the above line:
> 
>         ip netns exec "${ns}" nstat -as | grep Tcp
> 
> So, is it necessary for us to keep the above line?

Sorry, I made a mistake. The output of 'grep Tcp' includes the output
of 'grep MPTcp', so the information of dump_stats is indeed redundant.

So this patch looks good to me.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> 
> Thanks,
> -Geliang
> 
> > -	fi
> >  }
> >  
> >  chk_add_nr()

Re: [PATCH mptcp-next 01/11] mptcp: avoid dup SUB_CLOSED events after disconnect

[Geliang Tang]

Hi Matt,

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> In case of subflow disconnect(), which can also happen with the first
> subflow in case of errors like timeout or reset,
> mptcp_subflow_ctx_reset
> will reset most fields from the mptcp_subflow_context structure,
> including close_event_done. Then, when another subflow is closed, yet
> another SUB_CLOSED event for the disconnected initial subflow is
> sent.
> Because of the previous reset, there are no source address and
> destination port.
> 
> A solution is then to also check the subflow's local id: it shouldn't
> be
> negative anyway.
> 
> Another solution would be not to reset subflow->close_event_done at
> disconnect time, but when reused. But then, probably the whole reset
> could be done when being reused. Let's not change this logic, similar
> to TCP with tcp_disconnect().
> 
> Fixes: d82809b6c5f2 ("mptcp: avoid duplicated SUB_CLOSED events")
> Reported-by: Marco Angaroni <marco.angaroni@italtel.com>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/603
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/protocol.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 900f26e21acd..1d68648b5194 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2622,8 +2622,8 @@ void mptcp_close_ssk(struct sock *sk, struct
> sock *ssk,
>  	struct mptcp_sock *msk = mptcp_sk(sk);
>  	struct sk_buff *skb;
>  
> -	/* The first subflow can already be closed and still in the
> list */
> -	if (subflow->close_event_done)
> +	/* The 1st subflow can already be closed/disco and still in
> the

I suggest that we avoid abbreviating it as "disco" here and instead
write out "disconnected" in full. If that makes this line of comment
too long, it can be split into two lines.

Besides this, this patch looks good to me.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> list */
> +	if (subflow->close_event_done || READ_ONCE(subflow-
> >local_id) < 0)
>  		return;
>  
>  	subflow->close_event_done = true;

Re: [PATCH mptcp-next 02/11] selftests: mptcp: check no dup close events after error

[Geliang Tang]

Hi Matt,

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> This validates the previous commit: subflow closed events are re-sent
> with less info when the initial subflow is disconnected after an
> error
> and each time a subflow is closed after that.
> 
> In this new test, the userspace PM is involved because that's how it
> was
> discovered, but it is not specific to it. The initial subflow is
> terminated with a RESET, and that will cause the subflow disconnect.
> Then, a new subflow is initiated, but also got rejected, which cause
> a
> second subflow closed event, but not a third one.
> 
> While at it, in case of failure to get the expected amount of events,
> the events are printed.
> 
> The 'Fixes' tag here below is the same as the one from the previous
> commit: this patch here is not fixing anything wrong in the
> selftests,
> but it validates the previous fix for an issue introduced by this
> commit
> ID.
> 
> Fixes: d82809b6c5f2 ("mptcp: avoid duplicated SUB_CLOSED events")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 49
> +++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index b2e6e548f796..468ffb97293b 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3872,11 +3872,32 @@ chk_evt_nr()
>  	count=$(grep -cw "type:${evt}" "${evts}")
>  	if [ "${count}" != "${exp}" ]; then
>  		fail_test "got ${count} events, expected ${exp}"
> +		cat "${evts}"
>  	else
>  		print_ok
>  	fi
>  }
>  
> +# $1: ns ; $2: event type ; $3: expected count
> +wait_event()
> +{
> +	local ns="${1}"
> +	local evt_name="${2}"
> +	local exp="${3}"
> +
> +	local evt="${!evt_name}"
> +	local evts="${evts_ns1}"
> +	local count
> +
> +	[ "${ns}" == "ns2" ] && evts="${evts_ns2}"
> +
> +	for _ in $(seq 100); do
> +		count=$(grep -cw "type:${evt}" "${evts}")
> +		[ "${count}" -ge "${exp}" ] && break
> +		sleep 0.1
> +	done
> +}
> +
>  userspace_tests()
>  {
>  	# userspace pm type prevents add_addr
> @@ -4085,6 +4106,34 @@ userspace_tests()
>  		kill_events_pids
>  		mptcp_lib_kill_group_wait $tests_pid
>  	fi
> +
> +	# userspace pm no duplicated spurious close events after an
> error
> +	if reset_with_events "userspace pm no dup close events after
> error" &&
> +	   continue_if mptcp_lib_has_file
> '/proc/sys/net/mptcp/pm_type'; then
> +		set_userspace_pm $ns2
> +		pm_nl_set_limits $ns1 0 2
> +		{ timeout_test=120 test_linkfail=128 speed=slow \
> +			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
> +		local tests_pid=$!
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
> +		userspace_pm_add_sf $ns2 10.0.3.2 20
> +		chk_mptcp_info subflows 1 subflows 1
> +		chk_subflows_total 2 2
> +
> +		# force quick loss
> +		ip netns exec $ns2 sysctl -q
> net.ipv4.tcp_syn_retries=1
> +		if ip netns exec "${ns1}" ${iptables} -A INPUT -s
> "10.0.1.2" -p tcp --tcp-option 30 -j REJECT --reject-with tcp-reset
> &&
> +		   ip netns exec "${ns2}" ${iptables} -A INPUT -d
> "10.0.1.2" -p tcp --tcp-option 30 -j REJECT --reject-with tcp-reset;
> then

These two lines are too long and need to be wrapped.

Thanks,
-Geliang

> +			wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1
> +			wait_event ns1 MPTCP_LIB_EVENT_SUB_CLOSED 1
> +			chk_subflows_total 1 1
> +			userspace_pm_add_sf $ns2 10.0.1.2 0
> +			wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2
> +			chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2
> +		fi
> +		kill_events_pids
> +		mptcp_lib_kill_group_wait $tests_pid
> +	fi
>  }
>  
>  endpoint_tests()

Re: [PATCH mptcp-next 03/11] mptcp: only reset subflow errors when propagated

[Geliang Tang]

Hi Matt,

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> Some subflow socket errors need to be reported to the MPTCP socket:
> the
> initial subflow connect (MP_CAPABLE), and the ones from the fallback
> sockets. The others are not propagated.
> 
> The issue is that sock_error() was used to retrieve the error, which
> was
> also resetting the sk_err field. Because of that, when notifying the
> userspace about subflow close events later on from the MPTCP worker,
> the
> ssk->sk_err field was always 0.
> 
> Now, the error (sk_err) is only reset when propagating it to the msk.
> 
> Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Note: I guess we could also duplicate the error in
> mptcp_subflow_context
> struct before scheduling the worker, and use this field in
> mptcp_event_put_token_and_ssk(), but I don't see why we should always
> reset ssk->sk_err in __mptcp_subflow_error_report() in all cases.
> ---
>  net/mptcp/protocol.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 1d68648b5194..5d6c987089c6 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -817,10 +817,9 @@ static bool __mptcp_ofo_queue(struct mptcp_sock
> *msk)
>  
>  static bool __mptcp_subflow_error_report(struct sock *sk, struct
> sock *ssk)
>  {
> -	int err = sock_error(ssk);
>  	int ssk_state;
>  
> -	if (!err)
> +	if (!READ_ONCE(ssk->sk_err))
>  		return false;

Your fix works. I'm wondering if swapping the order of 'if (sk-
>sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk)))'
and 'if (!err)' would be a bit simpler, i.e.:

int err = sock_error(ssk);
int ssk_state;

if (sk->sk_state != TCP_SYN_SENT &&
    !__mptcp_check_fallback(mptcp_sk(sk)))
        return false;

if (!err)
        return false;

WDYT?

Thanks,
-Geliang

>  
>  	/* only propagate errors on fallen-back sockets or
> @@ -837,7 +836,7 @@ static bool __mptcp_subflow_error_report(struct
> sock *sk, struct sock *ssk)
>  	ssk_state = inet_sk_state_load(ssk);
>  	if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD))
>  		mptcp_set_state(sk, ssk_state);
> -	WRITE_ONCE(sk->sk_err, -err);
> +	WRITE_ONCE(sk->sk_err, -sock_error(ssk));
>  
>  	/* This barrier is coupled with smp_rmb() in mptcp_poll() */
>  	smp_wmb();

Re: [PATCH mptcp-next 03/11] mptcp: only reset subflow errors when propagated

[Geliang Tang]

On Sun, 2026-01-04 at 11:30 +0800, Geliang Tang wrote:
> Hi Matt,
> 
> On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> > Some subflow socket errors need to be reported to the MPTCP socket:
> > the
> > initial subflow connect (MP_CAPABLE), and the ones from the
> > fallback
> > sockets. The others are not propagated.
> > 
> > The issue is that sock_error() was used to retrieve the error,
> > which
> > was
> > also resetting the sk_err field. Because of that, when notifying
> > the
> > userspace about subflow close events later on from the MPTCP
> > worker,
> > the
> > ssk->sk_err field was always 0.
> > 
> > Now, the error (sk_err) is only reset when propagating it to the
> > msk.
> > 
> > Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk")
> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > ---
> > Note: I guess we could also duplicate the error in
> > mptcp_subflow_context
> > struct before scheduling the worker, and use this field in
> > mptcp_event_put_token_and_ssk(), but I don't see why we should
> > always
> > reset ssk->sk_err in __mptcp_subflow_error_report() in all cases.
> > ---
> >  net/mptcp/protocol.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index 1d68648b5194..5d6c987089c6 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -817,10 +817,9 @@ static bool __mptcp_ofo_queue(struct
> > mptcp_sock
> > *msk)
> >  
> >  static bool __mptcp_subflow_error_report(struct sock *sk, struct
> > sock *ssk)
> >  {
> > -	int err = sock_error(ssk);
> >  	int ssk_state;
> >  
> > -	if (!err)
> > +	if (!READ_ONCE(ssk->sk_err))
> >  		return false;
> 
> Your fix works. I'm wondering if swapping the order of 'if (sk-
> > sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk)))'
> and 'if (!err)' would be a bit simpler, i.e.:
> 
> int err = sock_error(ssk);
> int ssk_state;
> 
> if (sk->sk_state != TCP_SYN_SENT &&
>     !__mptcp_check_fallback(mptcp_sk(sk)))
>         return false;
> 
> if (!err)
>         return false;

Sorry, I was mistaken. It should be like this:

int ssk_state;
int err;

if (sk->sk_state != TCP_SYN_SENT &&
    !__mptcp_check_fallback(mptcp_sk(sk)))
        return false;

err = sock_error(ssk);
if (!err)
        return false;

> 
> WDYT?
> 
> Thanks,
> -Geliang
> 
> >  
> >  	/* only propagate errors on fallen-back sockets or
> > @@ -837,7 +836,7 @@ static bool __mptcp_subflow_error_report(struct
> > sock *sk, struct sock *ssk)
> >  	ssk_state = inet_sk_state_load(ssk);
> >  	if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD))
> >  		mptcp_set_state(sk, ssk_state);
> > -	WRITE_ONCE(sk->sk_err, -err);
> > +	WRITE_ONCE(sk->sk_err, -sock_error(ssk));
> >  
> >  	/* This barrier is coupled with smp_rmb() in mptcp_poll()
> > */
> >  	smp_wmb();

Re: [PATCH mptcp-next 07/11] selftests: mptcp: join: userspace: wait for new events

[Matthieu Baerts]

Hi Geliang,

Thank you for the reviews!

On 28/12/2025 03:48, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for this patch.
> 
> On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
>> Instead of waiting for a random amount of time (1 second), wait for
>> an
>> event to be received on the other side.
>>
>> To do that, when an address is announced (userspace_pm_add_addr), the
>> ANNOUNCED is expected. When a new subflow is created
>> (userspace_pm_add_sf), the SUB_ESTABLISHED event is expected.
>>
>> With this, the tests can finish quicker.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> index cf28537b71f7..63a84a4d43a6 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> @@ -3717,7 +3717,6 @@ userspace_pm_add_addr()
>>  	tk=$(mptcp_lib_evts_get_info token "$evts")
>>  
>>  	ip netns exec $1 ./pm_nl_ctl ann $2 token $tk id $3
>> -	sleep 1
>>  }
>>  
>>  # $1: ns ; $2: id
>> @@ -3748,7 +3747,6 @@ userspace_pm_add_sf()
>>  
>>  	ip netns exec $1 ./pm_nl_ctl csf lip $2 lid $3 \
>>  				rip $da rport $dp token $tk
>> -	sleep 1
>>  }
>>  
>>  # $1: ns ; $2: addr $3: event type
>> @@ -4000,7 +3998,9 @@ userspace_tests()
>>  		local tests_pid=$!
>>  		wait_event ns1 MPTCP_LIB_EVENT_ESTABLISHED 1
>>  		userspace_pm_add_addr $ns1 10.0.2.1 10
>> +		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 1
>>  		userspace_pm_add_addr $ns1 10.0.3.1 20
>> +		wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 2
> 
> This has been changed to wait for 2 seconds. Can it revert to waiting
> for only 1 second like before?

The last parameter (2) is the min expected number of such events. Here,
two ADD_ADDR are expected to be seen on the other host, because
userspace_pm_add_addr has been called twice from ns1.

We could only keep the second "wait_event ANNOUNCED", maybe clearer and
faster?

  userspace_pm_add_addr $ns1 10.0.2.1 10
  userspace_pm_add_addr $ns1 10.0.3.1 20
  wait_event ns2 MPTCP_LIB_EVENT_ANNOUNCED 2

WDYT?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 10/11] selftests: mptcp: connect: fix maybe-uninitialize warn

[Matthieu Baerts]

Hi Geliang,

On 28/12/2025 03:54, Geliang Tang wrote:

> Thanks for this fix. In my previous commit "selftests: mptcp: connect:
> fix uninitialized peer warning" [1], I also fixed this.

Ah yes, sorry, I missed that patch. Why did you check if "peer" was set
in main_loop()?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.