1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v7 0/7] mptcp: update userspace pm mptcp_info fields
  4. View patch

[PATCH mptcp-next v7 7/7] selftests: mptcp: check userspace mptcp_info

Geliang Tang posted 7 patches 2 years, 8 months ago
Maintainers: Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
[PATCH mptcp-next v7 7/7] selftests: mptcp: check userspace mptcp_info
Posted by Geliang Tang 2 years, 8 months ago 
This patch invokes chk_mptcp_info() to check mptcp_info of userspace PM.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index fafd19ec7e1f..bc47b99f47e5 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -842,8 +842,20 @@ do_transfer()
 				tk=$(grep "type:1," "$evts_ns1" |
 				     sed -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q')
 				ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id
+				chk_mptcp_info subflows_1
 				sleep 1
 				ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id
+				addr="::ffff:$addr"
+				sp=$(grep "type:10" "$evts_ns1" |
+				     sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
+				da=$(grep "type:10" "$evts_ns1" |
+				     sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
+				dp=$(grep "type:10" "$evts_ns1" |
+				     sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q')
+				ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip $addr lport $sp \
+									rip $da rport $dp token $tk
+				sleep 1
+				chk_mptcp_info subflows_0
 			fi
 
 			counter=$((counter + 1))
@@ -906,11 +918,15 @@ do_transfer()
 				dp=$(sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q' "$evts_ns2")
 				ip netns exec ${connector_ns} ./pm_nl_ctl csf lip $addr lid $id \
 									rip $da rport $dp token $tk
+				chk_mptcp_info subflows_1
 				sleep 1
 				sp=$(grep "type:10" "$evts_ns2" |
 				     sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
+				ip netns exec ${connector_ns} ./pm_nl_ctl rem token $tk id $id
 				ip netns exec ${connector_ns} ./pm_nl_ctl dsf lip $addr lport $sp \
 									rip $da rport $dp token $tk
+				sleep 1
+				chk_mptcp_info subflows_0
 			fi
 			counter=$((counter + 1))
 			add_nr_ns2=$((add_nr_ns2 - 1))
@@ -3123,7 +3139,7 @@ userspace_tests()
 		pm_nl_set_limits $ns1 0 1
 		run_tests $ns1 $ns2 10.0.1.1 0 0 userspace_1 slow
 		chk_join_nr 1 1 1
-		chk_rm_nr 0 1
+		chk_rm_nr 1 1
 		kill_events_pids
 	fi
 }
@@ -3148,6 +3164,10 @@ endpoint_tests()
 		pm_nl_add_endpoint $ns2 10.0.2.2 flags signal
 		pm_nl_check_endpoint 0 "modif is allowed" \
 			$ns2 10.0.2.2 id 1 flags signal
+
+		chk_mptcp_info subflows_1
+		pm_nl_del_endpoint $ns2 1 10.0.2.2
+		chk_mptcp_info subflows_0
 		kill_tests_wait
 	fi
 
-- 
2.35.3
Re: [PATCH mptcp-next v7 7/7] selftests: mptcp: check userspace mptcp_info
Posted by Matthieu Baerts 2 years, 8 months ago 
Hi Geliang,

I think you missed my comments I sent on v5. I re-added them here +
additional ones.

On 14/04/2023 11:11, Geliang Tang wrote:
> This patch invokes chk_mptcp_info() to check mptcp_info of userspace PM.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index fafd19ec7e1f..bc47b99f47e5 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -842,8 +842,20 @@ do_transfer()
>  				tk=$(grep "type:1," "$evts_ns1" |
>  				     sed -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q')
>  				ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id
> +				chk_mptcp_info subflows_1

Probably best to do the check after the sleep here just below.

Also, this will print a message before displaying the title of the test,
can you fix that please? → maybe do the checks in endpoint_tests()?

(I'm not a big fan of this sleep: it might take longer on a very
slow/busy environment making the test unstable (or way shorter and we
wait for nothing) → but it was already there before so OK to keep it)

>  				sleep 1
>  				ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id
> +				addr="::ffff:$addr"
> +				sp=$(grep "type:10" "$evts_ns1" |
> +				     sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
> +				da=$(grep "type:10" "$evts_ns1" |
> +				     sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
> +				dp=$(grep "type:10" "$evts_ns1" |
> +				     sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q')
> +				ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip $addr lport $sp \
> +									rip $da rport $dp token $tk

See my comment on patch 2/7, this should be part of patch 2/7 if you
don't mind.

Also, on slow environments, could we not have a situation where the
REMOVE_ADDR signal is sent to the other peer, it directly reacts by
removing the subflow and at the end, this "dsf" command is not doing
anything, causing the test to fail because it expects the listener side
to delete the address.

Should you not first delete the subflow, then send the remove addr? (or
skip the deletion of the subflow command and expect the client to remove
the subflow?)

> +				sleep 1
> +				chk_mptcp_info subflows_0
>  			fi
>  
>  			counter=$((counter + 1))
> @@ -906,11 +918,15 @@ do_transfer()
>  				dp=$(sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q' "$evts_ns2")
>  				ip netns exec ${connector_ns} ./pm_nl_ctl csf lip $addr lid $id \
>  									rip $da rport $dp token $tk
> +				chk_mptcp_info subflows_1

Here as well: it would be better to do that after the sleep here below
and the message will be printed before the title. → maybe do the checks
in endpoint_tests()?

>  				sleep 1
>  				sp=$(grep "type:10" "$evts_ns2" |
>  				     sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
> +				ip netns exec ${connector_ns} ./pm_nl_ctl rem token $tk id $id

I think this new line and the switch from "chk_rm_nr 0 1" to "chk_rm_nr
1 1" here below should be part of a dedicated commit and mention it is
linked to patch 4/7 ("mptcp: add addr into userspace pm list"). It might
even make sense to add this new dedicated commit just after this patch
4/7, no?

>  				ip netns exec ${connector_ns} ./pm_nl_ctl dsf lip $addr lport $sp \
>  									rip $da rport $dp token $tk
> +				sleep 1
> +				chk_mptcp_info subflows_0
>  			fi
>  			counter=$((counter + 1))
>  			add_nr_ns2=$((add_nr_ns2 - 1))
> @@ -3123,7 +3139,7 @@ userspace_tests()
>  		pm_nl_set_limits $ns1 0 1
>  		run_tests $ns1 $ns2 10.0.1.1 0 0 userspace_1 slow
>  		chk_join_nr 1 1 1
> -		chk_rm_nr 0 1
> +		chk_rm_nr 1 1

(see above: to this can go to a dedicated commit)

>  		kill_events_pids
>  	fi
>  }
> @@ -3148,6 +3164,10 @@ endpoint_tests()
>  		pm_nl_add_endpoint $ns2 10.0.2.2 flags signal
>  		pm_nl_check_endpoint 0 "modif is allowed" \
>  			$ns2 10.0.2.2 id 1 flags signal
> +
> +		chk_mptcp_info subflows_1

It might make more sense to add "chk_mptcp_info subflows_1" after each
call to pm_nl_check_endpoint here above, no?

> +		pm_nl_del_endpoint $ns2 1 10.0.2.2

You need to add at least have "sleep 0.5" here I think.
Or better, use:

  wait_rm_addr ${ns2} 1

> +		chk_mptcp_info subflows_0


>  		kill_tests_wait
>  	fi
>  

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.