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

Merge branch 'mptcp-subflow-accounting-fix' 

Mat Martineau says:

====================
mptcp: Subflow accounting fix

This series contains a bug fix affecting the in-kernel path manager
(patch 1), where closing subflows would sometimes not adjust the PM's
count of active subflows. Patch 2 updates the selftests to exercise the
new code.
====================

Link: https://lore.kernel.org/r/20220512232642.541301-1-mathew.j.martineau@linux.intel.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 04c494e6 e274f715

net/mptcp/pm.c

+2 −3
Original line number Diff line number Diff line
@@ -178,14 +178,13 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk, 
	struct mptcp_pm_data *pm = &msk->pm;

	bool update_subflows;



	update_subflows = (ssk->sk_state == TCP_CLOSE) &&

			  (subflow->request_join || subflow->mp_join);

	update_subflows = subflow->request_join || subflow->mp_join;

	if (!READ_ONCE(pm->work_pending) && !update_subflows)

		return;



	spin_lock_bh(&pm->lock);

	if (update_subflows)

		pm->subflows--;

		__mptcp_pm_close_subflow(msk);



	/* Even if this subflow is not really established, tell the PM to try

	 * to pick the next ones, if possible.

net/mptcp/protocol.h

+14 −0
Original line number Diff line number Diff line
@@ -833,6 +833,20 @@ unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk); 
unsigned int mptcp_pm_get_subflows_max(const struct mptcp_sock *msk);

unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk);



/* called under PM lock */

static inline void __mptcp_pm_close_subflow(struct mptcp_sock *msk)

{

	if (--msk->pm.subflows < mptcp_pm_get_subflows_max(msk))

		WRITE_ONCE(msk->pm.accept_subflow, true);

}



static inline void mptcp_pm_close_subflow(struct mptcp_sock *msk)

{

	spin_lock_bh(&msk->pm.lock);

	__mptcp_pm_close_subflow(msk);

	spin_unlock_bh(&msk->pm.lock);

}



void mptcp_sockopt_sync(struct mptcp_sock *msk, struct sock *ssk);

void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk);

net/mptcp/subflow.c

+9 −3
Original line number Diff line number Diff line
@@ -1422,20 +1422,20 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, 
	struct sockaddr_storage addr;

	int remote_id = remote->id;

	int local_id = loc->id;

	int err = -ENOTCONN;

	struct socket *sf;

	struct sock *ssk;

	u32 remote_token;

	int addrlen;

	int ifindex;

	u8 flags;

	int err;



	if (!mptcp_is_fully_established(sk))

		return -ENOTCONN;

		goto err_out;



	err = mptcp_subflow_create_socket(sk, &sf);

	if (err)

		return err;

		goto err_out;



	ssk = sf->sk;

	subflow = mptcp_subflow_ctx(ssk);
@@ -1492,6 +1492,12 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, 
failed:

	subflow->disposable = 1;

	sock_release(sf);



err_out:

	/* we account subflows before the creation, and this failures will not

	 * be caught by sk_state_change()

	 */

	mptcp_pm_close_subflow(msk);

	return err;

}

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

+46 −2
Original line number Diff line number Diff line
@@ -1444,6 +1444,33 @@ chk_prio_nr() 
	[ "${dump_stats}" = 1 ] && dump_stats

}



chk_subflow_nr()

{

	local need_title="$1"

	local msg="$2"

	local subflow_nr=$3

	local cnt1

	local cnt2



	if [ -n "${need_title}" ]; then

		printf "%03u %-36s %s" "${TEST_COUNT}" "${TEST_NAME}" "${msg}"

	else

		printf "%-${nr_blank}s %s" " " "${msg}"

	fi



	cnt1=$(ss -N $ns1 -tOni | grep -c token)

	cnt2=$(ss -N $ns2 -tOni | grep -c token)

	if [ "$cnt1" != "$subflow_nr" -o "$cnt2" != "$subflow_nr" ]; then

		echo "[fail] got $cnt1:$cnt2 subflows expected $subflow_nr"

		fail_test

		dump_stats=1

	else

		echo "[ ok ]"

	fi



	[ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni | grep token; ip -n $ns1 mptcp endpoint )

}



chk_link_usage()

{

	local ns=$1
@@ -2556,7 +2583,7 @@ fastclose_tests() 
	fi

}



implicit_tests()

endpoint_tests()

{

	# userspace pm type prevents add_addr

	if reset "implicit EP"; then
@@ -2578,6 +2605,23 @@ implicit_tests() 
			$ns2 10.0.2.2 id 1 flags signal

		wait

	fi



	if reset "delete and re-add"; then

		pm_nl_set_limits $ns1 1 1

		pm_nl_set_limits $ns2 1 1

		pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow

		run_tests $ns1 $ns2 10.0.1.1 4 0 0 slow &



		wait_mpj $ns2

		pm_nl_del_endpoint $ns2 2 10.0.2.2

		sleep 0.5

		chk_subflow_nr needtitle "after delete" 1



		pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow

		wait_mpj $ns2

		chk_subflow_nr "" "after re-add" 2

		wait

	fi

}



# [$1: error message]
@@ -2624,7 +2668,7 @@ all_tests_sorted=( 
	d@deny_join_id0_tests

	m@fullmesh_tests

	z@fastclose_tests

	I@implicit_tests

	I@endpoint_tests

)



all_tests_args=""