From: Matthieu Baerts <matttbe@kernel.org>
To: Geliang Tang <geliang@kernel.org>, mptcp@lists.linux.dev
Cc: Geliang Tang <tanggeliang@kylinos.cn>
Subject: Re: [PATCH mptcp-next v3 2/2] selftests: mptcp: add mptcp_lib_check_tools helper
Date: Mon, 19 Feb 2024 10:01:00 +0100 [thread overview]
Message-ID: <ac77e129-a6ef-42e7-84e8-fb7e19bc6b30@kernel.org> (raw)
In-Reply-To: <428122f8879686b24852d3e46850b9e3538afb99.1708330720.git.tanggeliang@kylinos.cn>
Hi Geliang,
Thank you for the new version.
On 19/02/2024 09:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> This patch exports check_tools() helper from mptcp_join.sh into
> mptcp_lib.sh as a public one mptcp_lib_check_tools(). The arguments
> "ip", "ss", and "iptables" are passed into this helper to indicate
'ip6tables' can be passed too since v3.
> whether to check ip tool, ss tool and iptables tools.
>
> This helper can be used in every scripts.
(...)
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index d797b27b7626..d9df6eb6a52a 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -152,34 +152,12 @@ cleanup_partial()
> done
> }
>
> -check_tools()
> -{
> - mptcp_lib_check_mptcp
> - mptcp_lib_check_kallsyms
> -
> - if ! ip -Version &> /dev/null; then
> - echo "SKIP: Could not run test without ip tool"
> - exit $ksft_skip
> - fi
> -
> - if ! ss -h | grep -q MPTCP; then
> - echo "SKIP: ss tool does not support MPTCP"
> - exit $ksft_skip
> - fi
> -
> - 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"
> - exit $ksft_skip
> - fi
> -}
> -
> init() {
> init=1
>
> - check_tools
> + mptcp_lib_check_mptcp
> + mptcp_lib_check_kallsyms
> + mptcp_lib_check_tools ip ss iptables
You should use "${iptables}" -- just in case we switch to iptables-nft
or something else later -- and add "${ip6tables}" now that the check is
split.
> sin=$(mktemp)
> sout=$(mktemp)
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 108a1e12436c..b92c5278730b 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -319,3 +319,34 @@ mptcp_lib_wait_local_port_listen() {
> sleep 0.1
> done
> }
> +
> +mptcp_lib_check_tools() {
> + local tool
> +
> + for tool in "${@}"; do
> + case "${tool}" in
> + "ip")
> + if ! ip -Version &> /dev/null; then
> + mptcp_lib_print_warn "SKIP: Could not run test without ip tool"
> + exit ${KSFT_SKIP}
> + fi
> + ;;
> + "ss")
> + if ! ss -h | grep -q MPTCP; then
> + mptcp_lib_print_warn "SKIP: ss tool does not support MPTCP"
> + exit ${KSFT_SKIP}
> + fi
> + ;;
> + "iptables"* | "ip6tables"*)
> + if ! "${tool}" -V &> /dev/null; then
> + mptcp_lib_print_warn "SKIP: Could not run all tests without ${tool} tool"
> + exit ${KSFT_SKIP}
> + fi
> + ;;
> + *)
> + mptcp_lib_print_warn "Unsupported tool: ${tool}"
> + exit ${KSFT_SKIP}
If we are here, it means we have an internal error: we are checking a
tool we don't support. I think you should then use 'mptcp_lib_print_err'
and exit an error "${KSFT_FAIL}".
If you prefer, you can also print a more explicit message, e.g.
Internal error: unsupported tool: ${tool}
Up to you, this error should not happen.
> + ;;
> + esac
> + done
> +}
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index dac8e1fc7143..edb71f44be45 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -89,20 +89,7 @@ cleanup()
>
> mptcp_lib_check_mptcp
> mptcp_lib_check_kallsyms
> -
> -ip -Version > /dev/null 2>&1
> -if [ $? -ne 0 ];then
> - echo "SKIP: Could not run test without ip tool"
> - exit $ksft_skip
> -fi
> -
> -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"
> - exit $ksft_skip
> -fi
> +mptcp_lib_check_tools ip iptables
Same here: use "${iptables}" instead, and add "${ip6tables}".
>
> check_mark()
> {
(...)
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
next prev parent reply other threads:[~2024-02-19 9:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-19 8:22 [PATCH mptcp-next v3 0/2] add helpers and vars in mptcp_lib.sh, part 1 Geliang Tang
2024-02-19 8:22 ` [PATCH mptcp-next v3 1/2] selftests: mptcp: join: add ss mptcp support check Geliang Tang
2024-02-19 8:52 ` Matthieu Baerts
2024-02-19 10:43 ` Matthieu Baerts
2024-02-19 8:22 ` [PATCH mptcp-next v3 2/2] selftests: mptcp: add mptcp_lib_check_tools helper Geliang Tang
2024-02-19 9:01 ` Matthieu Baerts [this message]
2024-02-19 9:17 ` selftests: mptcp: add mptcp_lib_check_tools helper: Tests Results MPTCP CI
2024-02-19 9:47 ` MPTCP CI
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ac77e129-a6ef-42e7-84e8-fb7e19bc6b30@kernel.org \
--to=matttbe@kernel.org \
--cc=geliang@kernel.org \
--cc=mptcp@lists.linux.dev \
--cc=tanggeliang@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.