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.