| Message ID | 20190801152541.245833-12-sgarzare@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Ahern |
| Headers | show |
| Series | VSOCK: add vsock_test test suite | expand |
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 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 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
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
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 <jhansen@vmware.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- tools/testing/vsock/util.c | 38 ++++++++++++++++++++++++++++++++ tools/testing/vsock/util.h | 1 + tools/testing/vsock/vsock_test.c | 10 +++++++++ 3 files changed, 49 insertions(+)