[PATCH mptcp-next] selftests: mptcp: stop forcing iptables-legacy

[PATCH mptcp-next] selftests: mptcp: stop forcing iptables-legacy

[Matthieu Baerts (NGI0)]

Commit 0c4cd3f86a40 ("selftests: mptcp: join: use 'iptables-legacy' if
available") and a5a5990c099d ("selftests: mptcp: sockopt: use
'iptables-legacy' if available") forced using iptables-legacy if
available.

This was needed because of some issues that were visible when testing
the kselftests on a v5.15.x with iptables-nft as default backend. It
looks like these errors are no longer present. As mentioned by Pablo [1],
the errors were maybe due to missing kernel config. We can then use
iptables-nft if it is the default one, instead of using a legacy tool.

We can then check the variables iptables and ip6tables are valid. We can
keep the variables to easily change it later or add options.

Link: https://lore.kernel.org/netdev/ZbFiixyMFpQnxzCH@calendula/ [1]
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 12 ++++--------
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 12 ++++--------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c07386e21e0a..20140ee4de2e 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -161,15 +161,11 @@ check_tools()
 		exit $ksft_skip
 	fi
 
-	# 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"
+	if ! "${iptables}" -V &> /dev/null; then
+		echo "SKIP: Could not run all tests without ${iptables} tool"
 		exit $ksft_skip
-	elif ! ip6tables -V &> /dev/null; then
-		echo "SKIP: Could not run all tests without ip6tables tool"
+	elif ! "${ip6tables}" -V &> /dev/null; then
+		echo "SKIP: Could not run all tests without ${ip6tables} tool"
 		exit $ksft_skip
 	fi
 }
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index c643872ddf47..dac8e1fc7143 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -96,15 +96,11 @@ if [ $? -ne 0 ];then
 	exit $ksft_skip
 fi
 
-# 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"
+if ! "${iptables}" -V &> /dev/null; then
+	echo "SKIP: Could not run all tests without ${iptables} tool"
 	exit $ksft_skip
-elif ! ip6tables -V &> /dev/null; then
-	echo "SKIP: Could not run all tests without ip6tables tool"
+elif ! "${ip6tables}" -V &> /dev/null; then
+	echo "SKIP: Could not run all tests without ${ip6tables} tool"
 	exit $ksft_skip
 fi
 

---
base-commit: c2469b38e369c3f2b9577beeb9470cc757abc1b9
change-id: 20240207-selftests-mptcp-iptables-nft-2bfd4a4cfb81

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

Re: selftests: mptcp: stop forcing iptables-legacy: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7819512363

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


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-normal

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 (NGI0 Core)

Re: selftests: mptcp: stop forcing iptables-legacy: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6418879304433664
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6418879304433664/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5527929375948800
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5527929375948800/summary/summary.txt

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


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 (NGI0 Core)

Re: [PATCH mptcp-next] selftests: mptcp: stop forcing iptables-legacy

[Mat Martineau]

On Wed, 7 Feb 2024, Matthieu Baerts (NGI0) wrote:

> Commit 0c4cd3f86a40 ("selftests: mptcp: join: use 'iptables-legacy' if
> available") and a5a5990c099d ("selftests: mptcp: sockopt: use

Hi Matthieu -

You can add the word "commit" right before "a5a5990c099d" in the commit 
message and that will satisfy checkpatch. Fine to do that when applying 
this to the topgit tree, everything else LGTM:

Reviewed-by: Mat Martineau <martineau@kernel.org>


> 'iptables-legacy' if available") forced using iptables-legacy if
> available.
>
> This was needed because of some issues that were visible when testing
> the kselftests on a v5.15.x with iptables-nft as default backend. It
> looks like these errors are no longer present. As mentioned by Pablo [1],
> the errors were maybe due to missing kernel config. We can then use
> iptables-nft if it is the default one, instead of using a legacy tool.
>
> We can then check the variables iptables and ip6tables are valid. We can
> keep the variables to easily change it later or add options.
>
> Link: https://lore.kernel.org/netdev/ZbFiixyMFpQnxzCH@calendula/ [1]
> Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh    | 12 ++++--------
> tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 12 ++++--------
> 2 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c07386e21e0a..20140ee4de2e 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -161,15 +161,11 @@ check_tools()
> 		exit $ksft_skip
> 	fi
>
> -	# 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"
> +	if ! "${iptables}" -V &> /dev/null; then
> +		echo "SKIP: Could not run all tests without ${iptables} tool"
> 		exit $ksft_skip
> -	elif ! ip6tables -V &> /dev/null; then
> -		echo "SKIP: Could not run all tests without ip6tables tool"
> +	elif ! "${ip6tables}" -V &> /dev/null; then
> +		echo "SKIP: Could not run all tests without ${ip6tables} tool"
> 		exit $ksft_skip
> 	fi
> }
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index c643872ddf47..dac8e1fc7143 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -96,15 +96,11 @@ if [ $? -ne 0 ];then
> 	exit $ksft_skip
> fi
>
> -# 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"
> +if ! "${iptables}" -V &> /dev/null; then
> +	echo "SKIP: Could not run all tests without ${iptables} tool"
> 	exit $ksft_skip
> -elif ! ip6tables -V &> /dev/null; then
> -	echo "SKIP: Could not run all tests without ip6tables tool"
> +elif ! "${ip6tables}" -V &> /dev/null; then
> +	echo "SKIP: Could not run all tests without ${ip6tables} tool"
> 	exit $ksft_skip
> fi
>
>
> ---
> base-commit: c2469b38e369c3f2b9577beeb9470cc757abc1b9
> change-id: 20240207-selftests-mptcp-iptables-nft-2bfd4a4cfb81
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>

Re: [PATCH mptcp-next] selftests: mptcp: stop forcing iptables-legacy

[Matthieu Baerts]

Hi Mat,

On 14/02/2024 20:09, Mat Martineau wrote:
> On Wed, 7 Feb 2024, Matthieu Baerts (NGI0) wrote:
> 
>> Commit 0c4cd3f86a40 ("selftests: mptcp: join: use 'iptables-legacy' if
>> available") and a5a5990c099d ("selftests: mptcp: sockopt: use
> 
> Hi Matthieu -
> 
> You can add the word "commit" right before "a5a5990c099d" in the commit
> message and that will satisfy checkpatch.

Good idea! I ignored the warning because there was already a "Commit"
before -- should have been "commit*s*" in fact --, but best to add this
second 'commit' to avoid a warning on Netdev's patchwork!

> Fine to do that when applying
> this to the topgit tree

Just did that!

> everything else LGTM:
> 
> Reviewed-by: Mat Martineau <martineau@kernel.org>

Thank you for the review!

Now in our tree (feat. for -next):

New patches for t/upstream:
- 55900e5db350: selftests: mptcp: stop forcing iptables-legacy
- Results: ce1856db2249..52e05de42e10 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240215T111117

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: selftests: mptcp: stop forcing iptables-legacy: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7906351480

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


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-normal

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 (NGI0 Core)

Re: selftests: mptcp: stop forcing iptables-legacy: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

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

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

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


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 (NGI0 Core)