Re: [PATCH mptcp-next v3 20/29] selftests: mptcp: add userspace pm fullmesh tests

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 25/09/2023 10:41, Geliang Tang wrote:
> This patch adds fullmesh selftests for userspace pm both on server side
> and on client side. For the server side test, add two endpoints with
> fullmesh flag on ns2, then signal an address on ns1 by userspace PM to
> trigger the fullmesh connections. For the client side test, just use
> userspace PM to create multiple subflows to do the fullmesh connections.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index aac50ef86785..d883c6c2426b 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3532,6 +3532,46 @@ userspace_tests()
>  		kill_events_pids
>  		wait $tests_pid
>  	fi
> +
> +	# userspace pm server fullmesh
> +	if reset_with_events "userspace pm server fullmesh" &&
> +	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
> +		set_userspace_pm $ns1
> +		pm_nl_set_limits $ns2 5 5
> +		pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow,fullmesh
> +		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow,fullmesh
> +		speed=10 \
> +			run_tests $ns1 $ns2 10.0.1.1 &
> +		local tests_pid=$!
> +		wait_mpj $ns1
> +		userspace_pm_add_addr $ns1 10.0.2.1 10
> +		chk_join_nr 4 4 4
> +		chk_add_nr 1 1
> +		chk_mptcp_info subflows 4 subflows 4
> +		chk_subflows_total 5 5

It is strange to me to see 5 subflows. Is it because not all endpoints
are marked as fullmesh?

Would it not be clearer to do a typical fullmesh cases using 2 endpoints
(A, B) on each side? So we would have 4 subflows: A-A, A-B, B-B, B-A.

> +		chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
> +		mptcp_lib_evts_kill
> +		wait $tests_pid
> +	fi
> +
> +	# userspace pm client fullmesh
> +	if reset_with_events "userspace pm client fullmesh" &&
> +	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
> +		pm_nl_set_limits $ns1 5 5
> +		set_userspace_pm $ns2
> +		speed=10 \
> +			run_tests $ns1 $ns2 10.0.1.1 &
> +		local tests_pid=$!
> +		wait_mpj $ns1
> +		userspace_pm_add_sf $ns2 10.0.2.2 20
> +		userspace_pm_add_sf $ns2 10.0.3.2 30
> +		userspace_pm_add_sf $ns2 10.0.4.2 40

Same here, we should mix endpoints: here it looks like you create 4
subflows from 4 different IPs, that's not really a "fullmesh": the same
endpoint should be re-used multiple times to really validate the
"fullmesh" case, e.g.

 - Client has 2 endpoints: 1.1.1.1 and 2.2.2.2
 - Server has 2 endpoints: 8.8.8.8 and 9.9.9.9

We should have 2x2 subflows:

 - 1.1.1.1 <-> 8.8.8.8
 - 1.1.1.1 <-> 9.9.9.9
 - 2.2.2.2 <-> 8.8.8.8
 - 2.2.2.2 <-> 9.9.9.9

> +		chk_join_nr 3 3 3
> +		chk_mptcp_info subflows 3 subflows 3
> +		chk_subflows_total 4 4
> +		mptcp_lib_evts_kill
> +		wait $tests_pid
> +	fi
>  }
>  
>  endpoint_tests()

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net