The vsock_diag.ko module already has a test suite but the core AF_VSOCK
functionality has no tests. This patch series adds several test cases that
exercise AF_VSOCK SOCK_STREAM socket semantics (send/recv, connect/accept,
half-closed connections, simultaneous connections).
Stefan: Do you think we should have a single application or is better to
split it in single tests (e.g. vsock_test_send_recv, vsock_test_half_close,
vsock_test_multiconnection, etc.)?
Jorgen: Please test the VMCI transport and let me know if the fixes work.
I added the '--transport' parameter to skip the read() on the half-closed
connection test.
Dexuan: Do you think can be useful to test HyperV?
The v1 of this series was originally sent by Stefan. I rebased on master
and tried to fix some issues reported by Jorgen.
v2:
- Patch 1 added by Stefan to fix header include in vsock_diag_test.
- Patch 2 added by Stefan to add SPDX identifiers. I modified it to be
aligned with SPDX currently used on master.
- Patch 3:
* fixed SPDX [Stefano].
- Patch 7:
* Drop unnecessary includes [Stefan]
* Fixed SPDX [Stefano]
* Set MULTICONN_NFDS to 100 [Stefano]
* Changed (i % 1) in (i % 2) in the 'multiconn' test [Stefano]
- Patches 8,9,10 added to skip read after close in test_stream*close tests
on a VMCI host.
- Patch 11 added to wait for the remote to close the connection, before
check if a send returns -EPIPE.
v1: https://patchwork.ozlabs.org/cover/847998/
Stefan Hajnoczi (7):
VSOCK: fix header include in vsock_diag_test
VSOCK: add SPDX identifiers to vsock tests
VSOCK: extract utility functions from vsock_diag_test.c
VSOCK: extract connect/accept functions from vsock_diag_test.c
VSOCK: add full barrier between test cases
VSOCK: add send_byte()/recv_byte() test utilities
VSOCK: add AF_VSOCK test cases
Stefano Garzarella (4):
VSOCK: add vsock_get_local_cid() test utility
vsock_test: add --transport parameter
vsock_test: skip read() in test_stream*close tests on a VMCI host
vsock_test: wait for the remote to close the connection
tools/testing/vsock/.gitignore | 1 +
tools/testing/vsock/Makefile | 9 +-
tools/testing/vsock/README | 3 +-
tools/testing/vsock/control.h | 1 +
tools/testing/vsock/timeout.h | 1 +
tools/testing/vsock/util.c | 342 ++++++++++++++++++++++++
tools/testing/vsock/util.h | 54 ++++
tools/testing/vsock/vsock_diag_test.c | 170 ++----------
tools/testing/vsock/vsock_test.c | 362 ++++++++++++++++++++++++++
9 files changed, 793 insertions(+), 150 deletions(-)
create mode 100644 tools/testing/vsock/util.c
create mode 100644 tools/testing/vsock/util.h
create mode 100644 tools/testing/vsock/vsock_test.c
--
2.20.1
Before check if a send returns -EPIPE, we need to make sure the
connection is closed.
To do that, we use epoll API to wait EPOLLRDHUP or EPOLLHUP events
on the socket.
Reported-by: Jorgen Hansen <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/util.c | 38 ++++++++++++++++++++++++++++++++
tools/testing/vsock/util.h | 1 +
tools/testing/vsock/vsock_test.c | 10 +++++++++
3 files changed, 49 insertions(+)
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 41b94495ecb1..425181fe196c 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -14,6 +14,7 @@
#include <signal.h>
#include <unistd.h>
#include <assert.h>
+#include <sys/epoll.h>
#include "timeout.h"
#include "control.h"
@@ -61,6 +62,43 @@ unsigned int vsock_get_local_cid(int fd)
return svm.svm_cid;
}
+/* Wait for the remote to close the connection */
+void vsock_wait_remote_close(int fd)
+{
+ struct epoll_event ev;
+ int epollfd, nfds;
+
+ epollfd = epoll_create1(0);
+ if (epollfd == -1) {
+ perror("epoll_create1");
+ exit(EXIT_FAILURE);
+ }
+
+ ev.events = EPOLLRDHUP | EPOLLHUP;
+ ev.data.fd = fd;
+ if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
+ perror("epoll_ctl");
+ exit(EXIT_FAILURE);
+ }
+
+ nfds = epoll_wait(epollfd, &ev, 1, TIMEOUT * 1000);
+ if (nfds == -1) {
+ perror("epoll_wait");
+ exit(EXIT_FAILURE);
+ }
+
+ if (nfds == 0) {
+ fprintf(stderr, "epoll_wait timed out\n");
+ exit(EXIT_FAILURE);
+ }
+
+ assert(nfds == 1);
+ assert(ev.events & (EPOLLRDHUP | EPOLLHUP));
+ assert(ev.data.fd == fd);
+
+ close(epollfd);
+}
+
/* Connect to <cid, port> and return the file descriptor. */
int vsock_stream_connect(unsigned int cid, unsigned int port)
{
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index 7fdb8100f035..89816966c05b 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -45,6 +45,7 @@ int vsock_stream_connect(unsigned int cid, unsigned int port);
int vsock_stream_accept(unsigned int cid, unsigned int port,
struct sockaddr_vm *clientaddrp);
unsigned int vsock_get_local_cid(int fd);
+void vsock_wait_remote_close(int fd);
void send_byte(int fd, int expected_ret);
void recv_byte(int fd, int expected_ret);
void run_tests(const struct test_case *test_cases,
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 64adf45501ca..a664675bec5a 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -84,6 +84,11 @@ static void test_stream_client_close_server(const struct test_opts *opts)
control_expectln("CLOSED");
+ /* Wait for the remote to close the connection, before check
+ * -EPIPE error on send.
+ */
+ vsock_wait_remote_close(fd);
+
send_byte(fd, -EPIPE);
/* Skip the read of data wrote by the peer if we are on VMCI and
@@ -113,6 +118,11 @@ static void test_stream_server_close_client(const struct test_opts *opts)
control_expectln("CLOSED");
+ /* Wait for the remote to close the connection, before check
+ * -EPIPE error on send.
+ */
+ vsock_wait_remote_close(fd);
+
send_byte(fd, -EPIPE);
/* Skip the read of data wrote by the peer if we are on VMCI and
--
2.20.1
This patch adds utility to get local CID, useful to
understand if we are in the host or guest.
Signed-off-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/util.c | 17 +++++++++++++++++
tools/testing/vsock/util.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index d46a6e079b96..41b94495ecb1 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -13,6 +13,7 @@
#include <stdlib.h>
#include <signal.h>
#include <unistd.h>
+#include <assert.h>
#include "timeout.h"
#include "control.h"
@@ -44,6 +45,22 @@ unsigned int parse_cid(const char *str)
return n;
}
+/* Get the local CID */
+unsigned int vsock_get_local_cid(int fd)
+{
+ struct sockaddr_vm svm;
+ socklen_t svm_len = sizeof(svm);
+
+ if (getsockname(fd, (struct sockaddr *) &svm, &svm_len)) {
+ perror("getsockname");
+ exit(EXIT_FAILURE);
+ }
+
+ assert(svm.svm_family == AF_VSOCK);
+
+ return svm.svm_cid;
+}
+
/* Connect to <cid, port> and return the file descriptor. */
int vsock_stream_connect(unsigned int cid, unsigned int port)
{
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index fe524d393d67..379e02ab59bb 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -36,6 +36,7 @@ unsigned int parse_cid(const char *str);
int vsock_stream_connect(unsigned int cid, unsigned int port);
int vsock_stream_accept(unsigned int cid, unsigned int port,
struct sockaddr_vm *clientaddrp);
+unsigned int vsock_get_local_cid(int fd);
void send_byte(int fd, int expected_ret);
void recv_byte(int fd, int expected_ret);
void run_tests(const struct test_case *test_cases,
--
2.20.1
From: Stefan Hajnoczi <[email protected]>
See code comment for details.
Signed-off-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/util.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index f838bcee3589..4280a56ba677 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -161,10 +161,24 @@ void run_tests(const struct test_case *test_cases,
printf("%s...", test_cases[i].name);
fflush(stdout);
- if (opts->mode == TEST_MODE_CLIENT)
+ if (opts->mode == TEST_MODE_CLIENT) {
+ /* Full barrier before executing the next test. This
+ * ensures that client and server are executing the
+ * same test case. In particular, it means whoever is
+ * faster will not see the peer still executing the
+ * last test. This is important because port numbers
+ * can be used by multiple test cases.
+ */
+ control_expectln("NEXT");
+ control_writeln("NEXT");
+
run = test_cases[i].run_client;
- else
+ } else {
+ control_writeln("NEXT");
+ control_expectln("NEXT");
+
run = test_cases[i].run_server;
+ }
if (run)
run(opts);
--
2.20.1
From: Stefan Hajnoczi <[email protected]>
Test cases will want to transfer data. This patch adds utility
functions to do this.
Signed-off-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/util.c | 99 ++++++++++++++++++++++++++++++++++++++
tools/testing/vsock/util.h | 2 +
2 files changed, 101 insertions(+)
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 4280a56ba677..d46a6e079b96 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -9,6 +9,7 @@
#include <errno.h>
#include <stdio.h>
+#include <stdint.h>
#include <stdlib.h>
#include <signal.h>
#include <unistd.h>
@@ -149,6 +150,104 @@ int vsock_stream_accept(unsigned int cid, unsigned int port,
return client_fd;
}
+/* Transmit one byte and check the return value.
+ *
+ * expected_ret:
+ * <0 Negative errno (for testing errors)
+ * 0 End-of-file
+ * 1 Success
+ */
+void send_byte(int fd, int expected_ret)
+{
+ const uint8_t byte = 'A';
+ ssize_t nwritten;
+
+ timeout_begin(TIMEOUT);
+ do {
+ nwritten = write(fd, &byte, sizeof(byte));
+ timeout_check("write");
+ } while (nwritten < 0 && errno == EINTR);
+ timeout_end();
+
+ if (expected_ret < 0) {
+ if (nwritten != -1) {
+ fprintf(stderr, "bogus write(2) return value %zd\n",
+ nwritten);
+ exit(EXIT_FAILURE);
+ }
+ if (errno != -expected_ret) {
+ perror("write");
+ exit(EXIT_FAILURE);
+ }
+ return;
+ }
+
+ if (nwritten < 0) {
+ perror("write");
+ exit(EXIT_FAILURE);
+ }
+ if (nwritten == 0) {
+ if (expected_ret == 0)
+ return;
+
+ fprintf(stderr, "unexpected EOF while sending byte\n");
+ exit(EXIT_FAILURE);
+ }
+ if (nwritten != sizeof(byte)) {
+ fprintf(stderr, "bogus write(2) return value %zd\n", nwritten);
+ exit(EXIT_FAILURE);
+ }
+}
+
+/* Receive one byte and check the return value.
+ *
+ * expected_ret:
+ * <0 Negative errno (for testing errors)
+ * 0 End-of-file
+ * 1 Success
+ */
+void recv_byte(int fd, int expected_ret)
+{
+ uint8_t byte;
+ ssize_t nread;
+
+ timeout_begin(TIMEOUT);
+ do {
+ nread = read(fd, &byte, sizeof(byte));
+ timeout_check("read");
+ } while (nread < 0 && errno == EINTR);
+ timeout_end();
+
+ if (expected_ret < 0) {
+ if (nread != -1) {
+ fprintf(stderr, "bogus read(2) return value %zd\n",
+ nread);
+ exit(EXIT_FAILURE);
+ }
+ if (errno != -expected_ret) {
+ perror("read");
+ exit(EXIT_FAILURE);
+ }
+ return;
+ }
+
+ if (nread < 0) {
+ perror("read");
+ exit(EXIT_FAILURE);
+ }
+ if (nread == 0) {
+ if (expected_ret == 0)
+ return;
+
+ fprintf(stderr, "unexpected EOF while receiving byte\n");
+ exit(EXIT_FAILURE);
+ }
+ if (nread != sizeof(byte)) {
+ fprintf(stderr, "bogus read(2) return value %zd\n", nread);
+ exit(EXIT_FAILURE);
+ }
+}
+
/* Run test cases. The program terminates if a failure occurs. */
void run_tests(const struct test_case *test_cases,
const struct test_opts *opts)
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index 1786305cfddd..fe524d393d67 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -36,6 +36,8 @@ unsigned int parse_cid(const char *str);
int vsock_stream_connect(unsigned int cid, unsigned int port);
int vsock_stream_accept(unsigned int cid, unsigned int port,
struct sockaddr_vm *clientaddrp);
+void send_byte(int fd, int expected_ret);
+void recv_byte(int fd, int expected_ret);
void run_tests(const struct test_case *test_cases,
const struct test_opts *opts);
--
2.20.1
> From: Stefano Garzarella <[email protected]>
> Sent: Thursday, August 1, 2019 8:26 AM
>
> The vsock_diag.ko module already has a test suite but the core AF_VSOCK
> functionality has no tests. This patch series adds several test cases that
> exercise AF_VSOCK SOCK_STREAM socket semantics (send/recv,
> connect/accept,
> half-closed connections, simultaneous connections).
>
> Dexuan: Do you think can be useful to test HyperV?
Hi Stefano,
Thanks! This should be useful, though I have to write the Windows host side
code to use the test program(s). :-)
Thanks,
-- Dexuan
From: Stefan Hajnoczi <[email protected]>
Move useful functions into a separate file in preparation for more
vsock test programs.
Signed-off-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
---
v2:
* aligned with the current SPDX [Stefano]
---
tools/testing/vsock/Makefile | 2 +-
tools/testing/vsock/util.c | 66 +++++++++++++++++++
tools/testing/vsock/util.h | 36 +++++++++++
tools/testing/vsock/vsock_diag_test.c | 92 +++++++--------------------
4 files changed, 125 insertions(+), 71 deletions(-)
create mode 100644 tools/testing/vsock/util.c
create mode 100644 tools/testing/vsock/util.h
diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
index d41a4e13960a..a916878a2d8c 100644
--- a/tools/testing/vsock/Makefile
+++ b/tools/testing/vsock/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
all: test
test: vsock_diag_test
-vsock_diag_test: vsock_diag_test.o timeout.o control.o
+vsock_diag_test: vsock_diag_test.o timeout.o control.o util.o
CFLAGS += -g -O2 -Werror -Wall -I. -I../../include -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -D_GNU_SOURCE
.PHONY: all test clean
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
new file mode 100644
index 000000000000..f40f45b36d2f
--- /dev/null
+++ b/tools/testing/vsock/util.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * vsock test utilities
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * Author: Stefan Hajnoczi <[email protected]>
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+
+#include "timeout.h"
+#include "util.h"
+
+/* Install signal handlers */
+void init_signals(void)
+{
+ struct sigaction act = {
+ .sa_handler = sigalrm,
+ };
+
+ sigaction(SIGALRM, &act, NULL);
+ signal(SIGPIPE, SIG_IGN);
+}
+
+/* Parse a CID in string representation */
+unsigned int parse_cid(const char *str)
+{
+ char *endptr = NULL;
+ unsigned long n;
+
+ errno = 0;
+ n = strtoul(str, &endptr, 10);
+ if (errno || *endptr != '\0') {
+ fprintf(stderr, "malformed CID \"%s\"\n", str);
+ exit(EXIT_FAILURE);
+ }
+ return n;
+}
+
+/* Run test cases. The program terminates if a failure occurs. */
+void run_tests(const struct test_case *test_cases,
+ const struct test_opts *opts)
+{
+ int i;
+
+ for (i = 0; test_cases[i].name; i++) {
+ void (*run)(const struct test_opts *opts);
+
+ printf("%s...", test_cases[i].name);
+ fflush(stdout);
+
+ if (opts->mode == TEST_MODE_CLIENT)
+ run = test_cases[i].run_client;
+ else
+ run = test_cases[i].run_server;
+
+ if (run)
+ run(opts);
+
+ printf("ok\n");
+ }
+}
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
new file mode 100644
index 000000000000..033e7d59a42a
--- /dev/null
+++ b/tools/testing/vsock/util.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef UTIL_H
+#define UTIL_H
+
+/* Tests can either run as the client or the server */
+enum test_mode {
+ TEST_MODE_UNSET,
+ TEST_MODE_CLIENT,
+ TEST_MODE_SERVER
+};
+
+/* Test runner options */
+struct test_opts {
+ enum test_mode mode;
+ unsigned int peer_cid;
+};
+
+/* A test case definition. Test functions must print failures to stderr and
+ * terminate with exit(EXIT_FAILURE).
+ */
+struct test_case {
+ const char *name; /* human-readable name */
+
+ /* Called when test mode is TEST_MODE_CLIENT */
+ void (*run_client)(const struct test_opts *opts);
+
+ /* Called when test mode is TEST_MODE_SERVER */
+ void (*run_server)(const struct test_opts *opts);
+};
+
+void init_signals(void);
+unsigned int parse_cid(const char *str);
+void run_tests(const struct test_case *test_cases,
+ const struct test_opts *opts);
+
+#endif /* UTIL_H */
diff --git a/tools/testing/vsock/vsock_diag_test.c b/tools/testing/vsock/vsock_diag_test.c
index fc391e041954..944c8a72eed7 100644
--- a/tools/testing/vsock/vsock_diag_test.c
+++ b/tools/testing/vsock/vsock_diag_test.c
@@ -9,12 +9,10 @@
#include <getopt.h>
#include <stdio.h>
-#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
-#include <signal.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>
@@ -28,12 +26,7 @@
#include "timeout.h"
#include "control.h"
-
-enum test_mode {
- TEST_MODE_UNSET,
- TEST_MODE_CLIENT,
- TEST_MODE_SERVER
-};
+#include "util.h"
/* Per-socket status */
struct vsock_stat {
@@ -334,7 +327,7 @@ static void free_sock_stat(struct list_head *sockets)
free(st);
}
-static void test_no_sockets(unsigned int peer_cid)
+static void test_no_sockets(const struct test_opts *opts)
{
LIST_HEAD(sockets);
@@ -345,7 +338,7 @@ static void test_no_sockets(unsigned int peer_cid)
free_sock_stat(&sockets);
}
-static void test_listen_socket_server(unsigned int peer_cid)
+static void test_listen_socket_server(const struct test_opts *opts)
{
union {
struct sockaddr sa;
@@ -383,7 +376,7 @@ static void test_listen_socket_server(unsigned int peer_cid)
free_sock_stat(&sockets);
}
-static void test_connect_client(unsigned int peer_cid)
+static void test_connect_client(const struct test_opts *opts)
{
union {
struct sockaddr sa;
@@ -392,7 +385,7 @@ static void test_connect_client(unsigned int peer_cid)
.svm = {
.svm_family = AF_VSOCK,
.svm_port = 1234,
- .svm_cid = peer_cid,
+ .svm_cid = opts->peer_cid,
},
};
int fd;
@@ -429,7 +422,7 @@ static void test_connect_client(unsigned int peer_cid)
free_sock_stat(&sockets);
}
-static void test_connect_server(unsigned int peer_cid)
+static void test_connect_server(const struct test_opts *opts)
{
union {
struct sockaddr sa;
@@ -481,9 +474,9 @@ static void test_connect_server(unsigned int peer_cid)
clientaddr.sa.sa_family);
exit(EXIT_FAILURE);
}
- if (clientaddr.svm.svm_cid != peer_cid) {
+ if (clientaddr.svm.svm_cid != opts->peer_cid) {
fprintf(stderr, "expected peer CID %u from accept(2), got %u\n",
- peer_cid, clientaddr.svm.svm_cid);
+ opts->peer_cid, clientaddr.svm.svm_cid);
exit(EXIT_FAILURE);
}
@@ -502,11 +495,7 @@ static void test_connect_server(unsigned int peer_cid)
free_sock_stat(&sockets);
}
-static struct {
- const char *name;
- void (*run_client)(unsigned int peer_cid);
- void (*run_server)(unsigned int peer_cid);
-} test_cases[] = {
+static struct test_case test_cases[] = {
{
.name = "No sockets",
.run_server = test_no_sockets,
@@ -523,30 +512,6 @@ static struct {
{},
};
-static void init_signals(void)
-{
- struct sigaction act = {
- .sa_handler = sigalrm,
- };
-
- sigaction(SIGALRM, &act, NULL);
- signal(SIGPIPE, SIG_IGN);
-}
-
-static unsigned int parse_cid(const char *str)
-{
- char *endptr = NULL;
- unsigned long int n;
-
- errno = 0;
- n = strtoul(str, &endptr, 10);
- if (errno || *endptr != '\0') {
- fprintf(stderr, "malformed CID \"%s\"\n", str);
- exit(EXIT_FAILURE);
- }
- return n;
-}
-
static const char optstring[] = "";
static const struct option longopts[] = {
{
@@ -601,9 +566,10 @@ int main(int argc, char **argv)
{
const char *control_host = NULL;
const char *control_port = NULL;
- int mode = TEST_MODE_UNSET;
- unsigned int peer_cid = VMADDR_CID_ANY;
- int i;
+ struct test_opts opts = {
+ .mode = TEST_MODE_UNSET,
+ .peer_cid = VMADDR_CID_ANY,
+ };
init_signals();
@@ -619,16 +585,16 @@ int main(int argc, char **argv)
break;
case 'm':
if (strcmp(optarg, "client") == 0)
- mode = TEST_MODE_CLIENT;
+ opts.mode = TEST_MODE_CLIENT;
else if (strcmp(optarg, "server") == 0)
- mode = TEST_MODE_SERVER;
+ opts.mode = TEST_MODE_SERVER;
else {
fprintf(stderr, "--mode must be \"client\" or \"server\"\n");
return EXIT_FAILURE;
}
break;
case 'p':
- peer_cid = parse_cid(optarg);
+ opts.peer_cid = parse_cid(optarg);
break;
case 'P':
control_port = optarg;
@@ -641,35 +607,21 @@ int main(int argc, char **argv)
if (!control_port)
usage();
- if (mode == TEST_MODE_UNSET)
+ if (opts.mode == TEST_MODE_UNSET)
usage();
- if (peer_cid == VMADDR_CID_ANY)
+ if (opts.peer_cid == VMADDR_CID_ANY)
usage();
if (!control_host) {
- if (mode != TEST_MODE_SERVER)
+ if (opts.mode != TEST_MODE_SERVER)
usage();
control_host = "0.0.0.0";
}
- control_init(control_host, control_port, mode == TEST_MODE_SERVER);
-
- for (i = 0; test_cases[i].name; i++) {
- void (*run)(unsigned int peer_cid);
+ control_init(control_host, control_port,
+ opts.mode == TEST_MODE_SERVER);
- printf("%s...", test_cases[i].name);
- fflush(stdout);
-
- if (mode == TEST_MODE_CLIENT)
- run = test_cases[i].run_client;
- else
- run = test_cases[i].run_server;
-
- if (run)
- run(peer_cid);
-
- printf("ok\n");
- }
+ run_tests(test_cases, &opts);
control_cleanup();
return EXIT_SUCCESS;
--
2.20.1
From: Stefan Hajnoczi <[email protected]>
The vsock_test.c program runs a test suite of AF_VSOCK test cases.
Signed-off-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
---
v2:
* Drop unnecessary includes [Stefan]
* Aligned with the current SPDX [Stefano]
* Set MULTICONN_NFDS to 100 [Stefano]
* Change (i % 1) in (i % 2) in the 'multiconn' test [Stefano]
---
tools/testing/vsock/.gitignore | 1 +
tools/testing/vsock/Makefile | 5 +-
tools/testing/vsock/README | 1 +
tools/testing/vsock/vsock_test.c | 312 +++++++++++++++++++++++++++++++
4 files changed, 317 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/vsock/vsock_test.c
diff --git a/tools/testing/vsock/.gitignore b/tools/testing/vsock/.gitignore
index dc5f11faf530..7f7a2ccc30c4 100644
--- a/tools/testing/vsock/.gitignore
+++ b/tools/testing/vsock/.gitignore
@@ -1,2 +1,3 @@
*.d
+vsock_test
vsock_diag_test
diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
index a916878a2d8c..f8293c6910c9 100644
--- a/tools/testing/vsock/Makefile
+++ b/tools/testing/vsock/Makefile
@@ -1,10 +1,11 @@
# SPDX-License-Identifier: GPL-2.0-only
all: test
-test: vsock_diag_test
+test: vsock_test vsock_diag_test
+vsock_test: vsock_test.o timeout.o control.o util.o
vsock_diag_test: vsock_diag_test.o timeout.o control.o util.o
CFLAGS += -g -O2 -Werror -Wall -I. -I../../include -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -D_GNU_SOURCE
.PHONY: all test clean
clean:
- ${RM} *.o *.d vsock_diag_test
+ ${RM} *.o *.d vsock_test vsock_diag_test
-include *.d
diff --git a/tools/testing/vsock/README b/tools/testing/vsock/README
index cf7dc64273bf..4d5045e7d2c3 100644
--- a/tools/testing/vsock/README
+++ b/tools/testing/vsock/README
@@ -5,6 +5,7 @@ Hyper-V.
The following tests are available:
+ * vsock_test - core AF_VSOCK socket functionality
* vsock_diag_test - vsock_diag.ko module for listing open sockets
The following prerequisite steps are not automated and must be performed prior
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
new file mode 100644
index 000000000000..06099d037405
--- /dev/null
+++ b/tools/testing/vsock/vsock_test.c
@@ -0,0 +1,312 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * vsock_test - vsock.ko test suite
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * Author: Stefan Hajnoczi <[email protected]>
+ */
+
+#include <getopt.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+
+#include "timeout.h"
+#include "control.h"
+#include "util.h"
+
+static void test_stream_connection_reset(const struct test_opts *opts)
+{
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } addr = {
+ .svm = {
+ .svm_family = AF_VSOCK,
+ .svm_port = 1234,
+ .svm_cid = opts->peer_cid,
+ },
+ };
+ int ret;
+ int fd;
+
+ fd = socket(AF_VSOCK, SOCK_STREAM, 0);
+
+ timeout_begin(TIMEOUT);
+ do {
+ ret = connect(fd, &addr.sa, sizeof(addr.svm));
+ timeout_check("connect");
+ } while (ret < 0 && errno == EINTR);
+ timeout_end();
+
+ if (ret != -1) {
+ fprintf(stderr, "expected connect(2) failure, got %d\n", ret);
+ exit(EXIT_FAILURE);
+ }
+ if (errno != ECONNRESET) {
+ fprintf(stderr, "unexpected connect(2) errno %d\n", errno);
+ exit(EXIT_FAILURE);
+ }
+
+ close(fd);
+}
+
+static void test_stream_client_close_client(const struct test_opts *opts)
+{
+ int fd;
+
+ fd = vsock_stream_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ send_byte(fd, 1);
+ close(fd);
+ control_writeln("CLOSED");
+}
+
+static void test_stream_client_close_server(const struct test_opts *opts)
+{
+ int fd;
+
+ fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ control_expectln("CLOSED");
+
+ send_byte(fd, -EPIPE);
+ recv_byte(fd, 1);
+ recv_byte(fd, 0);
+ close(fd);
+}
+
+static void test_stream_server_close_client(const struct test_opts *opts)
+{
+ int fd;
+
+ fd = vsock_stream_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ control_expectln("CLOSED");
+
+ send_byte(fd, -EPIPE);
+ recv_byte(fd, 1);
+ recv_byte(fd, 0);
+ close(fd);
+}
+
+static void test_stream_server_close_server(const struct test_opts *opts)
+{
+ int fd;
+
+ fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ send_byte(fd, 1);
+ close(fd);
+ control_writeln("CLOSED");
+}
+
+/* With the standard socket sizes, VMCI is able to support about 100
+ * concurrent stream connections.
+ */
+#define MULTICONN_NFDS 100
+
+static void test_stream_multiconn_client(const struct test_opts *opts)
+{
+ int fds[MULTICONN_NFDS];
+ int i;
+
+ for (i = 0; i < MULTICONN_NFDS; i++) {
+ fds[i] = vsock_stream_connect(opts->peer_cid, 1234);
+ if (fds[i] < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ for (i = 0; i < MULTICONN_NFDS; i++) {
+ if (i % 2)
+ recv_byte(fds[i], 1);
+ else
+ send_byte(fds[i], 1);
+ }
+
+ for (i = 0; i < MULTICONN_NFDS; i++)
+ close(fds[i]);
+}
+
+static void test_stream_multiconn_server(const struct test_opts *opts)
+{
+ int fds[MULTICONN_NFDS];
+ int i;
+
+ for (i = 0; i < MULTICONN_NFDS; i++) {
+ fds[i] = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fds[i] < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ for (i = 0; i < MULTICONN_NFDS; i++) {
+ if (i % 2)
+ send_byte(fds[i], 1);
+ else
+ recv_byte(fds[i], 1);
+ }
+
+ for (i = 0; i < MULTICONN_NFDS; i++)
+ close(fds[i]);
+}
+
+static struct test_case test_cases[] = {
+ {
+ .name = "SOCK_STREAM connection reset",
+ .run_client = test_stream_connection_reset,
+ },
+ {
+ .name = "SOCK_STREAM client close",
+ .run_client = test_stream_client_close_client,
+ .run_server = test_stream_client_close_server,
+ },
+ {
+ .name = "SOCK_STREAM server close",
+ .run_client = test_stream_server_close_client,
+ .run_server = test_stream_server_close_server,
+ },
+ {
+ .name = "SOCK_STREAM multiple connections",
+ .run_client = test_stream_multiconn_client,
+ .run_server = test_stream_multiconn_server,
+ },
+ {},
+};
+
+static const char optstring[] = "";
+static const struct option longopts[] = {
+ {
+ .name = "control-host",
+ .has_arg = required_argument,
+ .val = 'H',
+ },
+ {
+ .name = "control-port",
+ .has_arg = required_argument,
+ .val = 'P',
+ },
+ {
+ .name = "mode",
+ .has_arg = required_argument,
+ .val = 'm',
+ },
+ {
+ .name = "peer-cid",
+ .has_arg = required_argument,
+ .val = 'p',
+ },
+ {
+ .name = "help",
+ .has_arg = no_argument,
+ .val = '?',
+ },
+ {},
+};
+
+static void usage(void)
+{
+ fprintf(stderr, "Usage: vsock_test [--help] [--control-host=<host>] --control-port=<port> --mode=client|server --peer-cid=<cid>\n"
+ "\n"
+ " Server: vsock_test --control-port=1234 --mode=server --peer-cid=3\n"
+ " Client: vsock_test --control-host=192.168.0.1 --control-port=1234 --mode=client --peer-cid=2\n"
+ "\n"
+ "Run vsock.ko tests. Must be launched in both guest\n"
+ "and host. One side must use --mode=client and\n"
+ "the other side must use --mode=server.\n"
+ "\n"
+ "A TCP control socket connection is used to coordinate tests\n"
+ "between the client and the server. The server requires a\n"
+ "listen address and the client requires an address to\n"
+ "connect to.\n"
+ "\n"
+ "The CID of the other side must be given with --peer-cid=<cid>.\n");
+ exit(EXIT_FAILURE);
+}
+
+int main(int argc, char **argv)
+{
+ const char *control_host = NULL;
+ const char *control_port = NULL;
+ struct test_opts opts = {
+ .mode = TEST_MODE_UNSET,
+ .peer_cid = VMADDR_CID_ANY,
+ };
+
+ init_signals();
+
+ for (;;) {
+ int opt = getopt_long(argc, argv, optstring, longopts, NULL);
+
+ if (opt == -1)
+ break;
+
+ switch (opt) {
+ case 'H':
+ control_host = optarg;
+ break;
+ case 'm':
+ if (strcmp(optarg, "client") == 0)
+ opts.mode = TEST_MODE_CLIENT;
+ else if (strcmp(optarg, "server") == 0)
+ opts.mode = TEST_MODE_SERVER;
+ else {
+ fprintf(stderr, "--mode must be \"client\" or \"server\"\n");
+ return EXIT_FAILURE;
+ }
+ break;
+ case 'p':
+ opts.peer_cid = parse_cid(optarg);
+ break;
+ case 'P':
+ control_port = optarg;
+ break;
+ case '?':
+ default:
+ usage();
+ }
+ }
+
+ if (!control_port)
+ usage();
+ if (opts.mode == TEST_MODE_UNSET)
+ usage();
+ if (opts.peer_cid == VMADDR_CID_ANY)
+ usage();
+
+ if (!control_host) {
+ if (opts.mode != TEST_MODE_SERVER)
+ usage();
+ control_host = "0.0.0.0";
+ }
+
+ control_init(control_host, control_port,
+ opts.mode == TEST_MODE_SERVER);
+
+ run_tests(test_cases, &opts);
+
+ control_cleanup();
+ return EXIT_SUCCESS;
+}
--
2.20.1
Add new --transport parameter to skip some tests or checks
not supported by a specific transport.
Suggested-by: Jorgen Hansen <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/util.h | 8 ++++++++
tools/testing/vsock/vsock_test.c | 20 +++++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index 379e02ab59bb..7fdb8100f035 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -12,9 +12,17 @@ enum test_mode {
TEST_MODE_SERVER
};
+enum test_transport {
+ TEST_TRANSPORT_UNSET,
+ TEST_TRANSPORT_VMCI,
+ TEST_TRANSPORT_VIRTIO,
+ TEST_TRANSPORT_HYPERV
+};
+
/* Test runner options */
struct test_opts {
enum test_mode mode;
+ enum test_transport transport;
unsigned int peer_cid;
};
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 06099d037405..cb606091489f 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -218,6 +218,11 @@ static const struct option longopts[] = {
.has_arg = required_argument,
.val = 'p',
},
+ {
+ .name = "transport",
+ .has_arg = required_argument,
+ .val = 't',
+ },
{
.name = "help",
.has_arg = no_argument,
@@ -228,7 +233,7 @@ static const struct option longopts[] = {
static void usage(void)
{
- fprintf(stderr, "Usage: vsock_test [--help] [--control-host=<host>] --control-port=<port> --mode=client|server --peer-cid=<cid>\n"
+ fprintf(stderr, "Usage: vsock_test [--help] [--control-host=<host>] --control-port=<port> --mode=client|server --peer-cid=<cid> [--transport=vmci|virtio|hyperv]\n"
"\n"
" Server: vsock_test --control-port=1234 --mode=server --peer-cid=3\n"
" Client: vsock_test --control-host=192.168.0.1 --control-port=1234 --mode=client --peer-cid=2\n"
@@ -252,6 +257,7 @@ int main(int argc, char **argv)
const char *control_port = NULL;
struct test_opts opts = {
.mode = TEST_MODE_UNSET,
+ .transport = TEST_TRANSPORT_UNSET,
.peer_cid = VMADDR_CID_ANY,
};
@@ -283,6 +289,18 @@ int main(int argc, char **argv)
case 'P':
control_port = optarg;
break;
+ case 't':
+ if (strcmp(optarg, "vmci") == 0)
+ opts.transport = TEST_TRANSPORT_VMCI;
+ else if (strcmp(optarg, "virtio") == 0)
+ opts.transport = TEST_TRANSPORT_VIRTIO;
+ else if (strcmp(optarg, "hyperv") == 0)
+ opts.transport = TEST_TRANSPORT_HYPERV;
+ else {
+ fprintf(stderr, "--transport must be \"vmci\" or \"virtio\" or \"hyperv\"\n");
+ return EXIT_FAILURE;
+ }
+ break;
case '?':
default:
usage();
--
2.20.1
When VMCI transport is used, if the guest closes a connection,
all data is gone and EOF is returned, so we should skip the read
of data written by the peer before closing the connection.
Reported-by: Jorgen Hansen <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index cb606091489f..64adf45501ca 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -71,6 +71,7 @@ static void test_stream_client_close_client(const struct test_opts *opts)
static void test_stream_client_close_server(const struct test_opts *opts)
{
+ unsigned int local_cid;
int fd;
fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
@@ -79,16 +80,27 @@ static void test_stream_client_close_server(const struct test_opts *opts)
exit(EXIT_FAILURE);
}
+ local_cid = vsock_get_local_cid(fd);
+
control_expectln("CLOSED");
send_byte(fd, -EPIPE);
- recv_byte(fd, 1);
+
+ /* Skip the read of data wrote by the peer if we are on VMCI and
+ * we are on the host side, because when the guest closes a
+ * connection, all data is gone and EOF is returned.
+ */
+ if (!(opts->transport == TEST_TRANSPORT_VMCI &&
+ local_cid == VMADDR_CID_HOST))
+ recv_byte(fd, 1);
+
recv_byte(fd, 0);
close(fd);
}
static void test_stream_server_close_client(const struct test_opts *opts)
{
+ unsigned int local_cid;
int fd;
fd = vsock_stream_connect(opts->peer_cid, 1234);
@@ -97,10 +109,20 @@ static void test_stream_server_close_client(const struct test_opts *opts)
exit(EXIT_FAILURE);
}
+ local_cid = vsock_get_local_cid(fd);
+
control_expectln("CLOSED");
send_byte(fd, -EPIPE);
- recv_byte(fd, 1);
+
+ /* Skip the read of data wrote by the peer if we are on VMCI and
+ * we are on the host side, because when the guest closes a
+ * connection, all data is gone and EOF is returned.
+ */
+ if (!(opts->transport == TEST_TRANSPORT_VMCI &&
+ local_cid == VMADDR_CID_HOST))
+ recv_byte(fd, 1);
+
recv_byte(fd, 0);
close(fd);
}
--
2.20.1
Hello!
On 08/01/2019 06:25 PM, Stefano Garzarella wrote:
> When VMCI transport is used, if the guest closes a connection,
> all data is gone and EOF is returned, so we should skip the read
> of data written by the peer before closing the connection.
>
> Reported-by: Jorgen Hansen <[email protected]>
> Signed-off-by: Stefano Garzarella <[email protected]>
> ---
> tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index cb606091489f..64adf45501ca 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
[...]
> @@ -79,16 +80,27 @@ static void test_stream_client_close_server(const struct test_opts *opts)
> exit(EXIT_FAILURE);
> }
>
> + local_cid = vsock_get_local_cid(fd);
> +
> control_expectln("CLOSED");
>
> send_byte(fd, -EPIPE);
> - recv_byte(fd, 1);
> +
> + /* Skip the read of data wrote by the peer if we are on VMCI and
s/wrote/written/?
> + * we are on the host side, because when the guest closes a
> + * connection, all data is gone and EOF is returned.
> + */
> + if (!(opts->transport == TEST_TRANSPORT_VMCI &&
> + local_cid == VMADDR_CID_HOST))
> + recv_byte(fd, 1);
> +
> recv_byte(fd, 0);
> close(fd);
> }
[...]
MBR, Sergei
On Thu, Aug 01, 2019 at 06:53:32PM +0300, Sergei Shtylyov wrote:
> Hello!
>
Hi :)
> On 08/01/2019 06:25 PM, Stefano Garzarella wrote:
>
> > When VMCI transport is used, if the guest closes a connection,
> > all data is gone and EOF is returned, so we should skip the read
> > of data written by the peer before closing the connection.
> >
> > Reported-by: Jorgen Hansen <[email protected]>
> > Signed-off-by: Stefano Garzarella <[email protected]>
> > ---
> > tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++++++++++--
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> > index cb606091489f..64adf45501ca 100644
> > --- a/tools/testing/vsock/vsock_test.c
> > +++ b/tools/testing/vsock/vsock_test.c
> [...]
> > @@ -79,16 +80,27 @@ static void test_stream_client_close_server(const struct test_opts *opts)
> > exit(EXIT_FAILURE);
> > }
> >
> > + local_cid = vsock_get_local_cid(fd);
> > +
> > control_expectln("CLOSED");
> >
> > send_byte(fd, -EPIPE);
> > - recv_byte(fd, 1);
> > +
> > + /* Skip the read of data wrote by the peer if we are on VMCI and
>
> s/wrote/written/?
>
Thanks, I'll fix it!
Stefano
> > + * we are on the host side, because when the guest closes a
> > + * connection, all data is gone and EOF is returned.
> > + */
> > + if (!(opts->transport == TEST_TRANSPORT_VMCI &&
> > + local_cid == VMADDR_CID_HOST))
> > + recv_byte(fd, 1);
> > +
> > recv_byte(fd, 0);
> > close(fd);
> > }
> [...]
>
> MBR, Sergei
--
On Thu, Aug 01, 2019 at 04:16:37PM +0000, Dexuan Cui wrote:
> > From: Stefano Garzarella <[email protected]>
> > Sent: Thursday, August 1, 2019 8:26 AM
> >
> > The vsock_diag.ko module already has a test suite but the core AF_VSOCK
> > functionality has no tests. This patch series adds several test cases that
> > exercise AF_VSOCK SOCK_STREAM socket semantics (send/recv,
> > connect/accept,
> > half-closed connections, simultaneous connections).
> >
> > Dexuan: Do you think can be useful to test HyperV?
>
> Hi Stefano,
> Thanks! This should be useful, though I have to write the Windows host side
> code to use the test program(s). :-)
>
Oh, yeah, I thought so :-)
Let me know when you'll try to find out if there's a problem.
Thanks,
Stefano
On Thu, Aug 01, 2019 at 05:25:41PM +0200, Stefano Garzarella wrote:
> +/* Wait for the remote to close the connection */
> +void vsock_wait_remote_close(int fd)
> +{
> + struct epoll_event ev;
> + int epollfd, nfds;
> +
> + epollfd = epoll_create1(0);
> + if (epollfd == -1) {
> + perror("epoll_create1");
> + exit(EXIT_FAILURE);
> + }
> +
> + ev.events = EPOLLRDHUP | EPOLLHUP;
> + ev.data.fd = fd;
> + if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
> + perror("epoll_ctl");
> + exit(EXIT_FAILURE);
> + }
> +
> + nfds = epoll_wait(epollfd, &ev, 1, TIMEOUT * 1000);
> + if (nfds == -1) {
> + perror("epoll_wait");
> + exit(EXIT_FAILURE);
> + }
> +
> + if (nfds == 0) {
> + fprintf(stderr, "epoll_wait timed out\n");
> + exit(EXIT_FAILURE);
> + }
> +
> + assert(nfds == 1);
> + assert(ev.events & (EPOLLRDHUP | EPOLLHUP));
> + assert(ev.data.fd == fd);
> +
> + close(epollfd);
> +}
Please use timeout_begin()/timeout_end() so that the test cannot hang.
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 64adf45501ca..a664675bec5a 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -84,6 +84,11 @@ static void test_stream_client_close_server(const struct test_opts *opts)
>
> control_expectln("CLOSED");
>
> + /* Wait for the remote to close the connection, before check
> + * -EPIPE error on send.
> + */
> + vsock_wait_remote_close(fd);
Is control_expectln("CLOSED") still necessary now that we're waiting for
the poll event? The control message was an attempt to wait until the
other side closed the socket.
On Thu, Aug 01, 2019 at 05:25:40PM +0200, Stefano Garzarella wrote:
> When VMCI transport is used, if the guest closes a connection,
> all data is gone and EOF is returned, so we should skip the read
> of data written by the peer before closing the connection.
All transports should aim for identical semantics. I think virtio-vsock
should behave the same as VMCI since userspace applications should be
transport-independent.
Let's view this as a vsock bug. Is it feasible to change the VMCI
behavior so it's more like TCP sockets? If not, let's change the
virtio-vsock behavior to be compatible with VMCI.
On Tue, Aug 20, 2019 at 09:28:28AM +0100, Stefan Hajnoczi wrote:
> On Thu, Aug 01, 2019 at 05:25:41PM +0200, Stefano Garzarella wrote:
> > +/* Wait for the remote to close the connection */
> > +void vsock_wait_remote_close(int fd)
> > +{
> > + struct epoll_event ev;
> > + int epollfd, nfds;
> > +
> > + epollfd = epoll_create1(0);
> > + if (epollfd == -1) {
> > + perror("epoll_create1");
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + ev.events = EPOLLRDHUP | EPOLLHUP;
> > + ev.data.fd = fd;
> > + if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
> > + perror("epoll_ctl");
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + nfds = epoll_wait(epollfd, &ev, 1, TIMEOUT * 1000);
> > + if (nfds == -1) {
> > + perror("epoll_wait");
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + if (nfds == 0) {
> > + fprintf(stderr, "epoll_wait timed out\n");
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + assert(nfds == 1);
> > + assert(ev.events & (EPOLLRDHUP | EPOLLHUP));
> > + assert(ev.data.fd == fd);
> > +
> > + close(epollfd);
> > +}
>
> Please use timeout_begin()/timeout_end() so that the test cannot hang.
>
I used the TIMEOUT macro in the epoll_wait() to avoid the hang.
Do you think is better to use the timeout_begin()/timeout_end()?
In this case, should I remove the timeout in the epoll_wait()?
> > diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> > index 64adf45501ca..a664675bec5a 100644
> > --- a/tools/testing/vsock/vsock_test.c
> > +++ b/tools/testing/vsock/vsock_test.c
> > @@ -84,6 +84,11 @@ static void test_stream_client_close_server(const struct test_opts *opts)
> >
> > control_expectln("CLOSED");
> >
> > + /* Wait for the remote to close the connection, before check
> > + * -EPIPE error on send.
> > + */
> > + vsock_wait_remote_close(fd);
>
> Is control_expectln("CLOSED") still necessary now that we're waiting for
> the poll event? The control message was an attempt to wait until the
> other side closed the socket.
Right, I'll remove it in the v3
Thanks,
Stefano
On Tue, Aug 20, 2019 at 09:32:03AM +0100, Stefan Hajnoczi wrote:
> On Thu, Aug 01, 2019 at 05:25:40PM +0200, Stefano Garzarella wrote:
> > When VMCI transport is used, if the guest closes a connection,
> > all data is gone and EOF is returned, so we should skip the read
> > of data written by the peer before closing the connection.
>
> All transports should aim for identical semantics. I think virtio-vsock
> should behave the same as VMCI since userspace applications should be
> transport-independent.
Yes, it is a good point!
>
> Let's view this as a vsock bug. Is it feasible to change the VMCI
> behavior so it's more like TCP sockets? If not, let's change the
> virtio-vsock behavior to be compatible with VMCI.
I'm not sure it is feasible to change the VMCI behavior. IIUC reading the
Jorgen's answer [1], this was a decision made during the implementation.
@Jorgen: please, can you confirm? or not :-)
If it is the case, I'll change virtio-vsock to the same behavior.
Thanks,
Stefano
[1] https://patchwork.ozlabs.org/cover/847998/#1831400
On Thu, Aug 22, 2019 at 11:15:46AM +0200, Stefano Garzarella wrote:
> On Tue, Aug 20, 2019 at 09:28:28AM +0100, Stefan Hajnoczi wrote:
> > On Thu, Aug 01, 2019 at 05:25:41PM +0200, Stefano Garzarella wrote:
> > > +/* Wait for the remote to close the connection */
> > > +void vsock_wait_remote_close(int fd)
> > > +{
> > > + struct epoll_event ev;
> > > + int epollfd, nfds;
> > > +
> > > + epollfd = epoll_create1(0);
> > > + if (epollfd == -1) {
> > > + perror("epoll_create1");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + ev.events = EPOLLRDHUP | EPOLLHUP;
> > > + ev.data.fd = fd;
> > > + if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
> > > + perror("epoll_ctl");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + nfds = epoll_wait(epollfd, &ev, 1, TIMEOUT * 1000);
> > > + if (nfds == -1) {
> > > + perror("epoll_wait");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + if (nfds == 0) {
> > > + fprintf(stderr, "epoll_wait timed out\n");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + assert(nfds == 1);
> > > + assert(ev.events & (EPOLLRDHUP | EPOLLHUP));
> > > + assert(ev.data.fd == fd);
> > > +
> > > + close(epollfd);
> > > +}
> >
> > Please use timeout_begin()/timeout_end() so that the test cannot hang.
> >
>
> I used the TIMEOUT macro in the epoll_wait() to avoid the hang.
> Do you think is better to use the timeout_begin()/timeout_end()?
> In this case, should I remove the timeout in the epoll_wait()?
Oops, you're right. There are no other blocking calls in this function
so the existing patch is fine.
Thanks,
Stefan
Hi Stefan,
I'm thinking about dividing this test into single applications, one
for each test, do you think it makes sense?
Or is it just a useless complication?
Thanks,
Stefano
On Thu, Aug 1, 2019 at 5:27 PM Stefano Garzarella <[email protected]> wrote:
>
> From: Stefan Hajnoczi <[email protected]>
>
> The vsock_test.c program runs a test suite of AF_VSOCK test cases.
>
> Signed-off-by: Stefan Hajnoczi <[email protected]>
> Signed-off-by: Stefano Garzarella <[email protected]>
> ---
> v2:
> * Drop unnecessary includes [Stefan]
> * Aligned with the current SPDX [Stefano]
> * Set MULTICONN_NFDS to 100 [Stefano]
> * Change (i % 1) in (i % 2) in the 'multiconn' test [Stefano]
> ---
> tools/testing/vsock/.gitignore | 1 +
> tools/testing/vsock/Makefile | 5 +-
> tools/testing/vsock/README | 1 +
> tools/testing/vsock/vsock_test.c | 312 +++++++++++++++++++++++++++++++
> 4 files changed, 317 insertions(+), 2 deletions(-)
> create mode 100644 tools/testing/vsock/vsock_test.c
>
> diff --git a/tools/testing/vsock/.gitignore b/tools/testing/vsock/.gitignore
> index dc5f11faf530..7f7a2ccc30c4 100644
> --- a/tools/testing/vsock/.gitignore
> +++ b/tools/testing/vsock/.gitignore
> @@ -1,2 +1,3 @@
> *.d
> +vsock_test
> vsock_diag_test
> diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
> index a916878a2d8c..f8293c6910c9 100644
> --- a/tools/testing/vsock/Makefile
> +++ b/tools/testing/vsock/Makefile
> @@ -1,10 +1,11 @@
> # SPDX-License-Identifier: GPL-2.0-only
> all: test
> -test: vsock_diag_test
> +test: vsock_test vsock_diag_test
> +vsock_test: vsock_test.o timeout.o control.o util.o
> vsock_diag_test: vsock_diag_test.o timeout.o control.o util.o
>
> CFLAGS += -g -O2 -Werror -Wall -I. -I../../include -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -D_GNU_SOURCE
> .PHONY: all test clean
> clean:
> - ${RM} *.o *.d vsock_diag_test
> + ${RM} *.o *.d vsock_test vsock_diag_test
> -include *.d
> diff --git a/tools/testing/vsock/README b/tools/testing/vsock/README
> index cf7dc64273bf..4d5045e7d2c3 100644
> --- a/tools/testing/vsock/README
> +++ b/tools/testing/vsock/README
> @@ -5,6 +5,7 @@ Hyper-V.
>
> The following tests are available:
>
> + * vsock_test - core AF_VSOCK socket functionality
> * vsock_diag_test - vsock_diag.ko module for listing open sockets
>
> The following prerequisite steps are not automated and must be performed prior
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> new file mode 100644
> index 000000000000..06099d037405
> --- /dev/null
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * vsock_test - vsock.ko test suite
> + *
> + * Copyright (C) 2017 Red Hat, Inc.
> + *
> + * Author: Stefan Hajnoczi <[email protected]>
> + */
> +
> +#include <getopt.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <unistd.h>
> +
> +#include "timeout.h"
> +#include "control.h"
> +#include "util.h"
> +
> +static void test_stream_connection_reset(const struct test_opts *opts)
> +{
> + union {
> + struct sockaddr sa;
> + struct sockaddr_vm svm;
> + } addr = {
> + .svm = {
> + .svm_family = AF_VSOCK,
> + .svm_port = 1234,
> + .svm_cid = opts->peer_cid,
> + },
> + };
> + int ret;
> + int fd;
> +
> + fd = socket(AF_VSOCK, SOCK_STREAM, 0);
> +
> + timeout_begin(TIMEOUT);
> + do {
> + ret = connect(fd, &addr.sa, sizeof(addr.svm));
> + timeout_check("connect");
> + } while (ret < 0 && errno == EINTR);
> + timeout_end();
> +
> + if (ret != -1) {
> + fprintf(stderr, "expected connect(2) failure, got %d\n", ret);
> + exit(EXIT_FAILURE);
> + }
> + if (errno != ECONNRESET) {
> + fprintf(stderr, "unexpected connect(2) errno %d\n", errno);
> + exit(EXIT_FAILURE);
> + }
> +
> + close(fd);
> +}
> +
> +static void test_stream_client_close_client(const struct test_opts *opts)
> +{
> + int fd;
> +
> + fd = vsock_stream_connect(opts->peer_cid, 1234);
> + if (fd < 0) {
> + perror("connect");
> + exit(EXIT_FAILURE);
> + }
> +
> + send_byte(fd, 1);
> + close(fd);
> + control_writeln("CLOSED");
> +}
> +
> +static void test_stream_client_close_server(const struct test_opts *opts)
> +{
> + int fd;
> +
> + fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
> + if (fd < 0) {
> + perror("accept");
> + exit(EXIT_FAILURE);
> + }
> +
> + control_expectln("CLOSED");
> +
> + send_byte(fd, -EPIPE);
> + recv_byte(fd, 1);
> + recv_byte(fd, 0);
> + close(fd);
> +}
> +
> +static void test_stream_server_close_client(const struct test_opts *opts)
> +{
> + int fd;
> +
> + fd = vsock_stream_connect(opts->peer_cid, 1234);
> + if (fd < 0) {
> + perror("connect");
> + exit(EXIT_FAILURE);
> + }
> +
> + control_expectln("CLOSED");
> +
> + send_byte(fd, -EPIPE);
> + recv_byte(fd, 1);
> + recv_byte(fd, 0);
> + close(fd);
> +}
> +
> +static void test_stream_server_close_server(const struct test_opts *opts)
> +{
> + int fd;
> +
> + fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
> + if (fd < 0) {
> + perror("accept");
> + exit(EXIT_FAILURE);
> + }
> +
> + send_byte(fd, 1);
> + close(fd);
> + control_writeln("CLOSED");
> +}
> +
> +/* With the standard socket sizes, VMCI is able to support about 100
> + * concurrent stream connections.
> + */
> +#define MULTICONN_NFDS 100
> +
> +static void test_stream_multiconn_client(const struct test_opts *opts)
> +{
> + int fds[MULTICONN_NFDS];
> + int i;
> +
> + for (i = 0; i < MULTICONN_NFDS; i++) {
> + fds[i] = vsock_stream_connect(opts->peer_cid, 1234);
> + if (fds[i] < 0) {
> + perror("connect");
> + exit(EXIT_FAILURE);
> + }
> + }
> +
> + for (i = 0; i < MULTICONN_NFDS; i++) {
> + if (i % 2)
> + recv_byte(fds[i], 1);
> + else
> + send_byte(fds[i], 1);
> + }
> +
> + for (i = 0; i < MULTICONN_NFDS; i++)
> + close(fds[i]);
> +}
> +
> +static void test_stream_multiconn_server(const struct test_opts *opts)
> +{
> + int fds[MULTICONN_NFDS];
> + int i;
> +
> + for (i = 0; i < MULTICONN_NFDS; i++) {
> + fds[i] = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
> + if (fds[i] < 0) {
> + perror("accept");
> + exit(EXIT_FAILURE);
> + }
> + }
> +
> + for (i = 0; i < MULTICONN_NFDS; i++) {
> + if (i % 2)
> + send_byte(fds[i], 1);
> + else
> + recv_byte(fds[i], 1);
> + }
> +
> + for (i = 0; i < MULTICONN_NFDS; i++)
> + close(fds[i]);
> +}
> +
> +static struct test_case test_cases[] = {
> + {
> + .name = "SOCK_STREAM connection reset",
> + .run_client = test_stream_connection_reset,
> + },
> + {
> + .name = "SOCK_STREAM client close",
> + .run_client = test_stream_client_close_client,
> + .run_server = test_stream_client_close_server,
> + },
> + {
> + .name = "SOCK_STREAM server close",
> + .run_client = test_stream_server_close_client,
> + .run_server = test_stream_server_close_server,
> + },
> + {
> + .name = "SOCK_STREAM multiple connections",
> + .run_client = test_stream_multiconn_client,
> + .run_server = test_stream_multiconn_server,
> + },
> + {},
> +};
> +
> +static const char optstring[] = "";
> +static const struct option longopts[] = {
> + {
> + .name = "control-host",
> + .has_arg = required_argument,
> + .val = 'H',
> + },
> + {
> + .name = "control-port",
> + .has_arg = required_argument,
> + .val = 'P',
> + },
> + {
> + .name = "mode",
> + .has_arg = required_argument,
> + .val = 'm',
> + },
> + {
> + .name = "peer-cid",
> + .has_arg = required_argument,
> + .val = 'p',
> + },
> + {
> + .name = "help",
> + .has_arg = no_argument,
> + .val = '?',
> + },
> + {},
> +};
> +
> +static void usage(void)
> +{
> + fprintf(stderr, "Usage: vsock_test [--help] [--control-host=<host>] --control-port=<port> --mode=client|server --peer-cid=<cid>\n"
> + "\n"
> + " Server: vsock_test --control-port=1234 --mode=server --peer-cid=3\n"
> + " Client: vsock_test --control-host=192.168.0.1 --control-port=1234 --mode=client --peer-cid=2\n"
> + "\n"
> + "Run vsock.ko tests. Must be launched in both guest\n"
> + "and host. One side must use --mode=client and\n"
> + "the other side must use --mode=server.\n"
> + "\n"
> + "A TCP control socket connection is used to coordinate tests\n"
> + "between the client and the server. The server requires a\n"
> + "listen address and the client requires an address to\n"
> + "connect to.\n"
> + "\n"
> + "The CID of the other side must be given with --peer-cid=<cid>.\n");
> + exit(EXIT_FAILURE);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + const char *control_host = NULL;
> + const char *control_port = NULL;
> + struct test_opts opts = {
> + .mode = TEST_MODE_UNSET,
> + .peer_cid = VMADDR_CID_ANY,
> + };
> +
> + init_signals();
> +
> + for (;;) {
> + int opt = getopt_long(argc, argv, optstring, longopts, NULL);
> +
> + if (opt == -1)
> + break;
> +
> + switch (opt) {
> + case 'H':
> + control_host = optarg;
> + break;
> + case 'm':
> + if (strcmp(optarg, "client") == 0)
> + opts.mode = TEST_MODE_CLIENT;
> + else if (strcmp(optarg, "server") == 0)
> + opts.mode = TEST_MODE_SERVER;
> + else {
> + fprintf(stderr, "--mode must be \"client\" or \"server\"\n");
> + return EXIT_FAILURE;
> + }
> + break;
> + case 'p':
> + opts.peer_cid = parse_cid(optarg);
> + break;
> + case 'P':
> + control_port = optarg;
> + break;
> + case '?':
> + default:
> + usage();
> + }
> + }
> +
> + if (!control_port)
> + usage();
> + if (opts.mode == TEST_MODE_UNSET)
> + usage();
> + if (opts.peer_cid == VMADDR_CID_ANY)
> + usage();
> +
> + if (!control_host) {
> + if (opts.mode != TEST_MODE_SERVER)
> + usage();
> + control_host = "0.0.0.0";
> + }
> +
> + control_init(control_host, control_port,
> + opts.mode == TEST_MODE_SERVER);
> +
> + run_tests(test_cases, &opts);
> +
> + control_cleanup();
> + return EXIT_SUCCESS;
> +}
> --
> 2.20.1
On Wed, Oct 09, 2019 at 12:03:53PM +0200, Stefano Garzarella wrote:
> Hi Stefan,
> I'm thinking about dividing this test into single applications, one
> for each test, do you think it makes sense?
> Or is it just a useless complication?
I don't mind either way but personally I would leave it as a single
program.
Stefan
On Wed, Oct 09, 2019 at 04:15:03PM +0100, Stefan Hajnoczi wrote:
> On Wed, Oct 09, 2019 at 12:03:53PM +0200, Stefano Garzarella wrote:
> > Hi Stefan,
> > I'm thinking about dividing this test into single applications, one
> > for each test, do you think it makes sense?
> > Or is it just a useless complication?
>
> I don't mind either way but personally I would leave it as a single
> program.
>
Okay, since I had the doubt it was a useless complication and you prefer
a single application, I continue on this way :-)
Thanks,
Stefano