This patch adds the mptcp_info fields test cases, using '-D' option
to trigger it. Add a new function chk_mptcp_info() to check the
given number of the given mptcp_info field.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/330
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
v2:
 - use '-D' option instead of '-i'.
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 42e3bd1a05f5..46742453f883 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1719,6 +1719,40 @@ chk_subflow_nr()
 	fi
 }
 
+chk_mptcp_info()
+{
+	local nr_info=$1
+	local info
+	local cnt1
+	local cnt2
+	local dump_stats
+
+	if [[ "${nr_info}" = "subflows_"* ]]; then
+		info="subflows"
+		nr_info=${nr_info:9}
+	fi
+
+	printf "%-${nr_blank}s %-30s" " " "mptcp_info $info=$nr_info"
+
+	cnt1=$(ss -N $ns1 -inmHM | grep "$info:" |
+		sed -n 's/.*\('"$info"':\)\([[:digit:]]*\).*$/\2/p;q')
+	cnt2=$(ss -N $ns2 -inmHM | grep "$info:" |
+		sed -n 's/.*\('"$info"':\)\([[:digit:]]*\).*$/\2/p;q')
+	if [ "$cnt1" != "$nr_info" -o "$cnt2" != "$nr_info" ]; then
+		echo "[fail] got $cnt1:$cnt2 $info expected $nr_info"
+		fail_test
+		dump_stats=1
+	else
+		echo "[ ok ]"
+	fi
+
+	if [ "${dump_stats}" = 1 ]; then
+		ss -N $ns1 -inmHM
+		ss -N $ns2 -inmHM
+		dump_stats
+	fi
+}
+
 chk_link_usage()
 {
 	local ns=$1
@@ -3129,6 +3163,21 @@ endpoint_tests()
 	fi
 }
 
+mptcp_info_tests()
+{
+	# mptcp_info subflows
+	if reset "mptcp_info subflows"; then
+		pm_nl_set_limits $ns1 0 1
+		pm_nl_set_limits $ns2 0 1
+		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
+		run_tests $ns1 $ns2 10.0.1.1 0 0 0 speed_20 2>/dev/null &
+		wait_mpj $ns2
+		chk_join_nr 1 1 1
+		chk_mptcp_info subflows_1
+		kill_tests_wait
+	fi
+}
+
 # [$1: error message]
 usage()
 {
@@ -3177,6 +3226,7 @@ all_tests_sorted=(
 	F@fail_tests
 	u@userspace_tests
 	I@endpoint_tests
+	D@mptcp_info_tests
 )
 
 all_tests_args=""
-- 
2.35.3

Hi Geliang,

On 08/03/2023 09:06, Geliang Tang wrote:
> This patch adds the mptcp_info fields test cases, using '-D' option
> to trigger it. Add a new function chk_mptcp_info() to check the
> given number of the given mptcp_info field.

Thank you for this patch!

> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/330
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> v2:
>  - use '-D' option instead of '-i'.
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 42e3bd1a05f5..46742453f883 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1719,6 +1719,40 @@ chk_subflow_nr()
>  	fi
>  }
>  
> +chk_mptcp_info()
> +{
> +	local nr_info=$1
> +	local info
> +	local cnt1
> +	local cnt2
> +	local dump_stats
> +
> +	if [[ "${nr_info}" = "subflows_"* ]]; then

Should you add a "else" statement to fail if the condition is not met?

(you can "exit 1" directly if it is easier, it is an internal issue)

> +		info="subflows"
> +		nr_info=${nr_info:9}
> +	fi
> +
> +	printf "%-${nr_blank}s %-30s" " " "mptcp_info $info=$nr_info"
> +
> +	cnt1=$(ss -N $ns1 -inmHM | grep "$info:" |
> +		sed -n 's/.*\('"$info"':\)\([[:digit:]]*\).*$/\2/p;q')
> +	cnt2=$(ss -N $ns2 -inmHM | grep "$info:" |
> +		sed -n 's/.*\('"$info"':\)\([[:digit:]]*\).*$/\2/p;q')
> +	if [ "$cnt1" != "$nr_info" -o "$cnt2" != "$nr_info" ]; then

Just a suggestion, maybe it doesn't make sense but why not modifying
chk_subflow_nr() to do this extra check? I mean just to avoid doing
almost the same test as "delete and re-add" but looking at other counters.

Except if you plan to look at more counters from MPTCP INFO?

We can also modify chk_subflow_nr() to use chk_mptcp_info()

> +		echo "[fail] got $cnt1:$cnt2 $info expected $nr_info"
> +		fail_test
> +		dump_stats=1
> +	else
> +		echo "[ ok ]"
> +	fi
> +
> +	if [ "${dump_stats}" = 1 ]; then
> +		ss -N $ns1 -inmHM
> +		ss -N $ns2 -inmHM
> +		dump_stats
> +	fi
> +}
> +
>  chk_link_usage()
>  {
>  	local ns=$1
> @@ -3129,6 +3163,21 @@ endpoint_tests()
>  	fi
>  }
>  
> +mptcp_info_tests()
> +{
> +	# mptcp_info subflows
> +	if reset "mptcp_info subflows"; then
> +		pm_nl_set_limits $ns1 0 1
> +		pm_nl_set_limits $ns2 0 1
> +		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
> +		run_tests $ns1 $ns2 10.0.1.1 0 0 0 speed_20 2>/dev/null &
> +		wait_mpj $ns2
> +		chk_join_nr 1 1 1
> +		chk_mptcp_info subflows_1

It could be good to delete the endpoint and check the counters after to
make sure we have 0.

Or re-use "delete and re-add" test as discussed above where the counters
are checked before and after re-adding the endpoint. If you do that, it
might be good to also check counters at the beginning, before deleting
the endpoint.

> +		kill_tests_wait
> +	fi

Do you think we could have a similar test where the userspace pm is
used? (in this case, that will be part of "mptcp: update userspace pm
mptcp_info fields" v4 series)

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