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>

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

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.