2023-07-04 20:49:14

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 0/9] mptcp: fixes for v6.5

Here is a first batch of fixes for v6.5 and older.

The fixes are not linked to each others.

Patch 1 ensures subflows are unhashed before cleaning the backlog to
avoid races. This fixes another recent fix from v6.4.

Patch 2 does not rely on implicit state check in mptcp_listen() to avoid
races when receiving an MP_FASTCLOSE. A regression from v5.17.

The rest fixes issues in the selftests.

Patch 3 makes sure errors when setting up the environment are no longer
ignored. For v5.17+.

Patch 4 uses 'iptables-legacy' if available to be able to run on older
kernels. A fix for v5.13 and newer.

Patch 5 catches errors when issues are detected with packet marks. Also
for v5.13+.

Patch 6 uses the correct variable instead of an undefined one. Even if
there was no visible impact, it can help to find regressions later. An
issue visible in v5.19+.

Patch 7 makes sure errors with some sub-tests are reported to have the
selftest marked as failed as expected. Also for v5.19+.

Patch 8 adds a kernel config that is required to execute MPTCP
selftests. It is valid for v5.9+.

Patch 9 fixes issues when validating the userspace path-manager with
32-bit arch, an issue affecting v5.19+.

Signed-off-by: Matthieu Baerts <[email protected]>
---
Matthieu Baerts (7):
selftests: mptcp: connect: fail if nft supposed to work
selftests: mptcp: sockopt: use 'iptables-legacy' if available
selftests: mptcp: sockopt: return error if wrong mark
selftests: mptcp: userspace_pm: use correct server port
selftests: mptcp: userspace_pm: report errors with 'remove' tests
selftests: mptcp: depend on SYN_COOKIES
selftests: mptcp: pm_nl_ctl: fix 32-bit support

Paolo Abeni (2):
mptcp: ensure subflow is unhashed before cleaning the backlog
mptcp: do not rely on implicit state check in mptcp_listen()

net/mptcp/protocol.c | 7 +++++-
tools/testing/selftests/net/mptcp/config | 1 +
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 3 +++
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 29 ++++++++++++----------
tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 10 ++++----
tools/testing/selftests/net/mptcp/userspace_pm.sh | 4 ++-
6 files changed, 34 insertions(+), 20 deletions(-)
---
base-commit: 14bb236b29922c4f57d8c05bfdbcb82677f917c9
change-id: 20230704-upstream-net-20230704-misc-fixes-6-5-rc1-c52608649559

Best regards,
--
Matthieu Baerts <[email protected]>



2023-07-04 20:59:08

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 8/9] selftests: mptcp: depend on SYN_COOKIES

MPTCP selftests are using TCP SYN Cookies for quite a while now, since
v5.9.

Some CIs don't have this config option enabled and this is causing
issues in the tests:

# ns1 MPTCP -> ns1 (10.0.1.1:10000 ) MPTCP (duration 167ms) sysctl: cannot stat /proc/sys/net/ipv4/tcp_syncookies: No such file or directory
# [ OK ]./mptcp_connect.sh: line 554: [: -eq: unary operator expected

There is no impact in the results but the test is not doing what it is
supposed to do.

Fixes: fed61c4b584c ("selftests: mptcp: make 2nd net namespace use tcp syn cookies unconditionally")
Cc: [email protected]
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/net/mptcp/config | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/net/mptcp/config b/tools/testing/selftests/net/mptcp/config
index 6032f9b23c4c..e317c2e44dae 100644
--- a/tools/testing/selftests/net/mptcp/config
+++ b/tools/testing/selftests/net/mptcp/config
@@ -6,6 +6,7 @@ CONFIG_INET_DIAG=m
CONFIG_INET_MPTCP_DIAG=m
CONFIG_VETH=y
CONFIG_NET_SCH_NETEM=m
+CONFIG_SYN_COOKIES=y
CONFIG_NETFILTER=y
CONFIG_NETFILTER_ADVANCED=y
CONFIG_NETFILTER_NETLINK=m

--
2.40.1


2023-07-04 20:59:40

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 4/9] selftests: mptcp: sockopt: use 'iptables-legacy' if available

IPTables commands using 'iptables-nft' fail on old kernels, at least
on v5.15 because it doesn't see the default IPTables chains:

$ iptables -L
iptables/1.8.2 Failed to initialize nft: Protocol not supported

As a first step before switching to NFTables, we can use iptables-legacy
if available.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368
Fixes: dc65fe82fb07 ("selftests: mptcp: add packet mark test case")
Cc: [email protected]
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index f295a371ff14..c21bfd7f0c01 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -12,6 +12,8 @@ ksft_skip=4
timeout_poll=30
timeout_test=$((timeout_poll * 2 + 1))
mptcp_connect=""
+iptables="iptables"
+ip6tables="ip6tables"

sec=$(date +%s)
rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
@@ -25,7 +27,7 @@ add_mark_rules()
local m=$2

local t
- for t in iptables ip6tables; do
+ for t in ${iptables} ${ip6tables}; do
# just to debug: check we have multiple subflows connection requests
ip netns exec $ns $t -A OUTPUT -p tcp --syn -m mark --mark $m -j ACCEPT

@@ -95,14 +97,14 @@ if [ $? -ne 0 ];then
exit $ksft_skip
fi

-iptables -V > /dev/null 2>&1
-if [ $? -ne 0 ];then
+# Use the legacy version if available to support old kernel versions
+if iptables-legacy -V &> /dev/null; then
+ iptables="iptables-legacy"
+ ip6tables="ip6tables-legacy"
+elif ! iptables -V &> /dev/null; then
echo "SKIP: Could not run all tests without iptables tool"
exit $ksft_skip
-fi
-
-ip6tables -V > /dev/null 2>&1
-if [ $? -ne 0 ];then
+elif ! ip6tables -V &> /dev/null; then
echo "SKIP: Could not run all tests without ip6tables tool"
exit $ksft_skip
fi
@@ -112,10 +114,10 @@ check_mark()
local ns=$1
local af=$2

- local tables=iptables
+ local tables=${iptables}

if [ $af -eq 6 ];then
- tables=ip6tables
+ tables=${ip6tables}
fi

local counters values

--
2.40.1


2023-07-04 21:01:57

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 3/9] selftests: mptcp: connect: fail if nft supposed to work

In case of "external" errors when preparing the environment for the
TProxy tests, the subtests were marked as skipped.

This is fine but it means these errors are ignored. On MPTCP Public CI,
we do want to catch such issues and mark the selftest as failed if there
are such issues. We can then use mptcp_lib_fail_if_expected_feature()
helper that has been recently added to fail if needed.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368
Fixes: 5fb62e9cd3ad ("selftests: mptcp: add tproxy test case")
Cc: [email protected]
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 3 +++
1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 13561e5bc0cd..bbae40882bfa 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -718,6 +718,7 @@ table inet mangle {
EOF
if [ $? -ne 0 ]; then
echo "SKIP: $msg, could not load nft ruleset"
+ mptcp_lib_fail_if_expected_feature "nft rules"
return
fi

@@ -733,6 +734,7 @@ EOF
if [ $? -ne 0 ]; then
ip netns exec "$listener_ns" nft flush ruleset
echo "SKIP: $msg, ip $r6flag rule failed"
+ mptcp_lib_fail_if_expected_feature "ip rule"
return
fi

@@ -741,6 +743,7 @@ EOF
ip netns exec "$listener_ns" nft flush ruleset
ip -net "$listener_ns" $r6flag rule del fwmark 1 lookup 100
echo "SKIP: $msg, ip route add local $local_addr failed"
+ mptcp_lib_fail_if_expected_feature "ip route"
return
fi


--
2.40.1


2023-07-04 21:02:37

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 7/9] selftests: mptcp: userspace_pm: report errors with 'remove' tests

A message was mentioning an issue with the "remove" tests but the
selftest was not marked as failed.

Directly exit with an error like it is done everywhere else in this
selftest.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368
Fixes: 259a834fadda ("selftests: mptcp: functional tests for the userspace PM type")
Cc: [email protected]
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 841a67a7d524..b180133a30af 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -423,6 +423,7 @@ test_remove()
stdbuf -o0 -e0 printf "[OK]\n"
else
stdbuf -o0 -e0 printf "[FAIL]\n"
+ exit 1
fi

# RM_ADDR using an invalid addr id should result in no action
@@ -437,6 +438,7 @@ test_remove()
stdbuf -o0 -e0 printf "[OK]\n"
else
stdbuf -o0 -e0 printf "[FAIL]\n"
+ exit 1
fi

# RM_ADDR from the client to server machine

--
2.40.1


2023-07-04 21:03:50

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 5/9] selftests: mptcp: sockopt: return error if wrong mark

When an error was detected when checking the marks, a message was
correctly printed mentioning the error but followed by another one
saying everything was OK and the selftest was not marked as failed as
expected.

Now the 'ret' variable is directly set to 1 in order to make sure the
exit is done with an error, similar to what is done in other functions.
While at it, the error is correctly propagated to the caller.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368
Fixes: dc65fe82fb07 ("selftests: mptcp: add packet mark test case")
Cc: [email protected]
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
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 c21bfd7f0c01..dc8d473fc82c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -128,6 +128,7 @@ check_mark()
for v in $values; do
if [ $v -ne 0 ]; then
echo "FAIL: got $tables $values in ns $ns , not 0 - not all expected packets marked" 1>&2
+ ret=1
return 1
fi
done
@@ -227,11 +228,11 @@ do_transfer()
fi

if [ $local_addr = "::" ];then
- check_mark $listener_ns 6
- check_mark $connector_ns 6
+ check_mark $listener_ns 6 || retc=1
+ check_mark $connector_ns 6 || retc=1
else
- check_mark $listener_ns 4
- check_mark $connector_ns 4
+ check_mark $listener_ns 4 || retc=1
+ check_mark $connector_ns 4 || retc=1
fi

check_transfer $cin $sout "file received by server"

--
2.40.1


2023-07-04 21:03:50

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 6/9] selftests: mptcp: userspace_pm: use correct server port

"server4_port" variable is not set but "app4_port" is the server port in
v4 and the correct variable name to use.

The port is optional so there was no visible impact.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368
Fixes: ca188a25d43f ("selftests: mptcp: userspace PM support for MP_PRIO signals")
Cc: [email protected]
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 98d9e4d2d3fc..841a67a7d524 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -848,7 +848,7 @@ test_prio()
local count

# Send MP_PRIO signal from client to server machine
- ip netns exec "$ns2" ./pm_nl_ctl set 10.0.1.2 port "$client4_port" flags backup token "$client4_token" rip 10.0.1.1 rport "$server4_port"
+ ip netns exec "$ns2" ./pm_nl_ctl set 10.0.1.2 port "$client4_port" flags backup token "$client4_token" rip 10.0.1.1 rport "$app4_port"
sleep 0.5

# Check TX

--
2.40.1


2023-07-05 10:26:18

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net 0/9] mptcp: fixes for v6.5

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:

On Tue, 04 Jul 2023 22:44:32 +0200 you wrote:
> Here is a first batch of fixes for v6.5 and older.
>
> The fixes are not linked to each others.
>
> Patch 1 ensures subflows are unhashed before cleaning the backlog to
> avoid races. This fixes another recent fix from v6.4.
>
> [...]

Here is the summary with links:
- [net,1/9] mptcp: ensure subflow is unhashed before cleaning the backlog
https://git.kernel.org/netdev/net/c/3fffa15bfef4
- [net,2/9] mptcp: do not rely on implicit state check in mptcp_listen()
https://git.kernel.org/netdev/net/c/0226436acf24
- [net,3/9] selftests: mptcp: connect: fail if nft supposed to work
https://git.kernel.org/netdev/net/c/221e4550454a
- [net,4/9] selftests: mptcp: sockopt: use 'iptables-legacy' if available
https://git.kernel.org/netdev/net/c/a5a5990c099d
- [net,5/9] selftests: mptcp: sockopt: return error if wrong mark
https://git.kernel.org/netdev/net/c/9ac4c28eb70c
- [net,6/9] selftests: mptcp: userspace_pm: use correct server port
https://git.kernel.org/netdev/net/c/d8566d0e0392
- [net,7/9] selftests: mptcp: userspace_pm: report errors with 'remove' tests
https://git.kernel.org/netdev/net/c/966c6c3adfb1
- [net,8/9] selftests: mptcp: depend on SYN_COOKIES
https://git.kernel.org/netdev/net/c/6c8880fcaa5c
- [net,9/9] selftests: mptcp: pm_nl_ctl: fix 32-bit support
https://git.kernel.org/netdev/net/c/61d965805026

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html