Re: [PATCH mptcp-next v8 12/12] selftests: mptcp: add path_manager sysctl test

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 04/03/2025 12:40, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a new helper set_path_manager() to set the newly added
> net.mptcp.path_manager, and test it inside a userspace pm test.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 34 ++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index fa1929ca6ad4..3b43b42f9abe 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -91,6 +91,19 @@ CBPF_MPTCP_SUBOPTION_ADD_ADDR="14,
>  			       6 0 0 65535,
>  			       6 0 0 0"
>  
> +set_path_manager()
> +{
> +	local ns=$1
> +	local pm=$2
> +
> +	if ! ip netns exec ${ns} sysctl net.mptcp.available_path_managers |
> +	     grep -wq "${pm}"; then
> +		mptcp_lib_pr_fail "path manager ${pm} not found"
> +		return 1
> +	fi
> +	ip netns exec ${ns} sysctl -q net.mptcp.path_manager="${pm}"
> +}
> +
>  init_partial()
>  {
>  	capout=$(mktemp)
> @@ -3619,7 +3632,26 @@ userspace_tests()
>  	# userspace pm create destroy subflow
>  	if reset_with_events "userspace pm create destroy subflow" &&
>  	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
> -		set_userspace_pm $ns2
> +		if continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/path_manager'; then

Same here, we cannot have continue_if here.

> +			local pm1 pm2
> +
> +			set_path_manager $ns1 "kernel"
> +			set_path_manager $ns2 "userspace"
> +
> +			pm1=$(ip netns exec ${ns1} sysctl -n net.mptcp.pm_type)
> +			if [ "$pm1" != "0" ]; then
> +				mptcp_lib_pr_fail "ns1 pm_type mapping fails"
> +				return 1
> +			fi
> +
> +			pm2=$(ip netns exec ${ns2} sysctl -n net.mptcp.pm_type)
> +			if [ "$pm2" != "1" ]; then
> +				mptcp_lib_pr_fail "ns2 pm_type mapping fails"
> +				return 1
> +			fi

I think it would be better to do that in userspace_pm.sh as suggested in
my review from patch 11/12. So replacing patches 11 and 12 by this one. No?

> +		else
> +			set_userspace_pm $ns2
> +		fi
>  		pm_nl_set_limits $ns1 0 1
>  		{ speed=5 \
>  			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null

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