Messages in this thread | | | Date | Wed, 29 Nov 2023 10:36:48 +0100 | From | Stefano Garzarella <> | Subject | Re: [RFC PATCH v3 3/3] vsock/test: SO_RCVLOWAT + deferred credit update test |
| |
On Wed, Nov 29, 2023 at 12:16:54PM +0300, Arseniy Krasnov wrote: > > >On 29.11.2023 12:16, Stefano Garzarella wrote: >> On Wed, Nov 22, 2023 at 09:05:10PM +0300, Arseniy Krasnov wrote: >>> Test which checks, that updating SO_RCVLOWAT value also sends credit >>> update message. Otherwise mutual hungup may happen when receiver didn't >>> send credit update and then calls 'poll()' with non default SO_RCVLOWAT >>> value (e.g. waiting enough bytes to read), while sender waits for free >>> space at receiver's side. Important thing is that this test relies on >>> kernel's define for maximum packet size for virtio transport and this >>> value is not exported to user: VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (this >>> define is used to control moment when to send credit update message). >>> If this value or its usage will be changed in kernel - this test may >>> become useless/broken. >>> >>> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com> >>> --- >>> Changelog: >>> v1 -> v2: >>> * Update commit message by removing 'This patch adds XXX' manner. >>> * Update commit message by adding details about dependency for this >>> test from kernel internal define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. >>> * Add comment for this dependency in 'vsock_test.c' where this define >>> is duplicated. >>> v2 -> v3: >>> * Replace synchronization based on control TCP socket with vsock >>> data socket - this is needed to allow sender transmit data only >>> when new buffer size of receiver is visible to sender. Otherwise >>> there is race and test fails sometimes. >>> >>> tools/testing/vsock/vsock_test.c | 142 +++++++++++++++++++++++++++++++ >>> 1 file changed, 142 insertions(+) >>> >>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c >>> index 5b0e93f9996c..773a71260fba 100644 >>> --- a/tools/testing/vsock/vsock_test.c >>> +++ b/tools/testing/vsock/vsock_test.c >>> @@ -1225,6 +1225,143 @@ static void test_double_bind_connect_client(const struct test_opts *opts) >>> } >>> } >>> >>> +#define RCVLOWAT_CREDIT_UPD_BUF_SIZE (1024 * 128) >>> +/* This define is the same as in 'include/linux/virtio_vsock.h': >>> + * it is used to decide when to send credit update message during >>> + * reading from rx queue of a socket. Value and its usage in >>> + * kernel is important for this test. >>> + */ >>> +#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64) >>> + >>> +static void test_stream_rcvlowat_def_cred_upd_client(const struct test_opts *opts) >>> +{ >>> + size_t buf_size; >>> + void *buf; >>> + int fd; >>> + >>> + fd = vsock_stream_connect(opts->peer_cid, 1234); >>> + if (fd < 0) { >>> + perror("connect"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + /* Send 1 byte more than peer's buffer size. */ >>> + buf_size = RCVLOWAT_CREDIT_UPD_BUF_SIZE + 1; >>> + >>> + buf = malloc(buf_size); >>> + if (!buf) { >>> + perror("malloc"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + /* Wait until peer sets needed buffer size. */ >>> + recv_byte(fd, 1, 0); >>> + >>> + if (send(fd, buf, buf_size, 0) != buf_size) { >>> + perror("send failed"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + free(buf); >>> + close(fd); >>> +} >>> + >>> +static void test_stream_rcvlowat_def_cred_upd_server(const struct test_opts *opts) >>> +{ >>> + size_t recv_buf_size; >>> + struct pollfd fds; >>> + size_t buf_size; >>> + void *buf; >>> + int fd; >>> + >>> + fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL); >>> + if (fd < 0) { >>> + perror("accept"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + buf_size = RCVLOWAT_CREDIT_UPD_BUF_SIZE; >>> + >>> + if (setsockopt(fd, AF_VSOCK, SO_VM_SOCKETS_BUFFER_SIZE, >>> + &buf_size, sizeof(buf_size))) { >>> + perror("setsockopt(SO_VM_SOCKETS_BUFFER_SIZE)"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + /* Send one dummy byte here, because 'setsockopt()' above also >>> + * sends special packet which tells sender to update our buffer >>> + * size. This 'send_byte()' will serialize such packet with data >>> + * reads in a loop below. Sender starts transmission only when >>> + * it receives this single byte. >>> + */ >>> + send_byte(fd, 1, 0); >>> + >>> + buf = malloc(buf_size); >>> + if (!buf) { >>> + perror("malloc"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + /* Wait until there will be 128KB of data in rx queue. */ >>> + while (1) { >>> + ssize_t res; >>> + >>> + res = recv(fd, buf, buf_size, MSG_PEEK); >>> + if (res == buf_size) >>> + break; >>> + >>> + if (res <= 0) { >>> + fprintf(stderr, "unexpected 'recv()' return: %zi\n", res); >>> + exit(EXIT_FAILURE); >>> + } >>> + } >>> + >>> + /* There is 128KB of data in the socket's rx queue, >>> + * dequeue first 64KB, credit update is not sent. >>> + */ >>> + recv_buf_size = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE; >>> + recv_buf(fd, buf, recv_buf_size, 0, recv_buf_size); >>> + recv_buf_size++; >>> + >>> + /* Updating SO_RCVLOWAT will send credit update. */ >>> + if (setsockopt(fd, SOL_SOCKET, SO_RCVLOWAT, >>> + &recv_buf_size, sizeof(recv_buf_size))) { >>> + perror("setsockopt(SO_RCVLOWAT)"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + memset(&fds, 0, sizeof(fds)); >>> + fds.fd = fd; >>> + fds.events = POLLIN | POLLRDNORM | POLLERR | >>> + POLLRDHUP | POLLHUP; >>> + >>> + /* This 'poll()' will return once we receive last byte >>> + * sent by client. >>> + */ >>> + if (poll(&fds, 1, -1) < 0) { >>> + perror("poll"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + if (fds.revents & POLLERR) { >>> + fprintf(stderr, "'poll()' error\n"); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + if (fds.revents & (POLLIN | POLLRDNORM)) { >>> + recv_buf(fd, buf, recv_buf_size, 0, recv_buf_size); >> >> Should we set the socket non-blocking? >> >> Otherwise, here poll() might wake up even if there are not all the >> expected bytes due to some bug and recv() block waiting for the >> remaining bytes, so we might not notice the bug. > >Good point! or just use MSG_DONTWAIT flag for only this 'recv()'.
Yep, right!
Stefano
| |