netdev - Re: [PATCH net-next] selftests: forwarding: Avoid failures to source net/lib.sh

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZZcywXyDs6rL03Ok@Laptop-X1>
Date: Fri, 5 Jan 2024 06:35:45 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Benjamin Poirier <bpoirier@...dia.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Shuah Khan <shuah@...nel.org>, Petr Machata <petrm@...dia.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Ido Schimmel <idosch@...dia.com>,
	Tobias Waldekranz <tobias@...dekranz.com>,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: forwarding: Avoid failures to source
 net/lib.sh

On Thu, Jan 04, 2024 at 09:11:09AM -0500, Benjamin Poirier wrote:
> The expression "source ../lib.sh" added to net/forwarding/lib.sh in commit
> 25ae948b4478 ("selftests/net: add lib.sh") does not work for tests outside
> net/forwarding which source net/forwarding/lib.sh (1). It also does not
> work in some cases where only a subset of tests are exported (2).
> 
> Avoid the problems mentioned above by replacing the faulty expression with
> a copy of the content from net/lib.sh which is used by files under
> net/forwarding.
> 
> A more thorough solution which avoids duplicating content between
> net/lib.sh and net/forwarding/lib.sh has been posted here:
> https://lore.kernel.org/netdev/20231222135836.992841-1-bpoirier@nvidia.com/
> 
> The approach in the current patch is a stopgap solution to avoid submitting
> large changes at the eleventh hour of this development cycle.
> 
> Example of problem 1)
> 
> tools/testing/selftests/drivers/net/bonding$ ./dev_addr_lists.sh
> ./net_forwarding_lib.sh: line 41: ../lib.sh: No such file or directory
> TEST: bonding cleanup mode active-backup                            [ OK ]
> TEST: bonding cleanup mode 802.3ad                                  [ OK ]
> TEST: bonding LACPDU multicast address to slave (from bond down)    [ OK ]
> TEST: bonding LACPDU multicast address to slave (from bond up)      [ OK ]
> 
> An error message is printed but since the test does not use functions from
> net/lib.sh, the test results are not affected.
> 
> Example of problem 2)
> 
> tools/testing/selftests$ make install TARGETS="net/forwarding"
> tools/testing/selftests$ cd kselftest_install/net/forwarding/
> tools/testing/selftests/kselftest_install/net/forwarding$ ./pedit_ip.sh veth{0..3}
> lib.sh: line 41: ../lib.sh: No such file or directory
> TEST: ping                                                          [ OK ]
> TEST: ping6                                                         [ OK ]
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth1 ingress pedit ip src set 198.51.100.1               [FAIL]
>         Expected to get 10 packets, but got .
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth2 egress pedit ip src set 198.51.100.1                [FAIL]
>         Expected to get 10 packets, but got .
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth1 ingress pedit ip dst set 198.51.100.1               [FAIL]
>         Expected to get 10 packets, but got .
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth2 egress pedit ip dst set 198.51.100.1                [FAIL]
>         Expected to get 10 packets, but got .
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth1 ingress pedit ip6 src set 2001:db8:2::1             [FAIL]
>         Expected to get 10 packets, but got .
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth2 egress pedit ip6 src set 2001:db8:2::1              [FAIL]
>         Expected to get 10 packets, but got .
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth1 ingress pedit ip6 dst set 2001:db8:2::1             [FAIL]
>         Expected to get 10 packets, but got .
> ./pedit_ip.sh: line 135: busywait: command not found
> TEST: dev veth2 egress pedit ip6 dst set 2001:db8:2::1              [FAIL]
>         Expected to get 10 packets, but got .
> 
> In this case, the test results are affected.
> 
> Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
> Suggested-by: Ido Schimmel <idosch@...dia.com>
> Suggested-by: Petr Machata <petrm@...dia.com>
> Reviewed-by: Ido Schimmel <idosch@...dia.com>
> Tested-by: Petr Machata <petrm@...dia.com>
> Signed-off-by: Benjamin Poirier <bpoirier@...dia.com>
> ---
>  tools/testing/selftests/net/forwarding/lib.sh | 27 ++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index 69ef2a40df21..8a61464ab6eb 100755
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -38,7 +38,32 @@ if [[ -f $relative_path/forwarding.config ]]; then
>  	source "$relative_path/forwarding.config"
>  fi
>  
> -source ../lib.sh
> +# Kselftest framework requirement - SKIP code is 4.
> +ksft_skip=4
> +
> +busywait()
> +{
> +	local timeout=$1; shift
> +
> +	local start_time="$(date -u +%s%3N)"
> +	while true
> +	do
> +		local out
> +		out=$("$@")
> +		local ret=$?
> +		if ((!ret)); then
> +			echo -n "$out"
> +			return 0
> +		fi
> +
> +		local current_time="$(date -u +%s%3N)"
> +		if ((current_time - start_time > timeout)); then
> +			echo -n "$out"
> +			return 1
> +		fi
> +	done
> +}
> +
>  ##############################################################################
>  # Sanity checks
>  

Reviewed-by: Hangbin Liu <liuhangbin@...il.com>

Thanks
Hangbin