lkml.org 
[lkml]   [2023]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2023-11-29 10:37    [W:0.075 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site