* [PATCH 1/5] VSOCK: extract utility functions from vsock_diag_test.c
2017-12-13 14:49 [PATCH 0/5] VSOCK: add vsock_test test suite Stefan Hajnoczi
@ 2017-12-13 14:49 ` Stefan Hajnoczi
2017-12-13 14:49 ` [PATCH 2/5] VSOCK: extract connect/accept " Stefan Hajnoczi
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2017-12-13 14:49 UTC (permalink / raw)
To: netdev; +Cc: Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
Move useful functions into a separate file in preparation for more vsock
test programs.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tools/testing/vsock/Makefile | 2 +-
tools/testing/vsock/util.h | 35 +++++++++++++
tools/testing/vsock/util.c | 70 ++++++++++++++++++++++++++
tools/testing/vsock/vsock_diag_test.c | 92 +++++++++--------------------------
4 files changed, 128 insertions(+), 71 deletions(-)
create mode 100644 tools/testing/vsock/util.h
create mode 100644 tools/testing/vsock/util.c
diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
index 66ba0924194d..cf0650007fa8 100644
--- a/tools/testing/vsock/Makefile
+++ b/tools/testing/vsock/Makefile
@@ -1,6 +1,6 @@
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/uapi -I../../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.h b/tools/testing/vsock/util.h
new file mode 100644
index 000000000000..a51e17578ccf
--- /dev/null
+++ b/tools/testing/vsock/util.h
@@ -0,0 +1,35 @@
+#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/util.c b/tools/testing/vsock/util.c
new file mode 100644
index 000000000000..2567abd90153
--- /dev/null
+++ b/tools/testing/vsock/util.c
@@ -0,0 +1,70 @@
+/*
+ * vsock test utilities
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * Author: Stefan Hajnoczi <stefanha@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#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 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;
+}
+
+/* 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/vsock_diag_test.c b/tools/testing/vsock/vsock_diag_test.c
index e896a4af52f4..f1ace5d96e00 100644
--- a/tools/testing/vsock/vsock_diag_test.c
+++ b/tools/testing/vsock/vsock_diag_test.c
@@ -13,12 +13,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>
@@ -33,12 +31,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 {
@@ -339,7 +332,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);
@@ -350,7 +343,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;
@@ -388,7 +381,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;
@@ -397,7 +390,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;
@@ -434,7 +427,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;
@@ -486,9 +479,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);
}
@@ -507,11 +500,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,
@@ -528,30 +517,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[] = {
{
@@ -606,9 +571,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();
@@ -624,16 +590,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;
@@ -646,35 +612,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);
+ control_init(control_host, control_port,
+ opts.mode == TEST_MODE_SERVER);
- for (i = 0; test_cases[i].name; i++) {
- void (*run)(unsigned int peer_cid);
-
- 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.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/5] VSOCK: extract connect/accept functions from vsock_diag_test.c
2017-12-13 14:49 [PATCH 0/5] VSOCK: add vsock_test test suite Stefan Hajnoczi
2017-12-13 14:49 ` [PATCH 1/5] VSOCK: extract utility functions from vsock_diag_test.c Stefan Hajnoczi
@ 2017-12-13 14:49 ` Stefan Hajnoczi
2017-12-13 21:32 ` David Miller
2017-12-13 14:49 ` [PATCH 3/5] VSOCK: add full barrier between test cases Stefan Hajnoczi
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2017-12-13 14:49 UTC (permalink / raw)
To: netdev; +Cc: Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
Many test cases will need to connect to the server or accept incoming
connections. This patch extracts these operations into utility
functions that can be reused.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tools/testing/vsock/util.h | 6 ++
tools/testing/vsock/util.c | 108 ++++++++++++++++++++++++++++++++++
tools/testing/vsock/vsock_diag_test.c | 81 ++-----------------------
3 files changed, 119 insertions(+), 76 deletions(-)
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index a51e17578ccf..3e6971f15caa 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -1,6 +1,9 @@
#ifndef UTIL_H
#define UTIL_H
+#include <sys/socket.h>
+#include "../../../include/uapi/linux/vm_sockets.h"
+
/* Tests can either run as the client or the server */
enum test_mode {
TEST_MODE_UNSET,
@@ -29,6 +32,9 @@ struct test_case {
void init_signals(void);
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 run_tests(const struct test_case *test_cases,
const struct test_opts *opts);
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 2567abd90153..75a78f295b37 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -15,8 +15,10 @@
#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
+#include <unistd.h>
#include "timeout.h"
+#include "control.h"
#include "util.h"
/* Install signal handlers */
@@ -45,6 +47,112 @@ unsigned int parse_cid(const char *str)
return n;
}
+/* Connect to <cid, port> and return the file descriptor. */
+int vsock_stream_connect(unsigned int cid, unsigned int port)
+{
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } addr = {
+ .svm = {
+ .svm_family = AF_VSOCK,
+ .svm_port = port,
+ .svm_cid = cid,
+ },
+ };
+ int ret;
+ int fd;
+
+ control_expectln("LISTENING");
+
+ 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 < 0) {
+ int old_errno = errno;
+
+ close(fd);
+ fd = -1;
+ errno = old_errno;
+ }
+ return fd;
+}
+
+/* Listen on <cid, port> and return the first incoming connection. The remote
+ * address is stored to clientaddrp. clientaddrp may be NULL.
+ */
+int vsock_stream_accept(unsigned int cid, unsigned int port,
+ struct sockaddr_vm *clientaddrp)
+{
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } addr = {
+ .svm = {
+ .svm_family = AF_VSOCK,
+ .svm_port = port,
+ .svm_cid = cid,
+ },
+ };
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } clientaddr;
+ socklen_t clientaddr_len = sizeof(clientaddr.svm);
+ int fd;
+ int client_fd;
+ int old_errno;
+
+ fd = socket(AF_VSOCK, SOCK_STREAM, 0);
+
+ if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
+ perror("bind");
+ exit(EXIT_FAILURE);
+ }
+
+ if (listen(fd, 1) < 0) {
+ perror("listen");
+ exit(EXIT_FAILURE);
+ }
+
+ control_writeln("LISTENING");
+
+ timeout_begin(TIMEOUT);
+ do {
+ client_fd = accept(fd, &clientaddr.sa, &clientaddr_len);
+ timeout_check("accept");
+ } while (client_fd < 0 && errno == EINTR);
+ timeout_end();
+
+ old_errno = errno;
+ close(fd);
+ errno = old_errno;
+
+ if (client_fd < 0)
+ return client_fd;
+
+ if (clientaddr_len != sizeof(clientaddr.svm)) {
+ fprintf(stderr, "unexpected addrlen from accept(2), %zu\n",
+ (size_t)clientaddr_len);
+ exit(EXIT_FAILURE);
+ }
+ if (clientaddr.sa.sa_family != AF_VSOCK) {
+ fprintf(stderr, "expected AF_VSOCK from accept(2), got %d\n",
+ clientaddr.sa.sa_family);
+ exit(EXIT_FAILURE);
+ }
+
+ if (clientaddrp)
+ *clientaddrp = clientaddr.svm;
+ return client_fd;
+}
+
/* 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/vsock_diag_test.c b/tools/testing/vsock/vsock_diag_test.c
index f1ace5d96e00..654bbe2fc211 100644
--- a/tools/testing/vsock/vsock_diag_test.c
+++ b/tools/testing/vsock/vsock_diag_test.c
@@ -17,7 +17,6 @@
#include <string.h>
#include <errno.h>
#include <unistd.h>
-#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <linux/list.h>
@@ -26,7 +25,6 @@
#include <linux/sock_diag.h>
#include <netinet/tcp.h>
-#include "../../../include/uapi/linux/vm_sockets.h"
#include "../../../include/uapi/linux/vm_sockets_diag.h"
#include "timeout.h"
@@ -383,33 +381,12 @@ static void test_listen_socket_server(const struct test_opts *opts)
static void test_connect_client(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 fd;
- int ret;
LIST_HEAD(sockets);
struct vsock_stat *st;
- control_expectln("LISTENING");
-
- 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 < 0) {
+ fd = vsock_stream_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
perror("connect");
exit(EXIT_FAILURE);
}
@@ -429,66 +406,19 @@ static void test_connect_client(const struct test_opts *opts)
static void test_connect_server(const struct test_opts *opts)
{
- union {
- struct sockaddr sa;
- struct sockaddr_vm svm;
- } addr = {
- .svm = {
- .svm_family = AF_VSOCK,
- .svm_port = 1234,
- .svm_cid = VMADDR_CID_ANY,
- },
- };
- union {
- struct sockaddr sa;
- struct sockaddr_vm svm;
- } clientaddr;
- socklen_t clientaddr_len = sizeof(clientaddr.svm);
- LIST_HEAD(sockets);
struct vsock_stat *st;
- int fd;
+ LIST_HEAD(sockets);
int client_fd;
- fd = socket(AF_VSOCK, SOCK_STREAM, 0);
-
- if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
- perror("bind");
- exit(EXIT_FAILURE);
- }
-
- if (listen(fd, 1) < 0) {
- perror("listen");
- exit(EXIT_FAILURE);
- }
-
- control_writeln("LISTENING");
-
- timeout_begin(TIMEOUT);
- do {
- client_fd = accept(fd, &clientaddr.sa, &clientaddr_len);
- timeout_check("accept");
- } while (client_fd < 0 && errno == EINTR);
- timeout_end();
-
+ client_fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
if (client_fd < 0) {
perror("accept");
exit(EXIT_FAILURE);
}
- if (clientaddr.sa.sa_family != AF_VSOCK) {
- fprintf(stderr, "expected AF_VSOCK from accept(2), got %d\n",
- clientaddr.sa.sa_family);
- exit(EXIT_FAILURE);
- }
- if (clientaddr.svm.svm_cid != opts->peer_cid) {
- fprintf(stderr, "expected peer CID %u from accept(2), got %u\n",
- opts->peer_cid, clientaddr.svm.svm_cid);
- exit(EXIT_FAILURE);
- }
read_vsock_stat(&sockets);
- check_num_sockets(&sockets, 2);
- find_vsock_stat(&sockets, fd);
+ check_num_sockets(&sockets, 1);
st = find_vsock_stat(&sockets, client_fd);
check_socket_state(st, TCP_ESTABLISHED);
@@ -496,7 +426,6 @@ static void test_connect_server(const struct test_opts *opts)
control_expectln("DONE");
close(client_fd);
- close(fd);
free_sock_stat(&sockets);
}
--
2.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] VSOCK: extract connect/accept functions from vsock_diag_test.c
2017-12-13 14:49 ` [PATCH 2/5] VSOCK: extract connect/accept " Stefan Hajnoczi
@ 2017-12-13 21:32 ` David Miller
2017-12-14 9:49 ` Stefan Hajnoczi
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2017-12-13 21:32 UTC (permalink / raw)
To: stefanha; +Cc: netdev, jhansen, decui
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Wed, 13 Dec 2017 14:49:08 +0000
> +#include <sys/socket.h>
> +#include "../../../include/uapi/linux/vm_sockets.h"
> +
...
> -#include "../../../include/uapi/linux/vm_sockets.h"
> #include "../../../include/uapi/linux/vm_sockets_diag.h"
This really should never be necessary.
As part of the build, before tunning tests, one is required to do
"make headers_install" and whether as root or a normal user, this
will make the UAPI headers of the current kernel available to the
build of the testcases.
So please undo this weird relative include stuff.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] VSOCK: extract connect/accept functions from vsock_diag_test.c
2017-12-13 21:32 ` David Miller
@ 2017-12-14 9:49 ` Stefan Hajnoczi
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2017-12-14 9:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jhansen, decui
[-- Attachment #1: Type: text/plain, Size: 773 bytes --]
On Wed, Dec 13, 2017 at 04:32:58PM -0500, David Miller wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> Date: Wed, 13 Dec 2017 14:49:08 +0000
>
> > +#include <sys/socket.h>
> > +#include "../../../include/uapi/linux/vm_sockets.h"
> > +
> ...
> > -#include "../../../include/uapi/linux/vm_sockets.h"
> > #include "../../../include/uapi/linux/vm_sockets_diag.h"
>
> This really should never be necessary.
>
> As part of the build, before tunning tests, one is required to do
> "make headers_install" and whether as root or a normal user, this
> will make the UAPI headers of the current kernel available to the
> build of the testcases.
>
> So please undo this weird relative include stuff.
Thanks for explaining! Will fix in v2.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] VSOCK: add full barrier between test cases
2017-12-13 14:49 [PATCH 0/5] VSOCK: add vsock_test test suite Stefan Hajnoczi
2017-12-13 14:49 ` [PATCH 1/5] VSOCK: extract utility functions from vsock_diag_test.c Stefan Hajnoczi
2017-12-13 14:49 ` [PATCH 2/5] VSOCK: extract connect/accept " Stefan Hajnoczi
@ 2017-12-13 14:49 ` Stefan Hajnoczi
2017-12-13 14:49 ` [PATCH 4/5] VSOCK: add send_byte()/recv_byte() test utilities Stefan Hajnoczi
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2017-12-13 14:49 UTC (permalink / raw)
To: netdev; +Cc: Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
See code comment for details.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
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 75a78f295b37..2be923fe9922 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -165,10 +165,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.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/5] VSOCK: add send_byte()/recv_byte() test utilities
2017-12-13 14:49 [PATCH 0/5] VSOCK: add vsock_test test suite Stefan Hajnoczi
` (2 preceding siblings ...)
2017-12-13 14:49 ` [PATCH 3/5] VSOCK: add full barrier between test cases Stefan Hajnoczi
@ 2017-12-13 14:49 ` Stefan Hajnoczi
2017-12-13 14:49 ` [PATCH 5/5] VSOCK: add AF_VSOCK test cases Stefan Hajnoczi
2017-12-20 14:48 ` [PATCH 0/5] VSOCK: add vsock_test test suite Jorgen S. Hansen
5 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2017-12-13 14:49 UTC (permalink / raw)
To: netdev; +Cc: Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
Test cases will want to transfer data. This patch adds utility
functions to do this.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tools/testing/vsock/util.h | 2 +
tools/testing/vsock/util.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index 3e6971f15caa..cf43d42591bc 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -35,6 +35,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);
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 2be923fe9922..e28085a2a429 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -13,6 +13,7 @@
#include <errno.h>
#include <stdio.h>
+#include <stdint.h>
#include <stdlib.h>
#include <signal.h>
#include <unistd.h>
@@ -153,6 +154,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)
--
2.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/5] VSOCK: add AF_VSOCK test cases
2017-12-13 14:49 [PATCH 0/5] VSOCK: add vsock_test test suite Stefan Hajnoczi
` (3 preceding siblings ...)
2017-12-13 14:49 ` [PATCH 4/5] VSOCK: add send_byte()/recv_byte() test utilities Stefan Hajnoczi
@ 2017-12-13 14:49 ` Stefan Hajnoczi
2017-12-20 14:48 ` [PATCH 0/5] VSOCK: add vsock_test test suite Jorgen S. Hansen
5 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2017-12-13 14:49 UTC (permalink / raw)
To: netdev; +Cc: Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
The vsock_test.c program runs a test suite of AF_VSOCK test cases.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tools/testing/vsock/Makefile | 5 +-
tools/testing/vsock/vsock_test.c | 321 +++++++++++++++++++++++++++++++++++++++
tools/testing/vsock/.gitignore | 1 +
tools/testing/vsock/README | 1 +
4 files changed, 326 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/vsock/vsock_test.c
diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
index cf0650007fa8..9efb9010df2e 100644
--- a/tools/testing/vsock/Makefile
+++ b/tools/testing/vsock/Makefile
@@ -1,9 +1,10 @@
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/uapi -I../../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/vsock_test.c b/tools/testing/vsock/vsock_test.c
new file mode 100644
index 000000000000..c1fc376eea00
--- /dev/null
+++ b/tools/testing/vsock/vsock_test.c
@@ -0,0 +1,321 @@
+/*
+ * vsock_test - vsock.ko test suite
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * Author: Stefan Hajnoczi <stefanha@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <getopt.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <linux/list.h>
+#include <linux/net.h>
+#include <linux/netlink.h>
+#include <linux/sock_diag.h>
+#include <netinet/tcp.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");
+}
+
+#define MULTICONN_NFDS 1000
+
+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 % 1)
+ 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 % 1)
+ 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;
+}
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/README b/tools/testing/vsock/README
index 2cc6d7302db6..3014d9e4959a 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
--
2.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 0/5] VSOCK: add vsock_test test suite
2017-12-13 14:49 [PATCH 0/5] VSOCK: add vsock_test test suite Stefan Hajnoczi
` (4 preceding siblings ...)
2017-12-13 14:49 ` [PATCH 5/5] VSOCK: add AF_VSOCK test cases Stefan Hajnoczi
@ 2017-12-20 14:48 ` Jorgen S. Hansen
2018-01-02 12:05 ` Stefan Hajnoczi
5 siblings, 1 reply; 11+ messages in thread
From: Jorgen S. Hansen @ 2017-12-20 14:48 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: netdev, Dexuan Cui
> On Dec 13, 2017, at 3:49 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> 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).
>
> The test suite is modest but I hope to cover additional cases in the future.
> My goal is to have a shared test suite so VMCI, Hyper-V, and KVM can ensure
> that our transports behave the same.
>
> I have tested virtio-vsock.
>
> Jorgen: Please test the VMCI transport and let me know if anything needs to be
> adjusted. See tools/testing/vsock/README for information on how to run the
> test suite.
>
I tried running the vsock_test on VMCI, and all the tests failed in one way or
another:
1) connection reset test: when the guest tries to connect to the host, we
get EINVAL as the error instead of ECONNRESET. I’ll fix that.
2) client close and server close tests: On the host side, VMCI doesn’t
support reading data from a socket that has been closed by the
guest. When the guest closes a connection, all data is gone, and
we return EOF on the host side. So the tests that try to read data
after close, should not attempt that on VMCI host side. I got the
tests to pass by adding a getsockname call to determine if
the local CID was the host CID, and then skip the read attempt
in that case. We could add a vmci flag, that would enable
this behavior.
3) send_byte(fd, -EPIPE): for the VMCI transport, the close
isn’t necessarily visible immediately on the peer. So in most
cases, these send operations would complete with success.
I was running these tests using nested virtualization, so I
suspect that the problem is more likely to occur here, but
I had to add a sleep to be sure to get the EPIPE error.
4) server close test: the connect would sometimes fail. This looks
like an issue where we detect the peer close on the client side
before we complete the connection handshake on the client
side. There are two different channels used for the connection
handshake and the disconnect. I’ll look into this to see what
exactly is going on.
5) multiple connections tests: with the standard socket sizes,
VMCI is only able to support about 100 concurrent stream
connections so this test passes with MULTICONN_NFDS
set to 100.
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/5] VSOCK: add vsock_test test suite
2017-12-20 14:48 ` [PATCH 0/5] VSOCK: add vsock_test test suite Jorgen S. Hansen
@ 2018-01-02 12:05 ` Stefan Hajnoczi
2018-01-03 16:09 ` Jorgen S. Hansen
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2018-01-02 12:05 UTC (permalink / raw)
To: Jorgen S. Hansen; +Cc: netdev, Dexuan Cui
[-- Attachment #1: Type: text/plain, Size: 3303 bytes --]
On Wed, Dec 20, 2017 at 02:48:43PM +0000, Jorgen S. Hansen wrote:
>
> > On Dec 13, 2017, at 3:49 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > 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).
> >
> > The test suite is modest but I hope to cover additional cases in the future.
> > My goal is to have a shared test suite so VMCI, Hyper-V, and KVM can ensure
> > that our transports behave the same.
> >
> > I have tested virtio-vsock.
> >
> > Jorgen: Please test the VMCI transport and let me know if anything needs to be
> > adjusted. See tools/testing/vsock/README for information on how to run the
> > test suite.
> >
>
> I tried running the vsock_test on VMCI, and all the tests failed in one way or
> another:
Great, thank you for testing and looking into the failures!
> 1) connection reset test: when the guest tries to connect to the host, we
> get EINVAL as the error instead of ECONNRESET. I’ll fix that.
Yay, the tests found a bug!
> 2) client close and server close tests: On the host side, VMCI doesn’t
> support reading data from a socket that has been closed by the
> guest. When the guest closes a connection, all data is gone, and
> we return EOF on the host side. So the tests that try to read data
> after close, should not attempt that on VMCI host side. I got the
> tests to pass by adding a getsockname call to determine if
> the local CID was the host CID, and then skip the read attempt
> in that case. We could add a vmci flag, that would enable
> this behavior.
Interesting behavior. Is there a reason for disallowing half-closed
sockets on the host side?
> 3) send_byte(fd, -EPIPE): for the VMCI transport, the close
> isn’t necessarily visible immediately on the peer. So in most
> cases, these send operations would complete with success.
> I was running these tests using nested virtualization, so I
> suspect that the problem is more likely to occur here, but
> I had to add a sleep to be sure to get the EPIPE error.
Good point, you've discovered a race condition that affects all
transports. The vsock close state transition might not have occurred
yet when the TCP control channel receives the "CLOSED" message.
test_stream_client_close_server() needs to wait for the socket status to
change before attempting send_byte(fd, -EPIPE). I guess I'll have to
use vsock_diag or another kernel interface to check the socket's state.
> 5) multiple connections tests: with the standard socket sizes,
> VMCI is only able to support about 100 concurrent stream
> connections so this test passes with MULTICONN_NFDS
> set to 100.
The 1000 magic number is because many distros have a maximum number of
file descriptors ulimit of 1024. But it's an arbitrary number and we
could lower it to 100.
Is this VMCI concurrent stream limit a denial of service vector? Can an
unprivileged guest userspace process open many sockets to prevent
legitimate connections from other users within the same guest?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5] VSOCK: add vsock_test test suite
2018-01-02 12:05 ` Stefan Hajnoczi
@ 2018-01-03 16:09 ` Jorgen S. Hansen
0 siblings, 0 replies; 11+ messages in thread
From: Jorgen S. Hansen @ 2018-01-03 16:09 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: netdev, Dexuan Cui
> On Jan 2, 2018, at 1:05 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Wed, Dec 20, 2017 at 02:48:43PM +0000, Jorgen S. Hansen wrote:
>>
>>> On Dec 13, 2017, at 3:49 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>
>>> 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).
>>>
>>> The test suite is modest but I hope to cover additional cases in the future.
>>> My goal is to have a shared test suite so VMCI, Hyper-V, and KVM can ensure
>>> that our transports behave the same.
>>>
>>> I have tested virtio-vsock.
>>>
>>> Jorgen: Please test the VMCI transport and let me know if anything needs to be
>>> adjusted. See tools/testing/vsock/README for information on how to run the
>>> test suite.
>>>
>>
>> I tried running the vsock_test on VMCI, and all the tests failed in one way or
>> another:
>
> Great, thank you for testing and looking into the failures!
>
>> 1) connection reset test: when the guest tries to connect to the host, we
>> get EINVAL as the error instead of ECONNRESET. I’ll fix that.
>
> Yay, the tests found a bug!
>
>> 2) client close and server close tests: On the host side, VMCI doesn’t
>> support reading data from a socket that has been closed by the
>> guest. When the guest closes a connection, all data is gone, and
>> we return EOF on the host side. So the tests that try to read data
>> after close, should not attempt that on VMCI host side. I got the
>> tests to pass by adding a getsockname call to determine if
>> the local CID was the host CID, and then skip the read attempt
>> in that case. We could add a vmci flag, that would enable
>> this behavior.
>
> Interesting behavior. Is there a reason for disallowing half-closed
> sockets on the host side?
This is a consequence of the way the underlying VMCI queue pairs are
implemented. When the guest side closes a connection, it signals this
to the peer by detaching from the VMCI queue pair used for the data
transfer (the detach will result in an event being generated on the
peer side). However, the VMCI queue pair is allocated as part of the
guest memory, so when the guest detaches, that memory is reclaimed.
So the host side would need to create a copy of the contents of that
queue pair in kernel memory as part of the detach operation. When
this was implemented, it was decided that it was better to avoid
a potential large kernel memory allocation and the data copy at
detach time than to maintain the half close behavior of INET.
>
>> 5) multiple connections tests: with the standard socket sizes,
>> VMCI is only able to support about 100 concurrent stream
>> connections so this test passes with MULTICONN_NFDS
>> set to 100.
>
> The 1000 magic number is because many distros have a maximum number of
> file descriptors ulimit of 1024. But it's an arbitrary number and we
> could lower it to 100.
>
> Is this VMCI concurrent stream limit a denial of service vector? Can an
> unprivileged guest userspace process open many sockets to prevent
> legitimate connections from other users within the same guest?
vSocket uses VMCI queue pairs for the stream, and the VMCI device
only allows a limited amount of memory to be used for queue pairs
per VM. So it is possible to exhaust this shared resource. The queue
pairs are created as part of the connection establishment process, so
it would require the user process to both create and connect the sockets
to a host side endpoint (connections between guest processes will not
allocate VMCI queue pairs).
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 11+ messages in thread