[v5] mptcp: add support for mixed v4/v6

[PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles

Posted by Matthieu Baerts 3 years, 1 month ago

This script is running a few tests after having setup the environment.

Printing titles helps understand what is being tested.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---

Notes:
    v3->v4:
      - Add a title for the Cleanup (+ 'Done' at the end)

 .../selftests/net/mptcp/userspace_pm.sh       | 24 ++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index ab2d581f28a1..f9a03e6e968f 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
 ns1="ns1-$rndh"
 ns2="ns2-$rndh"
 
+print_title()
+{
+	stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
+}
+
 kill_wait()
 {
 	kill $1 > /dev/null 2>&1
@@ -51,7 +56,7 @@ kill_wait()
 
 cleanup()
 {
-	echo "cleanup"
+	print_title "Cleanup"
 
 	rm -rf $file $client_evts $server_evts
 
@@ -78,6 +83,8 @@ cleanup()
 	for netns in "$ns1" "$ns2" ;do
 		ip netns del "$netns"
 	done
+
+	stdbuf -o0 -e0 printf "Done\n"
 }
 
 trap cleanup EXIT
@@ -108,6 +115,7 @@ ip -net "$ns2" addr add dead:beef:1::2/64 dev ns2eth1 nodad
 ip -net "$ns2" addr add dead:beef:2::2/64 dev ns2eth1 nodad
 ip -net "$ns2" link set ns2eth1 up
 
+print_title "Init"
 stdbuf -o0 -e0 printf "Created network namespaces ns1, ns2         \t\t\t[OK]\n"
 
 make_file()
@@ -255,6 +263,8 @@ verify_announce_event()
 
 test_announce()
 {
+	print_title "Announce tests"
+
 	# Capture events on the network namespace running the server
 	:>"$server_evts"
 
@@ -359,6 +369,8 @@ verify_remove_event()
 
 test_remove()
 {
+	print_title "Remove tests"
+
 	# Capture events on the network namespace running the server
 	:>"$server_evts"
 
@@ -521,6 +533,8 @@ verify_subflow_events()
 
 test_subflows()
 {
+	print_title "Subflows v4 or v6 only tests"
+
 	# Capture events on the network namespace running the server
 	:>"$server_evts"
 
@@ -754,6 +768,8 @@ test_subflows()
 
 test_subflows_v4_v6_mix()
 {
+	print_title "Subflows v4 and v6 mix tests"
+
 	# Attempt to add a listener at 10.0.2.1:<subflow-port>
 	ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.2.1\
 	   $app6_port > /dev/null 2>&1 &
@@ -800,6 +816,8 @@ test_subflows_v4_v6_mix()
 
 test_prio()
 {
+	print_title "Prio tests"
+
 	local count
 
 	# Send MP_PRIO signal from client to server machine
@@ -876,6 +894,8 @@ verify_listener_events()
 
 test_listener()
 {
+	print_title "Listener tests"
+
 	# Capture events on the network namespace running the client
 	:>$client_evts
 
@@ -902,8 +922,10 @@ test_listener()
 	verify_listener_events $client_evts $LISTENER_CLOSED $AF_INET 10.0.2.2 $client4_port
 }
 
+print_title "Make connections"
 make_connection
 make_connection "v6"
+
 test_announce
 test_remove
 test_subflows
-- 
2.37.2

Re: [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles

Posted by Mat Martineau 3 years, 1 month ago

On Wed, 4 Jan 2023, Matthieu Baerts wrote:

> This script is running a few tests after having setup the environment.
>
> Printing titles helps understand what is being tested.
>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>
> Notes:
>    v3->v4:
>      - Add a title for the Cleanup (+ 'Done' at the end)
>
> .../selftests/net/mptcp/userspace_pm.sh       | 24 ++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index ab2d581f28a1..f9a03e6e968f 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
> ns1="ns1-$rndh"
> ns2="ns2-$rndh"
>
> +print_title()
> +{
> +	stdbuf -o0 -e0 printf "\t%s:\n" "${1}"

The indented titles seem kind of inverted to me:

"""
 	Init:
Created network namespaces ns1, ns2         			[OK]
 	Make connections:
Established IPv4 MPTCP Connection ns2 => ns1    		[OK]
Established IPv6 MPTCP Connection ns2 => ns1    		[OK]
 	Announce tests:
ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token    		[OK]
ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port 		[OK]
...
"""

It would read better to have the titles left-justified and the test 
results indented, but that makes all of the test results extra long. Seems 
like you are trying to avoid that (for good reason!).

What do you think about copying the "INFO: ..." style from 
mptcp_connect.sh?

"""
INFO: Init
Created network namespaces ns1, ns2                             [OK]
INFO: Make connections
Established IPv4 MPTCP Connection ns2 => ns1                    [OK]
Established IPv6 MPTCP Connection ns2 => ns1                    [OK]
INFO: Announce tests
ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token                   [OK]
ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port                [OK]
...
"""

- Mat


> +}
> +
> kill_wait()
> {
> 	kill $1 > /dev/null 2>&1
> @@ -51,7 +56,7 @@ kill_wait()
>
> cleanup()
> {
> -	echo "cleanup"
> +	print_title "Cleanup"
>
> 	rm -rf $file $client_evts $server_evts
>
> @@ -78,6 +83,8 @@ cleanup()
> 	for netns in "$ns1" "$ns2" ;do
> 		ip netns del "$netns"
> 	done
> +
> +	stdbuf -o0 -e0 printf "Done\n"
> }
>
> trap cleanup EXIT
> @@ -108,6 +115,7 @@ ip -net "$ns2" addr add dead:beef:1::2/64 dev ns2eth1 nodad
> ip -net "$ns2" addr add dead:beef:2::2/64 dev ns2eth1 nodad
> ip -net "$ns2" link set ns2eth1 up
>
> +print_title "Init"
> stdbuf -o0 -e0 printf "Created network namespaces ns1, ns2         \t\t\t[OK]\n"
>
> make_file()
> @@ -255,6 +263,8 @@ verify_announce_event()
>
> test_announce()
> {
> +	print_title "Announce tests"
> +
> 	# Capture events on the network namespace running the server
> 	:>"$server_evts"
>
> @@ -359,6 +369,8 @@ verify_remove_event()
>
> test_remove()
> {
> +	print_title "Remove tests"
> +
> 	# Capture events on the network namespace running the server
> 	:>"$server_evts"
>
> @@ -521,6 +533,8 @@ verify_subflow_events()
>
> test_subflows()
> {
> +	print_title "Subflows v4 or v6 only tests"
> +
> 	# Capture events on the network namespace running the server
> 	:>"$server_evts"
>
> @@ -754,6 +768,8 @@ test_subflows()
>
> test_subflows_v4_v6_mix()
> {
> +	print_title "Subflows v4 and v6 mix tests"
> +
> 	# Attempt to add a listener at 10.0.2.1:<subflow-port>
> 	ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.2.1\
> 	   $app6_port > /dev/null 2>&1 &
> @@ -800,6 +816,8 @@ test_subflows_v4_v6_mix()
>
> test_prio()
> {
> +	print_title "Prio tests"
> +
> 	local count
>
> 	# Send MP_PRIO signal from client to server machine
> @@ -876,6 +894,8 @@ verify_listener_events()
>
> test_listener()
> {
> +	print_title "Listener tests"
> +
> 	# Capture events on the network namespace running the client
> 	:>$client_evts
>
> @@ -902,8 +922,10 @@ test_listener()
> 	verify_listener_events $client_evts $LISTENER_CLOSED $AF_INET 10.0.2.2 $client4_port
> }
>
> +print_title "Make connections"
> make_connection
> make_connection "v6"
> +
> test_announce
> test_remove
> test_subflows
> -- 
> 2.37.2
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles

Posted by Matthieu Baerts 3 years, 1 month ago

Hi Mat,

Thank you for the review!

6 Jan 2023 02:19:26 Mat Martineau <mathew.j.martineau@linux.intel.com>:

> On Wed, 4 Jan 2023, Matthieu Baerts wrote:
>
>> This script is running a few tests after having setup the environment.
>>
>> Printing titles helps understand what is being tested.
>>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>
>> Notes:
>>    v3->v4:
>>      - Add a title for the Cleanup (+ 'Done' at the end)
>>
>> .../selftests/net/mptcp/userspace_pm.sh       | 24 ++++++++++++++++++-
>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>> index ab2d581f28a1..f9a03e6e968f 100755
>> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
>> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>> @@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
>> ns1="ns1-$rndh"
>> ns2="ns2-$rndh"
>>
>> +print_title()
>> +{
>> +   stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
>
> The indented titles seem kind of inverted to me:
>
> """
>     Init:
> Created network namespaces ns1, ns2                     [OK]
>     Make connections:
> Established IPv4 MPTCP Connection ns2 => ns1            [OK]
> Established IPv6 MPTCP Connection ns2 => ns1            [OK]
>     Announce tests:
> ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token           [OK]
> ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port        [OK]
> ...
> """
>
> It would read better to have the titles left-justified and the test results indented, but that makes all of the test results extra long. Seems like you are trying to avoid that (for good reason!).

Indeed :)

> What do you think about copying the "INFO: ..." style from mptcp_connect.sh?

Good idea, that works for me!

I can do the modification (s/\t/INFO: /) when applying the patches if that's OK for you (if there is nothing else to change of course).

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

Re: [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles

Posted by Mat Martineau 3 years, 1 month ago

On Fri, 6 Jan 2023, Matthieu Baerts wrote:

> Hi Mat,
>
> Thank you for the review!
>
> 6 Jan 2023 02:19:26 Mat Martineau <mathew.j.martineau@linux.intel.com>:
>
>> On Wed, 4 Jan 2023, Matthieu Baerts wrote:
>>
>>> This script is running a few tests after having setup the environment.
>>>
>>> Printing titles helps understand what is being tested.
>>>
>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>> ---
>>>
>>> Notes:
>>>    v3->v4:
>>>      - Add a title for the Cleanup (+ 'Done' at the end)
>>>
>>> .../selftests/net/mptcp/userspace_pm.sh       | 24 ++++++++++++++++++-
>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> index ab2d581f28a1..f9a03e6e968f 100755
>>> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> @@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
>>> ns1="ns1-$rndh"
>>> ns2="ns2-$rndh"
>>>
>>> +print_title()
>>> +{
>>> +   stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
>>
>> The indented titles seem kind of inverted to me:
>>
>> """
>>     Init:
>> Created network namespaces ns1, ns2                     [OK]
>>     Make connections:
>> Established IPv4 MPTCP Connection ns2 => ns1            [OK]
>> Established IPv6 MPTCP Connection ns2 => ns1            [OK]
>>     Announce tests:
>> ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token           [OK]
>> ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port        [OK]
>> ...
>> """
>>
>> It would read better to have the titles left-justified and the test results indented, but that makes all of the test results extra long. Seems like you are trying to avoid that (for good reason!).
>
> Indeed :)
>
>> What do you think about copying the "INFO: ..." style from mptcp_connect.sh?
>
> Good idea, that works for me!
>
> I can do the modification (s/\t/INFO: /) when applying the patches if that's OK for you (if there is nothing else to change of course).
>

Sure, that's fine with me. For patches 9-12:

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel