[PATCH mptcp-next 0/4] mptcp: run mptcp_sockopt from a new netns + cleanup

[PATCH mptcp-next 0/4] mptcp: run mptcp_sockopt from a new netns + cleanup

[Matthieu Baerts]

Here are some small fixes for the MPTCP selftests, no big changes.

The first patch is a fix and for -net: if we run mptcp_sockopt.sh after having
modified some settings related to MPTCP on the host, we can run into issues
without this patch.

The second patch uses the new netns for mptcp_inq, cleaner.

The third one remove some defined but unused vars.

The last one uniform 'rndh' variable because it was declared with undefined
variables in a few places.

Matthieu Baerts (4):
  selftests: mptcp: run mptcp_sockopt from a new netns
  selftests: mptcp: run mptcp_inq from a clean netns
  selftests: mptcp: removed defined but unused vars
  selftests: mptcp: uniform 'rndh' variable

 tools/testing/selftests/net/mptcp/diag.sh       |  1 +
 .../selftests/net/mptcp/mptcp_connect.sh        |  6 +-----
 tools/testing/selftests/net/mptcp/mptcp_join.sh |  5 +++--
 .../selftests/net/mptcp/mptcp_sockopt.sh        | 17 ++++++++---------
 .../testing/selftests/net/mptcp/simult_flows.sh |  4 +---
 .../testing/selftests/net/mptcp/userspace_pm.sh |  2 +-
 6 files changed, 15 insertions(+), 20 deletions(-)


base-commit: 1907372cfe4315a0491ae262ac23b1b4cd9cdf94
-- 
2.37.2

[PATCH mptcp-net 1/4] selftests: mptcp: run mptcp_sockopt from a new netns

[Matthieu Baerts]

Not running it from a new netns causes issues if some MPTCP settings are
modified, e.g. if MPTCP is disabled from the sysctl knob, if multiple
addresses are available and added to the MPTCP path-manager, etc.

In these cases, the created connection will not behave as expected, e.g.
unable to create an MPTCP socket, more than one subflow is seen, etc.

A new "sandbox" net namespace is now created and used to run
mptcp_sockopt from this controlled environment.

Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 0879da915014..80d36f7cfee8 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -35,8 +35,9 @@ init()
 
 	ns1="ns1-$rndh"
 	ns2="ns2-$rndh"
+	ns_sbox="ns_sbox-$rndh"
 
-	for netns in "$ns1" "$ns2";do
+	for netns in "$ns1" "$ns2" "$ns_sbox";do
 		ip netns add $netns || exit $ksft_skip
 		ip -net $netns link set lo up
 		ip netns exec $netns sysctl -q net.mptcp.enabled=1
@@ -73,7 +74,7 @@ init()
 
 cleanup()
 {
-	for netns in "$ns1" "$ns2"; do
+	for netns in "$ns1" "$ns2" "$ns_sbox"; do
 		ip netns del $netns
 	done
 	rm -f "$cin" "$cout"
@@ -243,7 +244,7 @@ do_mptcp_sockopt_tests()
 {
 	local lret=0
 
-	./mptcp_sockopt
+	ip netns exec "$ns_sbox" ./mptcp_sockopt
 	lret=$?
 
 	if [ $lret -ne 0 ]; then
@@ -252,7 +253,7 @@ do_mptcp_sockopt_tests()
 		return
 	fi
 
-	./mptcp_sockopt -6
+	ip netns exec "$ns_sbox" ./mptcp_sockopt -6
 	lret=$?
 
 	if [ $lret -ne 0 ]; then
-- 
2.37.2

[PATCH mptcp-next 2/4] selftests: mptcp: run mptcp_inq from a clean netns

[Matthieu Baerts]

A new "sandbox" net namespace is available where no other netfilter
rules have been added.

Use this new netns instead of re-using "ns1" and clean it.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 80d36f7cfee8..8a83100b212e 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -282,7 +282,7 @@ run_tests()
 
 do_tcpinq_test()
 {
-	ip netns exec "$ns1" ./mptcp_inq "$@"
+	ip netns exec "$ns_sbox" ./mptcp_inq "$@"
 	lret=$?
 	if [ $lret -ne 0 ];then
 		ret=$lret
@@ -298,9 +298,6 @@ do_tcpinq_tests()
 {
 	local lret=0
 
-	ip netns exec "$ns1" iptables -F
-	ip netns exec "$ns1" ip6tables -F
-
 	for args in "-t tcp" "-r tcp"; do
 		do_tcpinq_test $args
 		lret=$?
-- 
2.37.2

[PATCH mptcp-next 3/4] selftests: mptcp: removed defined but unused vars

[Matthieu Baerts]

Some variables were set but never used.

This was not causing any issues except adding some confusion and having
shellcheck complaining about them.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 3 ---
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 1 -
 tools/testing/selftests/net/mptcp/simult_flows.sh  | 3 ---
 3 files changed, 7 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 621af6895f4d..5942a16736a9 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -764,7 +764,6 @@ run_tests_peekmode()
 
 run_tests_disconnect()
 {
-	local peekmode="$1"
 	local old_cin=$cin
 	local old_sin=$sin
 
@@ -772,7 +771,6 @@ run_tests_disconnect()
 
 	# force do_transfer to cope with the multiple tranmissions
 	sin="$cin.disconnect"
-	sin_disconnect=$old_sin
 	cin="$cin.disconnect"
 	cin_disconnect="$old_cin"
 	connect_per_transfer=3
@@ -783,7 +781,6 @@ run_tests_disconnect()
 
 	# restore previous status
 	sin=$old_sin
-	sin_disconnect="$cout".disconnect
 	cin=$old_cin
 	cin_disconnect="$cin".disconnect
 	connect_per_transfer=1
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 8a83100b212e..6e8f4599cc44 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -10,7 +10,6 @@ ksft_skip=4
 timeout_poll=30
 timeout_test=$((timeout_poll * 2 + 1))
 mptcp_connect=""
-do_all_tests=1
 
 add_mark_rules()
 {
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index ffa13a957a36..8ae7e0c004fa 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -148,9 +148,6 @@ do_transfer()
 	:> "$sout"
 	:> "$capout"
 
-	local addr_port
-	addr_port=$(printf "%s:%d" ${connect_addr} ${port})
-
 	if $capture; then
 		local capuser
 		if [ -z $SUDO_USER ] ; then
-- 
2.37.2

[PATCH mptcp-next 4/4] selftests: mptcp: uniform 'rndh' variable

[Matthieu Baerts]

The definition of 'rndh' was probably copied from one script to another
but some times, 'sec' was not defined, not used and/or not spelled
properly.

Here all the 'rndh' are now defined the same way.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/diag.sh          | 1 +
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 3 +--
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 5 +++--
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 2 ++
 tools/testing/selftests/net/mptcp/simult_flows.sh  | 1 +
 tools/testing/selftests/net/mptcp/userspace_pm.sh  | 2 +-
 6 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 515859a5168b..24bcd7b9bdb2 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+sec=$(date +%s)
 rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
 ns="ns1-$rndh"
 ksft_skip=4
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 5942a16736a9..800adb4467f2 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -274,8 +274,7 @@ check_transfer()
 
 check_mptcp_disabled()
 {
-	local disabled_ns
-	disabled_ns="ns_disabled-$sech-$(mktemp -u XXXXXX)"
+	local disabled_ns="ns_disabled-$rndh"
 	ip netns add ${disabled_ns} || exit $ksft_skip
 
 	# net.mptcp.enabled should be enabled by default
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f3dd5f2a0272..53ecc8d4dc8a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -59,8 +59,9 @@ init_partial()
 {
 	capout=$(mktemp)
 
-	local rndh
-	rndh=$(mktemp -u XXXXXX)
+	local sec rndh
+	sec=$(date +%s)
+	rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
 
 	ns1="ns1-$rndh"
 	ns2="ns2-$rndh"
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 6e8f4599cc44..dbee386450f3 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -30,6 +30,8 @@ add_mark_rules()
 
 init()
 {
+	local sec rndh
+	sec=$(date +%s)
 	rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
 
 	ns1="ns1-$rndh"
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 8ae7e0c004fa..0915fb39afc3 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+sec=$(date +%s)
 rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
 ns1="ns1-$rndh"
 ns2="ns2-$rndh"
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 3229725b64b0..5dfc3ee74b98 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -33,7 +33,7 @@ client_addr_id=${RANDOM:0:2}
 server_addr_id=${RANDOM:0:2}
 
 sec=$(date +%s)
-rndh=$(stdbuf -o0 -e0 printf %x "$sec")-$(mktemp -u XXXXXX)
+rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
 ns1="ns1-$rndh"
 ns2="ns2-$rndh"
 
-- 
2.37.2

Re: selftests: mptcp: uniform 'rndh' variable: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4915441154916352
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4915441154916352/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/6041341061758976
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6041341061758976/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e2fd62b1a03e


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-net 1/4] selftests: mptcp: run mptcp_sockopt from a new netns

[Geliang Tang]

Matthieu Baerts <matthieu.baerts@tessares.net> 于2022年10月20日周四 00:20写道：
>
> Not running it from a new netns causes issues if some MPTCP settings are
> modified, e.g. if MPTCP is disabled from the sysctl knob, if multiple
> addresses are available and added to the MPTCP path-manager, etc.
>
> In these cases, the created connection will not behave as expected, e.g.
> unable to create an MPTCP socket, more than one subflow is seen, etc.
>
> A new "sandbox" net namespace is now created and used to run
> mptcp_sockopt from this controlled environment.
>
> Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index 0879da915014..80d36f7cfee8 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -35,8 +35,9 @@ init()
>
>         ns1="ns1-$rndh"
>         ns2="ns2-$rndh"
> +       ns_sbox="ns_sbox-$rndh"

Hi Matt, how about clearly defining ns_sbox (as well as ns1, ns2) as a
global variable at the beginning of this file, and then assign values
here, as the same as in mptcp_connect.sh and mptcp_join.sh?

Thanks,
-Geliang

>
> -       for netns in "$ns1" "$ns2";do
> +       for netns in "$ns1" "$ns2" "$ns_sbox";do
>                 ip netns add $netns || exit $ksft_skip
>                 ip -net $netns link set lo up
>                 ip netns exec $netns sysctl -q net.mptcp.enabled=1
> @@ -73,7 +74,7 @@ init()
>
>  cleanup()
>  {
> -       for netns in "$ns1" "$ns2"; do
> +       for netns in "$ns1" "$ns2" "$ns_sbox"; do
>                 ip netns del $netns
>         done
>         rm -f "$cin" "$cout"
> @@ -243,7 +244,7 @@ do_mptcp_sockopt_tests()
>  {
>         local lret=0
>
> -       ./mptcp_sockopt
> +       ip netns exec "$ns_sbox" ./mptcp_sockopt
>         lret=$?
>
>         if [ $lret -ne 0 ]; then
> @@ -252,7 +253,7 @@ do_mptcp_sockopt_tests()
>                 return
>         fi
>
> -       ./mptcp_sockopt -6
> +       ip netns exec "$ns_sbox" ./mptcp_sockopt -6
>         lret=$?
>
>         if [ $lret -ne 0 ]; then
> --
> 2.37.2
>
>

Re: [PATCH mptcp-next 0/4] mptcp: run mptcp_sockopt from a new netns + cleanup

[Mat Martineau]

On Wed, 19 Oct 2022, Matthieu Baerts wrote:

> Here are some small fixes for the MPTCP selftests, no big changes.
>
> The first patch is a fix and for -net: if we run mptcp_sockopt.sh after having
> modified some settings related to MPTCP on the host, we can run into issues
> without this patch.
>

Hi Matthieu -

The changes are looking good to me, and Geliang's suggestion on patch 1 
seems worthwhile for consistency. Can you post a v2 with the suggested 
global ns variables?


Thanks,

Mat


> The second patch uses the new netns for mptcp_inq, cleaner.
>
> The third one remove some defined but unused vars.
>
> The last one uniform 'rndh' variable because it was declared with undefined
> variables in a few places.
>
> Matthieu Baerts (4):
>  selftests: mptcp: run mptcp_sockopt from a new netns
>  selftests: mptcp: run mptcp_inq from a clean netns
>  selftests: mptcp: removed defined but unused vars
>  selftests: mptcp: uniform 'rndh' variable
>
> tools/testing/selftests/net/mptcp/diag.sh       |  1 +
> .../selftests/net/mptcp/mptcp_connect.sh        |  6 +-----
> tools/testing/selftests/net/mptcp/mptcp_join.sh |  5 +++--
> .../selftests/net/mptcp/mptcp_sockopt.sh        | 17 ++++++++---------
> .../testing/selftests/net/mptcp/simult_flows.sh |  4 +---
> .../testing/selftests/net/mptcp/userspace_pm.sh |  2 +-
> 6 files changed, 15 insertions(+), 20 deletions(-)
>
>
> base-commit: 1907372cfe4315a0491ae262ac23b1b4cd9cdf94
> -- 
> 2.37.2
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-net 1/4] selftests: mptcp: run mptcp_sockopt from a new netns

[Matthieu Baerts]

Hi Geliang, Mat,

On 23/10/2022 11:28, Geliang Tang wrote:
> Matthieu Baerts <matthieu.baerts@tessares.net> 于2022年10月20日周四 00:20写道：
>>
>> Not running it from a new netns causes issues if some MPTCP settings are
>> modified, e.g. if MPTCP is disabled from the sysctl knob, if multiple
>> addresses are available and added to the MPTCP path-manager, etc.
>>
>> In these cases, the created connection will not behave as expected, e.g.
>> unable to create an MPTCP socket, more than one subflow is seen, etc.
>>
>> A new "sandbox" net namespace is now created and used to run
>> mptcp_sockopt from this controlled environment.
>>
>> Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>> index 0879da915014..80d36f7cfee8 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>> @@ -35,8 +35,9 @@ init()
>>
>>         ns1="ns1-$rndh"
>>         ns2="ns2-$rndh"
>> +       ns_sbox="ns_sbox-$rndh"
> 
> Hi Matt, how about clearly defining ns_sbox (as well as ns1, ns2) as a
> global variable at the beginning of this file, and then assign values
> here, as the same as in mptcp_connect.sh and mptcp_join.sh?

Thank you for the idea, it would be clearer.

@Mat: Because this patch is for -net, would it not be better to do that
in an additional patch for net-next? Patch 2/4 is also for net-next and
depends on patch 1/4.

Or maybe it is OK to do that in -net because it is in the selftests? I
would prefer to split but I'm fine to do that if we prefer.

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

Re: [PATCH mptcp-net 1/4] selftests: mptcp: run mptcp_sockopt from a new netns

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

On Mon, 31 Oct 2022, Matthieu Baerts wrote:

> Hi Geliang, Mat,
>
> On 23/10/2022 11:28, Geliang Tang wrote:
>> Matthieu Baerts <matthieu.baerts@tessares.net> 于2022年10月20日周四 00:20写道：
>>>
>>> Not running it from a new netns causes issues if some MPTCP settings are
>>> modified, e.g. if MPTCP is disabled from the sysctl knob, if multiple
>>> addresses are available and added to the MPTCP path-manager, etc.
>>>
>>> In these cases, the created connection will not behave as expected, e.g.
>>> unable to create an MPTCP socket, more than one subflow is seen, etc.
>>>
>>> A new "sandbox" net namespace is now created and used to run
>>> mptcp_sockopt from this controlled environment.
>>>
>>> Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>> ---
>>>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>> index 0879da915014..80d36f7cfee8 100755
>>> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>> @@ -35,8 +35,9 @@ init()
>>>
>>>         ns1="ns1-$rndh"
>>>         ns2="ns2-$rndh"
>>> +       ns_sbox="ns_sbox-$rndh"
>>
>> Hi Matt, how about clearly defining ns_sbox (as well as ns1, ns2) as a
>> global variable at the beginning of this file, and then assign values
>> here, as the same as in mptcp_connect.sh and mptcp_join.sh?
>
> Thank you for the idea, it would be clearer.
>
> @Mat: Because this patch is for -net, would it not be better to do that
> in an additional patch for net-next? Patch 2/4 is also for net-next and
> depends on patch 1/4.
>
> Or maybe it is OK to do that in -net because it is in the selftests? I
> would prefer to split but I'm fine to do that if we prefer.

Oh - I missed that the series was a mix of net and net-next!

Sure, fine to split it.

--
Mat Martineau
Intel

Re: [PATCH mptcp-net 1/4] selftests: mptcp: run mptcp_sockopt from a new netns

[Matthieu Baerts]

Hi Mat,

On 31/10/2022 17:08, Mat Martineau wrote:
> On Mon, 31 Oct 2022, Matthieu Baerts wrote:
> 
>> Hi Geliang, Mat,
>>
>> On 23/10/2022 11:28, Geliang Tang wrote:
>>> Matthieu Baerts <matthieu.baerts@tessares.net> 于2022年10月20日周四
>>> 00:20写道：
>>>>
>>>> Not running it from a new netns causes issues if some MPTCP settings
>>>> are
>>>> modified, e.g. if MPTCP is disabled from the sysctl knob, if multiple
>>>> addresses are available and added to the MPTCP path-manager, etc.
>>>>
>>>> In these cases, the created connection will not behave as expected,
>>>> e.g.
>>>> unable to create an MPTCP socket, more than one subflow is seen, etc.
>>>>
>>>> A new "sandbox" net namespace is now created and used to run
>>>> mptcp_sockopt from this controlled environment.
>>>>
>>>> Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test
>>>> cases")
>>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>>> ---
>>>>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 9 +++++----
>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>> b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>> index 0879da915014..80d36f7cfee8 100755
>>>> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>> @@ -35,8 +35,9 @@ init()
>>>>
>>>>         ns1="ns1-$rndh"
>>>>         ns2="ns2-$rndh"
>>>> +       ns_sbox="ns_sbox-$rndh"
>>>
>>> Hi Matt, how about clearly defining ns_sbox (as well as ns1, ns2) as a
>>> global variable at the beginning of this file, and then assign values
>>> here, as the same as in mptcp_connect.sh and mptcp_join.sh?
>>
>> Thank you for the idea, it would be clearer.
>>
>> @Mat: Because this patch is for -net, would it not be better to do that
>> in an additional patch for net-next? Patch 2/4 is also for net-next and
>> depends on patch 1/4.
>>
>> Or maybe it is OK to do that in -net because it is in the selftests? I
>> would prefer to split but I'm fine to do that if we prefer.
> 
> Oh - I missed that the series was a mix of net and net-next!
> 
> Sure, fine to split it.

Great, I can do that.
Do you prefer a v2 or can I send a single patch on top of the v1?

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

Re: [PATCH mptcp-net 1/4] selftests: mptcp: run mptcp_sockopt from a new netns

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]

On Mon, 31 Oct 2022, Matthieu Baerts wrote:

> Hi Mat,
>
> On 31/10/2022 17:08, Mat Martineau wrote:
>> On Mon, 31 Oct 2022, Matthieu Baerts wrote:
>>
>>> Hi Geliang, Mat,
>>>
>>> On 23/10/2022 11:28, Geliang Tang wrote:
>>>> Matthieu Baerts <matthieu.baerts@tessares.net> 于2022年10月20日周四
>>>> 00:20写道：
>>>>>
>>>>> Not running it from a new netns causes issues if some MPTCP settings
>>>>> are
>>>>> modified, e.g. if MPTCP is disabled from the sysctl knob, if multiple
>>>>> addresses are available and added to the MPTCP path-manager, etc.
>>>>>
>>>>> In these cases, the created connection will not behave as expected,
>>>>> e.g.
>>>>> unable to create an MPTCP socket, more than one subflow is seen, etc.
>>>>>
>>>>> A new "sandbox" net namespace is now created and used to run
>>>>> mptcp_sockopt from this controlled environment.
>>>>>
>>>>> Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test
>>>>> cases")
>>>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>>>> ---
>>>>>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 9 +++++----
>>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>>> b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>>> index 0879da915014..80d36f7cfee8 100755
>>>>> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>>>>> @@ -35,8 +35,9 @@ init()
>>>>>
>>>>>         ns1="ns1-$rndh"
>>>>>         ns2="ns2-$rndh"
>>>>> +       ns_sbox="ns_sbox-$rndh"
>>>>
>>>> Hi Matt, how about clearly defining ns_sbox (as well as ns1, ns2) as a
>>>> global variable at the beginning of this file, and then assign values
>>>> here, as the same as in mptcp_connect.sh and mptcp_join.sh?
>>>
>>> Thank you for the idea, it would be clearer.
>>>
>>> @Mat: Because this patch is for -net, would it not be better to do that
>>> in an additional patch for net-next? Patch 2/4 is also for net-next and
>>> depends on patch 1/4.
>>>
>>> Or maybe it is OK to do that in -net because it is in the selftests? I
>>> would prefer to split but I'm fine to do that if we prefer.
>>
>> Oh - I missed that the series was a mix of net and net-next!
>>
>> Sure, fine to split it.
>
> Great, I can do that.
> Do you prefer a v2 or can I send a single patch on top of the v1?
>

Single patch is fine.

--
Mat Martineau
Intel

Re: [PATCH mptcp-next 0/4] mptcp: run mptcp_sockopt from a new netns + cleanup

[Mat Martineau]

On Wed, 19 Oct 2022, Matthieu Baerts wrote:

> Here are some small fixes for the MPTCP selftests, no big changes.
>
> The first patch is a fix and for -net: if we run mptcp_sockopt.sh after having
> modified some settings related to MPTCP on the host, we can run into issues
> without this patch.
>
> The second patch uses the new netns for mptcp_inq, cleaner.
>
> The third one remove some defined but unused vars.
>
> The last one uniform 'rndh' variable because it was declared with undefined
> variables in a few places.
>
> Matthieu Baerts (4):
>  selftests: mptcp: run mptcp_sockopt from a new netns
>  selftests: mptcp: run mptcp_inq from a clean netns
>  selftests: mptcp: removed defined but unused vars
>  selftests: mptcp: uniform 'rndh' variable
>
> tools/testing/selftests/net/mptcp/diag.sh       |  1 +
> .../selftests/net/mptcp/mptcp_connect.sh        |  6 +-----
> tools/testing/selftests/net/mptcp/mptcp_join.sh |  5 +++--
> .../selftests/net/mptcp/mptcp_sockopt.sh        | 17 ++++++++---------
> .../testing/selftests/net/mptcp/simult_flows.sh |  4 +---
> .../testing/selftests/net/mptcp/userspace_pm.sh |  2 +-
> 6 files changed, 15 insertions(+), 20 deletions(-)
>
>
> base-commit: 1907372cfe4315a0491ae262ac23b1b4cd9cdf94
> -- 
> 2.37.2

Ok, with the follow-up discussion and handling Geliang's suggested fix for 
mptcp-next as an additional patch, this series looks good to me:

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

--
Mat Martineau
Intel

Re: selftests: mptcp: uniform 'rndh' variable: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4978928891723776
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4978928891723776/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/6104828798566400
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6104828798566400/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/097d5592cae0


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-next 0/4] mptcp: run mptcp_sockopt from a new netns + cleanup

[Matthieu Baerts]

Hi Mat,

On 31/10/2022 23:04, Mat Martineau wrote:
> On Wed, 19 Oct 2022, Matthieu Baerts wrote:
> 
>> Here are some small fixes for the MPTCP selftests, no big changes.
>>
>> The first patch is a fix and for -net: if we run mptcp_sockopt.sh
>> after having
>> modified some settings related to MPTCP on the host, we can run into
>> issues
>> without this patch.
>>
>> The second patch uses the new netns for mptcp_inq, cleaner.
>>
>> The third one remove some defined but unused vars.
>>
>> The last one uniform 'rndh' variable because it was declared with
>> undefined
>> variables in a few places.
>>
>> Matthieu Baerts (4):
>>  selftests: mptcp: run mptcp_sockopt from a new netns
>>  selftests: mptcp: run mptcp_inq from a clean netns
>>  selftests: mptcp: removed defined but unused vars
>>  selftests: mptcp: uniform 'rndh' variable
> 
> Ok, with the follow-up discussion and handling Geliang's suggested fix
> for mptcp-next as an additional patch, this series looks good to me:
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thank you for the review!

Now in our tree (fix for net for patch 1/4, feat. for net-next with the
others):

New patches for t/upstream-net:
- c0dbc5e74ae0: selftests: mptcp: run mptcp_sockopt from a new netns
- Results: 21b3ec76c898..5a5c32fdb403 (export-net)

New patches for t/upstream:
- c0dbc5e74ae0: selftests: mptcp: run mptcp_sockopt from a new netns
- 972ae67540f4: selftests: mptcp: run mptcp_inq from a clean netns
- 6af6a13a0afc: selftests: mptcp: removed defined but unused vars
- ca4c25547d92: selftests: mptcp: uniform 'rndh' variable
- Results: d33b3e9215cd..5f9e0bf4aacf (export)


Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20221110T171431
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20221110T171431


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