Commit df8b9be3 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'mptcp-avoid-dup-nl-events-and-propagate-error' 

Matthieu Baerts says:

====================
mptcp: avoid dup NL events and propagate error

Here are two fixes affecting the MPTCP Netlink events with their tests:

- Patches 1 & 2: a subflow closed NL event was visible multiple times in
  some specific conditions. A fix for v5.12.

- Patches 3 & 4: subflow closed NL events never contained the error
  code, even when expected. A fix for v5.11.

Plus an extra fix:

- Patch 5: fix a false positive with the "signal addresses race test"
  subtest when validating the MPTCP Join selftest on a v5.15.y stable
  kernel.
====================

Link: https://patch.msgid.link/20260127-net-mptcp-dup-nl-events-v1-0-7f71e1bc4feb@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 95f82b2b c5d5ecf2

net/mptcp/protocol.c

+7 −6
Original line number Diff line number Diff line
@@ -821,11 +821,8 @@ 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)

		return false;

	int err;



	/* only propagate errors on fallen-back sockets or

	 * on MPC connect
@@ -833,6 +830,10 @@ static bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk) 
	if (sk->sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk)))

		return false;



	err = sock_error(ssk);

	if (!err)

		return false;



	/* We need to propagate only transition to CLOSE state.

	 * Orphaned socket will see such state change via

	 * subflow_sched_work_if_closed() and that path will properly
@@ -2598,8 +2599,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 first subflow can already be closed or disconnected */

	if (subflow->close_event_done || READ_ONCE(subflow->local_id) < 0)

		return;



	subflow->close_event_done = true;

tools/testing/selftests/net/mptcp/mptcp_join.sh

+74 −7
Original line number Diff line number Diff line
@@ -2329,17 +2329,16 @@ signal_address_tests() 
		ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1

		speed=slow \

			run_tests $ns1 $ns2 10.0.1.1

		chk_join_nr 3 3 3



		# It is not directly linked to the commit introducing this

		# symbol but for the parent one which is linked anyway.

		if ! mptcp_lib_kallsyms_has "mptcp_pm_subflow_check_next$"; then

			chk_join_nr 3 3 2

			chk_add_nr 4 4

		else

			chk_join_nr 3 3 3

		if mptcp_lib_kallsyms_has "mptcp_pm_subflow_check_next$"; then

			# the server will not signal the address terminating

			# the MPC subflow

			chk_add_nr 3 3

		else

			chk_add_nr 4 4

		fi

	fi

}
@@ -3847,21 +3846,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
@@ -3872,11 +3878,42 @@ chk_evt_nr() 
	count=$(grep -cw "type:${evt}" "${evts}")

	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

}



# $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 +4122,36 @@ 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 error 2

		fi

		kill_events_pids

		mptcp_lib_kill_group_wait $tests_pid

	fi

}



endpoint_tests()