[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2a2qhhyui2by6cw3nqepwgfxxrknyjx5rgaybt4dvqowflom2r@i55r2csxbmb4>
Date: Tue, 30 Sep 2025 10:37:52 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Bobby Eshleman <bobbyeshleman@...il.com>
Cc: Shuah Khan <shuah@...nel.org>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
Stefan Hajnoczi <stefanha@...hat.com>, "Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Eugenio Pérez <eperezma@...hat.com>, "K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
Bryan Tan <bryan-bt.tan@...adcom.com>, Vishnu Dasa <vishnu.dasa@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, virtualization@...ts.linux.dev, netdev@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
linux-hyperv@...r.kernel.org, berrange@...hat.com, Bobby Eshleman <bobbyeshleman@...a.com>
Subject: Re: [PATCH net-next v6 8/9] selftests/vsock: invoke vsock_test
through helpers
On Tue, Sep 16, 2025 at 04:43:52PM -0700, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@...a.com>
>
>Add helper calls vm_vsock_test() and host_vsock_test() to invoke the
>vsock_test binary. This encapsulates several items of repeat logic, such
>as waiting for the server to reach listening state and
>enabling/disabling the bash option pipefail to avoid pipe-style logging
>from hiding failures.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@...a.com>
>---
> tools/testing/selftests/vsock/vmtest.sh | 120 ++++++++++++++++++++++++++++----
> 1 file changed, 108 insertions(+), 12 deletions(-)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index 183647a86c8a..5e36d1068f6f 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -248,6 +248,7 @@ wait_for_listener()
> local port=$1
> local interval=$2
> local max_intervals=$3
>+ local old_pipefail
> local protocol=tcp
> local pattern
> local i
>@@ -256,6 +257,13 @@ wait_for_listener()
>
> # for tcp protocol additionally check the socket state
> [ "${protocol}" = "tcp" ] && pattern="${pattern}0A"
>+
>+ # 'grep -q' exits on match, sending SIGPIPE to 'awk', which exits with
>+ # an error, causing the if-condition to fail when pipefail is set.
>+ # Instead, temporarily disable pipefail and restore it later.
>+ old_pipefail=$(set -o | awk '/^pipefail[[:space:]]+(on|off)$/{print $2}')
>+ set +o pipefail
>+
> for i in $(seq "${max_intervals}"); do
> if awk '{print $2" "$4}' /proc/net/"${protocol}"* | \
> grep -q "${pattern}"; then
>@@ -263,6 +271,10 @@ wait_for_listener()
> fi
> sleep "${interval}"
> done
>+
>+ if [[ "${old_pipefail}" == on ]]; then
>+ set -o pipefail
>+ fi
> }
>
> vm_wait_for_listener() {
>@@ -314,28 +326,112 @@ log_guest() {
> LOG_PREFIX=guest log $@
> }
>
>+vm_vsock_test() {
>+ local ns=$1
>+ local mode=$2
>+ local rc
>+
>+ set -o pipefail
>+ if [[ "${mode}" == client ]]; then
>+ local host=$3
I don't really like having the number and type of parameters of a
function depend on others, maintaining it could become a mess.
Can we avoid “mode” altogether and use “host” to discriminate between
server and client?
e.g. if “host” == server then we launch the server, otherwise we
interpret it as IP, or something else.
>+ local cid=$4
>+ local port=$5
>+
>+ # log output and use pipefail to respect vsock_test errors
>+ vm_ssh "${ns}" -- "${VSOCK_TEST}" \
>+ --mode=client \
>+ --control-host="${host}" \
>+ --peer-cid="${cid}" \
>+ --control-port="${port}" \
>+ 2>&1 | log_guest
>+ rc=$?
>+ else
>+ local cid=$3
>+ local port=$4
>+
>+ # log output and use pipefail to respect vsock_test errors
>+ vm_ssh "${ns}" -- "${VSOCK_TEST}" \
>+ --mode=server \
>+ --peer-cid="${cid}" \
>+ --control-port="${port}" \
>+ 2>&1 | log_guest &
>+ rc=$?
>+
>+ if [[ $rc -ne 0 ]]; then
>+ set +o pipefail
>+ return $rc
>+ fi
>+
>+ vm_wait_for_listener "${ns}" "${port}"
>+ rc=$?
>+ fi
>+ set +o pipefail
>+
>+ return $rc
> }
>
>+host_vsock_test() {
>+ local ns=$1
>+ local mode=$2
>+ local cmd
>+
>+ if [[ "${ns}" == none ]]; then
>+ cmd="${VSOCK_TEST}"
>+ else
>+ cmd="ip netns exec ${ns} ${VSOCK_TEST}"
>+ fi
>+
>+ # log output and use pipefail to respect vsock_test errors
>+ set -o pipefail
>+ if [[ "${mode}" == client ]]; then
>+ local host=$3
Ditto.
The rest LGTM.
Thanks,
Stefano
>+ local cid=$4
>+ local port=$5
>+
>+ ${cmd} \
>+ --mode="${mode}" \
>+ --peer-cid="${cid}" \
>+ --control-host="${host}" \
>+ --control-port="${port}" 2>&1 | log_host
>+ rc=$?
>+ else
>+ local cid=$3
>+ local port=$4
>+
>+ ${cmd} \
>+ --mode="${mode}" \
>+ --peer-cid="${cid}" \
>+ --control-port="${port}" 2>&1 | log_host &
>+ rc=$?
>+
>+ if [[ $rc -ne 0 ]]; then
>+ return $rc
>+ fi
>+
>+ host_wait_for_listener "${ns}" "${port}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}"
>+ rc=$?
>+ fi
>+ set +o pipefail
>
>+ return $rc
> }
>
> test_vm_server_host_client() {
>+ vm_vsock_test "none" "server" 2 "${TEST_GUEST_PORT}"
>+ host_vsock_test "none" "client" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
>+}
>
>- vm_ssh -- "${VSOCK_TEST}" \
>- --mode=server \
>- --control-port="${TEST_GUEST_PORT}" \
>- --peer-cid=2 \
>- 2>&1 | log_guest &
>+test_vm_client_host_server() {
>+ host_vsock_test "none" "server" "${VSOCK_CID}" "${TEST_HOST_PORT_LISTENER}"
>+ vm_vsock_test "none" "client" "10.0.2.2" 2 "${TEST_HOST_PORT_LISTENER}"
>+}
>
>- vm_wait_for_listener "${TEST_GUEST_PORT}"
>+test_vm_loopback() {
>+ vm_vsock_test "none" "server" 1 "${TEST_HOST_PORT_LISTENER}"
>+ vm_vsock_test "none" "client" "127.0.0.1" 1 "${TEST_HOST_PORT_LISTENER}"
>+}
>
>- ${VSOCK_TEST} \
>- --mode=client \
>- --control-host=127.0.0.1 \
>- --peer-cid="${VSOCK_CID}" \
>- --control-port="${TEST_HOST_PORT}" 2>&1 | log_host
>
>- return $?
> }
>
> test_vm_client_host_server() {
>
>--
>2.47.3
>
Powered by blists - more mailing lists