Re: [PATCH mptcp-next v15 4/7] selftests: mptcp: userspace pm remove initial subflow

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 12/10/2023 10:52, Geliang Tang wrote:
> This patch adds a selftest for userpsace PM to remove the initial
> subflow. Use userspace_pm_add_sf() to add a subflow, and pass initial
> ip address to userspace_pm_rm_sf() to remove the initial subflow.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index ab6908b7b143..154978481409 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3507,6 +3507,32 @@ userspace_tests()
>  		kill_events_pids
>  		wait $tests_pid
>  	fi
> +
> +	# userspace pm remove initial subflow
> +	if reset_with_events "userspace pm remove initial subflow" &&
> +	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
> +		set_userspace_pm $ns2
> +		pm_nl_set_limits $ns1 0 1
> +		speed=10 \
> +			run_tests $ns1 $ns2 10.0.1.1 &
> +		local tests_pid=$!
> +		wait_mpj $ns2
> +		userspace_pm_add_sf $ns2 10.0.3.2 20
> +		chk_join_nr 1 1 1
> +		chk_mptcp_info subflows 1 subflows 1
> +		chk_subflows_total 2 2
> +		userspace_pm_rm_sf $ns2 10.0.1.2
> +		# we don't look at the counter linked to the RM_ADDR but
> +		# to the one linked to the subflows that have been removed
> +		chk_rm_nr 0 1
> +		chk_rst_nr 0 0 invert
> +		chk_mptcp_info subflows 1 subflows 1
> +		# No subflow removed in ns1, one
> +		# subflow has been removed in ns2
> +		chk_subflows_total 2 1

We should really have "1 1" here: on both the client and the server
side, the initial subflow is being closed, so we should only have one
counter. Same comment on patch 7/7.

Maybe my suggestion from patch 3/7 will fix this issue.

Also, how are you checking this locally: with and without SS supporting
'subflows_total' counter?

Please note that the CI complained about these two tests (and the 2
others I mentioned before with 'subflows_total'). I suspect that you
also need to modify "chk_subflows_total()" and use 'state established'
filter for ss:

  ss -N (...) -ti state established

No?

Cheers,
Matt

> +		kill_events_pids
> +		wait $tests_pid
> +	fi
>  }
>  
>  endpoint_tests()

-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net