[RFC PATCH v1 1/3] af_vsock: add two new tests for SOCK_SEQPACKET

[RFC PATCH v1 1/3] af_vsock: add two new tests for SOCK_SEQPACKET

[Krasnov Arseniy Vladimirovich]

This adds two tests: for receive timeout and reading to invalid
buffer provided by user. I forgot to put both patches to main
patchset.

Arseniy Krasnov(2):

af_vsock: SOCK_SEQPACKET receive timeout test
af_vsock: SOCK_SEQPACKET broken buffer test

tools/testing/vsock/vsock_test.c | 170 +++++++++++++++++++++++++++++++++++++++
1 file changed, 170 insertions(+)

-- 
2.25.1

[RFC PATCH v1 2/3] af_vsock: SOCK_SEQPACKET receive timeout test

[Krasnov Arseniy Vladimirovich]

Test for receive timeout check: connection is established,
receiver sets timeout, but sender does nothing. Receiver's
'read()' call must return EAGAIN.

Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
 tools/testing/vsock/vsock_test.c | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 2a3638c0a008..aa2de27d0f77 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -391,6 +391,50 @@ static void test_seqpacket_msg_trunc_server(const struct test_opts *opts)
 	close(fd);
 }
 
+static void test_seqpacket_timeout_client(const struct test_opts *opts)
+{
+	int fd;
+	struct timeval tv;
+	char dummy;
+
+	fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
+	if (fd < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	tv.tv_sec = 1;
+	tv.tv_usec = 0;
+
+	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)) == -1) {
+		perror("setsockopt 'SO_RCVTIMEO'");
+		exit(EXIT_FAILURE);
+	}
+
+	if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
+	    (errno != EAGAIN)) {
+		perror("EAGAIN expected");
+		exit(EXIT_FAILURE);
+	}
+
+	control_writeln("WAITDONE");
+	close(fd);
+}
+
+static void test_seqpacket_timeout_server(const struct test_opts *opts)
+{
+	int fd;
+
+	fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
+	if (fd < 0) {
+		perror("accept");
+		exit(EXIT_FAILURE);
+	}
+
+	control_expectln("WAITDONE");
+	close(fd);
+}
+
 static struct test_case test_cases[] = {
 	{
 		.name = "SOCK_STREAM connection reset",
@@ -431,6 +475,11 @@ static struct test_case test_cases[] = {
 		.run_client = test_seqpacket_msg_trunc_client,
 		.run_server = test_seqpacket_msg_trunc_server,
 	},
+	{
+		.name = "SOCK_SEQPACKET timeout",
+		.run_client = test_seqpacket_timeout_client,
+		.run_server = test_seqpacket_timeout_server,
+	},
 	{},
 };
 
-- 
2.25.1

Re: [RFC PATCH v1 2/3] af_vsock: SOCK_SEQPACKET receive timeout test

[Stefano Garzarella]

On Fri, Mar 11, 2022 at 10:55:42AM +0000, Krasnov Arseniy Vladimirovich wrote:
>Test for receive timeout check: connection is established,
>receiver sets timeout, but sender does nothing. Receiver's
>'read()' call must return EAGAIN.
>
>Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>---
> tools/testing/vsock/vsock_test.c | 49 ++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 2a3638c0a008..aa2de27d0f77 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -391,6 +391,50 @@ static void test_seqpacket_msg_trunc_server(const struct test_opts *opts)
> 	close(fd);
> }
>
>+static void test_seqpacket_timeout_client(const struct test_opts *opts)
>+{
>+	int fd;
>+	struct timeval tv;
>+	char dummy;
>+
>+	fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	tv.tv_sec = 1;
>+	tv.tv_usec = 0;
>+
>+	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)) == -1) {
>+		perror("setsockopt 'SO_RCVTIMEO'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
>+	    (errno != EAGAIN)) {
>+		perror("EAGAIN expected");
>+		exit(EXIT_FAILURE);
>+	}

The patch LGTM, maybe the only thing I would add here is a check on the 
time spent in the read(), to see that it is approximately the timeout we 
have set.

Thanks,
Stefano

Re: [RFC PATCH v1 2/3] af_vsock: SOCK_SEQPACKET receive timeout test

[Krasnov Arseniy Vladimirovich]

On 15.03.2022 11:15, Stefano Garzarella wrote:
> On Fri, Mar 11, 2022 at 10:55:42AM +0000, Krasnov Arseniy Vladimirovich wrote:
>> Test for receive timeout check: connection is established,
>> receiver sets timeout, but sender does nothing. Receiver's
>> 'read()' call must return EAGAIN.
>>
>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>> ---
>> tools/testing/vsock/vsock_test.c | 49 ++++++++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index 2a3638c0a008..aa2de27d0f77 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -391,6 +391,50 @@ static void test_seqpacket_msg_trunc_server(const struct test_opts *opts)
>>     close(fd);
>> }
>>
>> +static void test_seqpacket_timeout_client(const struct test_opts *opts)
>> +{
>> +    int fd;
>> +    struct timeval tv;
>> +    char dummy;
>> +
>> +    fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>> +    if (fd < 0) {
>> +        perror("connect");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    tv.tv_sec = 1;
>> +    tv.tv_usec = 0;
>> +
>> +    if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)) == -1) {
>> +        perror("setsockopt 'SO_RCVTIMEO'");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
>> +        (errno != EAGAIN)) {
>> +        perror("EAGAIN expected");
>> +        exit(EXIT_FAILURE);
>> +    }
> 
> The patch LGTM, maybe the only thing I would add here is a check on the time spent in the read(), to see that it is approximately the timeout we have set.

Ack, I'll add it on v2

> 
> Thanks,
> Stefano
> 

[RFC PATCH v1 3/3] af_vsock: SOCK_SEQPACKET broken buffer test

[Krasnov Arseniy Vladimirovich]

Add test where sender sends two message, each with own
data pattern. Reader tries to read first to broken buffer:
it has three pages size, but middle page is unmapped. Then,
reader tries to read second message to valid buffer. Test
checks, that uncopied part of first message was dropped
and thus not copied as part of second message.

Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
 tools/testing/vsock/vsock_test.c | 121 +++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index aa2de27d0f77..686af712b4ad 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/mman.h>
 
 #include "timeout.h"
 #include "control.h"
@@ -435,6 +436,121 @@ static void test_seqpacket_timeout_server(const struct test_opts *opts)
 	close(fd);
 }
 
+#define BUF_PATTERN_1 'a'
+#define BUF_PATTERN_2 'b'
+
+static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opts)
+{
+	int fd;
+	unsigned char *buf1;
+	unsigned char *buf2;
+	int buf_size = getpagesize() * 3;
+
+	fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
+	if (fd < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	buf1 = malloc(buf_size);
+	if (buf1 == NULL) {
+		perror("'malloc()' for 'buf1'");
+		exit(EXIT_FAILURE);
+	}
+
+	buf2 = malloc(buf_size);
+	if (buf2 == NULL) {
+		perror("'malloc()' for 'buf2'");
+		exit(EXIT_FAILURE);
+	}
+
+	memset(buf1, BUF_PATTERN_1, buf_size);
+	memset(buf2, BUF_PATTERN_2, buf_size);
+
+	if (send(fd, buf1, buf_size, 0) != buf_size) {
+		perror("send failed");
+		exit(EXIT_FAILURE);
+	}
+
+	if (send(fd, buf2, buf_size, 0) != buf_size) {
+		perror("send failed");
+		exit(EXIT_FAILURE);
+	}
+
+	close(fd);
+}
+
+static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts *opts)
+{
+	int fd;
+	unsigned char *broken_buf;
+	unsigned char *valid_buf;
+	int page_size = getpagesize();
+	int buf_size = page_size * 3;
+	ssize_t res;
+	int prot = PROT_READ | PROT_WRITE;
+	int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+	int i;
+
+	fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
+	if (fd < 0) {
+		perror("accept");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Setup first buffer. */
+	broken_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
+	if (broken_buf == MAP_FAILED) {
+		perror("mmap for 'broken_buf'");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Unmap "hole" in buffer. */
+	if (munmap(broken_buf + page_size, page_size)) {
+		perror("'broken_buf' setup");
+		exit(EXIT_FAILURE);
+	}
+
+	valid_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
+	if (valid_buf == MAP_FAILED) {
+		perror("mmap for 'valid_buf'");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Try to fill buffer with unmapped middle. */
+	res = read(fd, broken_buf, buf_size);
+	if (res != -1) {
+		perror("invalid read result of 'broken_buf'");
+		exit(EXIT_FAILURE);
+	}
+
+	if (errno != ENOMEM) {
+		perror("invalid errno of 'broken_buf'");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Try to fill valid buffer. */
+	res = read(fd, valid_buf, buf_size);
+	if (res != buf_size) {
+		perror("invalid read result of 'valid_buf'");
+		exit(EXIT_FAILURE);
+	}
+
+	for (i = 0; i < buf_size; i++) {
+		if (valid_buf[i] != BUF_PATTERN_2) {
+			perror("invalid pattern for valid buf");
+			exit(EXIT_FAILURE);
+		}
+	}
+
+
+	/* Unmap buffers. */
+	munmap(broken_buf, page_size);
+	munmap(broken_buf + page_size * 2, page_size);
+	munmap(valid_buf, buf_size);
+	close(fd);
+}
+
 static struct test_case test_cases[] = {
 	{
 		.name = "SOCK_STREAM connection reset",
@@ -480,6 +596,11 @@ static struct test_case test_cases[] = {
 		.run_client = test_seqpacket_timeout_client,
 		.run_server = test_seqpacket_timeout_server,
 	},
+	{
+		.name = "SOCK_SEQPACKET invalid receive buffer",
+		.run_client = test_seqpacket_invalid_rec_buffer_client,
+		.run_server = test_seqpacket_invalid_rec_buffer_server,
+	},
 	{},
 };
 
-- 
2.25.1

Re: [RFC PATCH v1 3/3] af_vsock: SOCK_SEQPACKET broken buffer test

[Stefano Garzarella]

On Fri, Mar 11, 2022 at 10:58:32AM +0000, Krasnov Arseniy Vladimirovich wrote:
>Add test where sender sends two message, each with own
>data pattern. Reader tries to read first to broken buffer:
>it has three pages size, but middle page is unmapped. Then,
>reader tries to read second message to valid buffer. Test
>checks, that uncopied part of first message was dropped
>and thus not copied as part of second message.
>
>Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>---
> tools/testing/vsock/vsock_test.c | 121 +++++++++++++++++++++++++++++++
> 1 file changed, 121 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index aa2de27d0f77..686af712b4ad 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -16,6 +16,7 @@
> #include <linux/kernel.h>
> #include <sys/types.h>
> #include <sys/socket.h>
>+#include <sys/mman.h>
>
> #include "timeout.h"
> #include "control.h"
>@@ -435,6 +436,121 @@ static void test_seqpacket_timeout_server(const struct test_opts *opts)
> 	close(fd);
> }
>
>+#define BUF_PATTERN_1 'a'
>+#define BUF_PATTERN_2 'b'
>+
>+static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opts)
>+{
>+	int fd;
>+	unsigned char *buf1;
>+	unsigned char *buf2;
>+	int buf_size = getpagesize() * 3;
>+
>+	fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	buf1 = malloc(buf_size);
>+	if (buf1 == NULL) {
>+		perror("'malloc()' for 'buf1'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	buf2 = malloc(buf_size);
>+	if (buf2 == NULL) {
>+		perror("'malloc()' for 'buf2'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	memset(buf1, BUF_PATTERN_1, buf_size);
>+	memset(buf2, BUF_PATTERN_2, buf_size);
>+
>+	if (send(fd, buf1, buf_size, 0) != buf_size) {
>+		perror("send failed");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (send(fd, buf2, buf_size, 0) != buf_size) {
>+		perror("send failed");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	close(fd);
>+}
>+
>+static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts *opts)
>+{
>+	int fd;
>+	unsigned char *broken_buf;
>+	unsigned char *valid_buf;
>+	int page_size = getpagesize();
>+	int buf_size = page_size * 3;
>+	ssize_t res;
>+	int prot = PROT_READ | PROT_WRITE;
>+	int flags = MAP_PRIVATE | MAP_ANONYMOUS;
>+	int i;
>+
>+	fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
>+	if (fd < 0) {
>+		perror("accept");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	/* Setup first buffer. */
>+	broken_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>+	if (broken_buf == MAP_FAILED) {
>+		perror("mmap for 'broken_buf'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	/* Unmap "hole" in buffer. */
>+	if (munmap(broken_buf + page_size, page_size)) {
>+		perror("'broken_buf' setup");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	valid_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>+	if (valid_buf == MAP_FAILED) {
>+		perror("mmap for 'valid_buf'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	/* Try to fill buffer with unmapped middle. */
>+	res = read(fd, broken_buf, buf_size);
>+	if (res != -1) {
>+		perror("invalid read result of 'broken_buf'");

if `res` is valid, errno is not set, better to use fprintf(stderr, ...) 
printing the expected and received result.
Take a look at test_stream_connection_reset()

>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (errno != ENOMEM) {
>+		perror("invalid errno of 'broken_buf'");

Instead of "invalid", I would say "unexpected".

>+		exit(EXIT_FAILURE);
>+	}


>+
>+	/* Try to fill valid buffer. */
>+	res = read(fd, valid_buf, buf_size);
>+	if (res != buf_size) {
>+		perror("invalid read result of 'valid_buf'");

I would split in 2 checks:
- (res < 0) then use perror()
- (res != buf_size) then use fprintf(stderr, ...) printing the expected 
   and received result.

>+		exit(EXIT_FAILURE);
>+	}
>+
>+	for (i = 0; i < buf_size; i++) {
>+		if (valid_buf[i] != BUF_PATTERN_2) {
>+			perror("invalid pattern for valid buf");

errno is not set here, better to use fprintf(stderr, ...)

>+			exit(EXIT_FAILURE);
>+		}
>+	}

What about replace this for with a memcmp()?

>+
>+
>+	/* Unmap buffers. */
>+	munmap(broken_buf, page_size);
>+	munmap(broken_buf + page_size * 2, page_size);
>+	munmap(valid_buf, buf_size);
>+	close(fd);
>+}
>+
> static struct test_case test_cases[] = {
> 	{
> 		.name = "SOCK_STREAM connection reset",
>@@ -480,6 +596,11 @@ static struct test_case test_cases[] = {
> 		.run_client = test_seqpacket_timeout_client,
> 		.run_server = test_seqpacket_timeout_server,
> 	},
>+	{
>+		.name = "SOCK_SEQPACKET invalid receive buffer",
>+		.run_client = test_seqpacket_invalid_rec_buffer_client,
>+		.run_server = test_seqpacket_invalid_rec_buffer_server,
>+	},


Is this the right behavior? If read() fails because the buffer is 
invalid, do we throw out the whole packet?

I was expecting the packet not to be consumed, have you tried AF_UNIX, 
does it have the same behavior?

Thanks,
Stefano

Re: [RFC PATCH v1 3/3] af_vsock: SOCK_SEQPACKET broken buffer test

[Krasnov Arseniy Vladimirovich]

On 15.03.2022 11:36, Stefano Garzarella wrote:
> On Fri, Mar 11, 2022 at 10:58:32AM +0000, Krasnov Arseniy Vladimirovich wrote:
>> Add test where sender sends two message, each with own
>> data pattern. Reader tries to read first to broken buffer:
>> it has three pages size, but middle page is unmapped. Then,
>> reader tries to read second message to valid buffer. Test
>> checks, that uncopied part of first message was dropped
>> and thus not copied as part of second message.
>>
>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>> ---
>> tools/testing/vsock/vsock_test.c | 121 +++++++++++++++++++++++++++++++
>> 1 file changed, 121 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index aa2de27d0f77..686af712b4ad 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -16,6 +16,7 @@
>> #include <linux/kernel.h>
>> #include <sys/types.h>
>> #include <sys/socket.h>
>> +#include <sys/mman.h>
>>
>> #include "timeout.h"
>> #include "control.h"
>> @@ -435,6 +436,121 @@ static void test_seqpacket_timeout_server(const struct test_opts *opts)
>>     close(fd);
>> }
>>
>> +#define BUF_PATTERN_1 'a'
>> +#define BUF_PATTERN_2 'b'
>> +
>> +static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opts)
>> +{
>> +    int fd;
>> +    unsigned char *buf1;
>> +    unsigned char *buf2;
>> +    int buf_size = getpagesize() * 3;
>> +
>> +    fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>> +    if (fd < 0) {
>> +        perror("connect");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    buf1 = malloc(buf_size);
>> +    if (buf1 == NULL) {
>> +        perror("'malloc()' for 'buf1'");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    buf2 = malloc(buf_size);
>> +    if (buf2 == NULL) {
>> +        perror("'malloc()' for 'buf2'");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    memset(buf1, BUF_PATTERN_1, buf_size);
>> +    memset(buf2, BUF_PATTERN_2, buf_size);
>> +
>> +    if (send(fd, buf1, buf_size, 0) != buf_size) {
>> +        perror("send failed");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    if (send(fd, buf2, buf_size, 0) != buf_size) {
>> +        perror("send failed");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    close(fd);
>> +}
>> +
>> +static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts *opts)
>> +{
>> +    int fd;
>> +    unsigned char *broken_buf;
>> +    unsigned char *valid_buf;
>> +    int page_size = getpagesize();
>> +    int buf_size = page_size * 3;
>> +    ssize_t res;
>> +    int prot = PROT_READ | PROT_WRITE;
>> +    int flags = MAP_PRIVATE | MAP_ANONYMOUS;
>> +    int i;
>> +
>> +    fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
>> +    if (fd < 0) {
>> +        perror("accept");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    /* Setup first buffer. */
>> +    broken_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>> +    if (broken_buf == MAP_FAILED) {
>> +        perror("mmap for 'broken_buf'");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    /* Unmap "hole" in buffer. */
>> +    if (munmap(broken_buf + page_size, page_size)) {
>> +        perror("'broken_buf' setup");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    valid_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>> +    if (valid_buf == MAP_FAILED) {
>> +        perror("mmap for 'valid_buf'");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    /* Try to fill buffer with unmapped middle. */
>> +    res = read(fd, broken_buf, buf_size);
>> +    if (res != -1) {
>> +        perror("invalid read result of 'broken_buf'");
> 
> if `res` is valid, errno is not set, better to use fprintf(stderr, ...) printing the expected and received result.
> Take a look at test_stream_connection_reset()

Ack, fix it in v2

> 
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    if (errno != ENOMEM) {
>> +        perror("invalid errno of 'broken_buf'");
> 
> Instead of "invalid", I would say "unexpected".

Ack

> 
>> +        exit(EXIT_FAILURE);
>> +    }
> 
> 
>> +
>> +    /* Try to fill valid buffer. */
>> +    res = read(fd, valid_buf, buf_size);
>> +    if (res != buf_size) {
>> +        perror("invalid read result of 'valid_buf'");
> 
> I would split in 2 checks:
> - (res < 0) then use perror()
> - (res != buf_size) then use fprintf(stderr, ...) printing the expected   and received result.

Ack

> 
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    for (i = 0; i < buf_size; i++) {
>> +        if (valid_buf[i] != BUF_PATTERN_2) {
>> +            perror("invalid pattern for valid buf");
> 
> errno is not set here, better to use fprintf(stderr, ...)

Ack

> 
>> +            exit(EXIT_FAILURE);
>> +        }
>> +    }
> 
> What about replace this for with a memcmp()?

Ack

> 
>> +
>> +
>> +    /* Unmap buffers. */
>> +    munmap(broken_buf, page_size);
>> +    munmap(broken_buf + page_size * 2, page_size);
>> +    munmap(valid_buf, buf_size);
>> +    close(fd);
>> +}
>> +
>> static struct test_case test_cases[] = {
>>     {
>>         .name = "SOCK_STREAM connection reset",
>> @@ -480,6 +596,11 @@ static struct test_case test_cases[] = {
>>         .run_client = test_seqpacket_timeout_client,
>>         .run_server = test_seqpacket_timeout_server,
>>     },
>> +    {
>> +        .name = "SOCK_SEQPACKET invalid receive buffer",
>> +        .run_client = test_seqpacket_invalid_rec_buffer_client,
>> +        .run_server = test_seqpacket_invalid_rec_buffer_server,
>> +    },
> 
> 
> Is this the right behavior? If read() fails because the buffer is invalid, do we throw out the whole packet?
> 
> I was expecting the packet not to be consumed, have you tried AF_UNIX, does it have the same behavior?

I've just checked AF_UNIX implementation of SEQPACKET receive in net/unix/af_unix.c. So, if 'skb_copy_datagram_msg()'
fails, it calls 'skb_free_datagram()'. I think this means that whole sk buff will be dropped, but anyway, i'll check
this behaviour in practice. See '__unix_dgram_recvmsg()' in net/unix/af_unix.c.


> 
> Thanks,
> Stefano
> 

Re: [RFC PATCH v1 3/3] af_vsock: SOCK_SEQPACKET broken buffer test

[Stefano Garzarella]

On Tue, Mar 15, 2022 at 09:34:35AM +0000, Krasnov Arseniy Vladimirovich 
wrote:
>On 15.03.2022 11:36, Stefano Garzarella wrote:
>>
>> Is this the right behavior? If read() fails because the buffer is invalid, do we throw out the whole packet?
>>
>> I was expecting the packet not to be consumed, have you tried AF_UNIX, does it have the same behavior?
>
>I've just checked AF_UNIX implementation of SEQPACKET receive in net/unix/af_unix.c. So, if 'skb_copy_datagram_msg()'
>fails, it calls 'skb_free_datagram()'. I think this means that whole sk buff will be dropped, but anyway, i'll check
>this behaviour in practice. See '__unix_dgram_recvmsg()' in net/unix/af_unix.c.
>

Yep. you are right it seems to be discarded but I don't know that
code very well, so better to test as you said ;-)

Thanks,
Stefano

Re: [RFC PATCH v1 3/3] af_vsock: SOCK_SEQPACKET broken buffer test

[Krasnov Arseniy Vladimirovich]

On 15.03.2022 12:35, Arseniy Krasnov wrote:
> On 15.03.2022 11:36, Stefano Garzarella wrote:
>> On Fri, Mar 11, 2022 at 10:58:32AM +0000, Krasnov Arseniy Vladimirovich wrote:
>>> Add test where sender sends two message, each with own
>>> data pattern. Reader tries to read first to broken buffer:
>>> it has three pages size, but middle page is unmapped. Then,
>>> reader tries to read second message to valid buffer. Test
>>> checks, that uncopied part of first message was dropped
>>> and thus not copied as part of second message.
>>>
>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>>> ---
>>> tools/testing/vsock/vsock_test.c | 121 +++++++++++++++++++++++++++++++
>>> 1 file changed, 121 insertions(+)
>>>
>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>> index aa2de27d0f77..686af712b4ad 100644
>>> --- a/tools/testing/vsock/vsock_test.c
>>> +++ b/tools/testing/vsock/vsock_test.c
>>> @@ -16,6 +16,7 @@
>>> #include <linux/kernel.h>
>>> #include <sys/types.h>
>>> #include <sys/socket.h>
>>> +#include <sys/mman.h>
>>>
>>> #include "timeout.h"
>>> #include "control.h"
>>> @@ -435,6 +436,121 @@ static void test_seqpacket_timeout_server(const struct test_opts *opts)
>>>     close(fd);
>>> }
>>>
>>> +#define BUF_PATTERN_1 'a'
>>> +#define BUF_PATTERN_2 'b'
>>> +
>>> +static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opts)
>>> +{
>>> +    int fd;
>>> +    unsigned char *buf1;
>>> +    unsigned char *buf2;
>>> +    int buf_size = getpagesize() * 3;
>>> +
>>> +    fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>>> +    if (fd < 0) {
>>> +        perror("connect");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    buf1 = malloc(buf_size);
>>> +    if (buf1 == NULL) {
>>> +        perror("'malloc()' for 'buf1'");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    buf2 = malloc(buf_size);
>>> +    if (buf2 == NULL) {
>>> +        perror("'malloc()' for 'buf2'");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    memset(buf1, BUF_PATTERN_1, buf_size);
>>> +    memset(buf2, BUF_PATTERN_2, buf_size);
>>> +
>>> +    if (send(fd, buf1, buf_size, 0) != buf_size) {
>>> +        perror("send failed");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    if (send(fd, buf2, buf_size, 0) != buf_size) {
>>> +        perror("send failed");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    close(fd);
>>> +}
>>> +
>>> +static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts *opts)
>>> +{
>>> +    int fd;
>>> +    unsigned char *broken_buf;
>>> +    unsigned char *valid_buf;
>>> +    int page_size = getpagesize();
>>> +    int buf_size = page_size * 3;
>>> +    ssize_t res;
>>> +    int prot = PROT_READ | PROT_WRITE;
>>> +    int flags = MAP_PRIVATE | MAP_ANONYMOUS;
>>> +    int i;
>>> +
>>> +    fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
>>> +    if (fd < 0) {
>>> +        perror("accept");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    /* Setup first buffer. */
>>> +    broken_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>>> +    if (broken_buf == MAP_FAILED) {
>>> +        perror("mmap for 'broken_buf'");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    /* Unmap "hole" in buffer. */
>>> +    if (munmap(broken_buf + page_size, page_size)) {
>>> +        perror("'broken_buf' setup");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    valid_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>>> +    if (valid_buf == MAP_FAILED) {
>>> +        perror("mmap for 'valid_buf'");
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    /* Try to fill buffer with unmapped middle. */
>>> +    res = read(fd, broken_buf, buf_size);
>>> +    if (res != -1) {
>>> +        perror("invalid read result of 'broken_buf'");
>>
>> if `res` is valid, errno is not set, better to use fprintf(stderr, ...) printing the expected and received result.
>> Take a look at test_stream_connection_reset()
> 
> Ack, fix it in v2
> 
>>
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    if (errno != ENOMEM) {
>>> +        perror("invalid errno of 'broken_buf'");
>>
>> Instead of "invalid", I would say "unexpected".
> 
> Ack
> 
>>
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>
>>
>>> +
>>> +    /* Try to fill valid buffer. */
>>> +    res = read(fd, valid_buf, buf_size);
>>> +    if (res != buf_size) {
>>> +        perror("invalid read result of 'valid_buf'");
>>
>> I would split in 2 checks:
>> - (res < 0) then use perror()
>> - (res != buf_size) then use fprintf(stderr, ...) printing the expected   and received result.
> 
> Ack
> 
>>
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>> +    for (i = 0; i < buf_size; i++) {
>>> +        if (valid_buf[i] != BUF_PATTERN_2) {
>>> +            perror("invalid pattern for valid buf");
>>
>> errno is not set here, better to use fprintf(stderr, ...)
> 
> Ack
> 
>>
>>> +            exit(EXIT_FAILURE);
>>> +        }
>>> +    }
>>
>> What about replace this for with a memcmp()?

memcmp() will require special buffer with BUF_PATTERN_2 data as
second argument. I think 'if()' condition is better here, as we
compare each element of buffer with single byte. Anyway, 'memcmp()'
does the same things inside itself.

> 
> Ack
> 
>>
>>> +
>>> +
>>> +    /* Unmap buffers. */
>>> +    munmap(broken_buf, page_size);
>>> +    munmap(broken_buf + page_size * 2, page_size);
>>> +    munmap(valid_buf, buf_size);
>>> +    close(fd);
>>> +}
>>> +
>>> static struct test_case test_cases[] = {
>>>     {
>>>         .name = "SOCK_STREAM connection reset",
>>> @@ -480,6 +596,11 @@ static struct test_case test_cases[] = {
>>>         .run_client = test_seqpacket_timeout_client,
>>>         .run_server = test_seqpacket_timeout_server,
>>>     },
>>> +    {
>>> +        .name = "SOCK_SEQPACKET invalid receive buffer",
>>> +        .run_client = test_seqpacket_invalid_rec_buffer_client,
>>> +        .run_server = test_seqpacket_invalid_rec_buffer_server,
>>> +    },
>>
>>
>> Is this the right behavior? If read() fails because the buffer is invalid, do we throw out the whole packet?
>>
>> I was expecting the packet not to be consumed, have you tried AF_UNIX, does it have the same behavior?
> 
> I've just checked AF_UNIX implementation of SEQPACKET receive in net/unix/af_unix.c. So, if 'skb_copy_datagram_msg()'
> fails, it calls 'skb_free_datagram()'. I think this means that whole sk buff will be dropped, but anyway, i'll check
> this behaviour in practice. See '__unix_dgram_recvmsg()' in net/unix/af_unix.c.

So i've checked that assumption for SEQPACKET + AF_UNIX: when user passes broken buffer to
the kernel(for example with unmapped page in the mid), rest of message will be dropped. Next
read will never get tail of the dropped message.

> 
> 
>>
>> Thanks,
>> Stefano
>>
> 

Re: [RFC PATCH v1 3/3] af_vsock: SOCK_SEQPACKET broken buffer test

[Stefano Garzarella]

On Tue, Mar 15, 2022 at 12:43:13PM +0000, Krasnov Arseniy Vladimirovich wrote:
>On 15.03.2022 12:35, Arseniy Krasnov wrote:
>> On 15.03.2022 11:36, Stefano Garzarella wrote:
>>> On Fri, Mar 11, 2022 at 10:58:32AM +0000, Krasnov Arseniy Vladimirovich wrote:
>>>> Add test where sender sends two message, each with own
>>>> data pattern. Reader tries to read first to broken buffer:
>>>> it has three pages size, but middle page is unmapped. Then,
>>>> reader tries to read second message to valid buffer. Test
>>>> checks, that uncopied part of first message was dropped
>>>> and thus not copied as part of second message.
>>>>
>>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>>>> ---
>>>> tools/testing/vsock/vsock_test.c | 121 +++++++++++++++++++++++++++++++
>>>> 1 file changed, 121 insertions(+)
>>>>
>>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>>> index aa2de27d0f77..686af712b4ad 100644
>>>> --- a/tools/testing/vsock/vsock_test.c
>>>> +++ b/tools/testing/vsock/vsock_test.c
>>>> @@ -16,6 +16,7 @@
>>>> #include <linux/kernel.h>
>>>> #include <sys/types.h>
>>>> #include <sys/socket.h>
>>>> +#include <sys/mman.h>
>>>>
>>>> #include "timeout.h"
>>>> #include "control.h"
>>>> @@ -435,6 +436,121 @@ static void test_seqpacket_timeout_server(const struct test_opts *opts)
>>>>     close(fd);
>>>> }
>>>>
>>>> +#define BUF_PATTERN_1 'a'
>>>> +#define BUF_PATTERN_2 'b'
>>>> +
>>>> +static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opts)
>>>> +{
>>>> +    int fd;
>>>> +    unsigned char *buf1;
>>>> +    unsigned char *buf2;
>>>> +    int buf_size = getpagesize() * 3;
>>>> +
>>>> +    fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>>>> +    if (fd < 0) {
>>>> +        perror("connect");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    buf1 = malloc(buf_size);
>>>> +    if (buf1 == NULL) {
>>>> +        perror("'malloc()' for 'buf1'");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    buf2 = malloc(buf_size);
>>>> +    if (buf2 == NULL) {
>>>> +        perror("'malloc()' for 'buf2'");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    memset(buf1, BUF_PATTERN_1, buf_size);
>>>> +    memset(buf2, BUF_PATTERN_2, buf_size);
>>>> +
>>>> +    if (send(fd, buf1, buf_size, 0) != buf_size) {
>>>> +        perror("send failed");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    if (send(fd, buf2, buf_size, 0) != buf_size) {
>>>> +        perror("send failed");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    close(fd);
>>>> +}
>>>> +
>>>> +static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts *opts)
>>>> +{
>>>> +    int fd;
>>>> +    unsigned char *broken_buf;
>>>> +    unsigned char *valid_buf;
>>>> +    int page_size = getpagesize();
>>>> +    int buf_size = page_size * 3;
>>>> +    ssize_t res;
>>>> +    int prot = PROT_READ | PROT_WRITE;
>>>> +    int flags = MAP_PRIVATE | MAP_ANONYMOUS;
>>>> +    int i;
>>>> +
>>>> +    fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
>>>> +    if (fd < 0) {
>>>> +        perror("accept");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    /* Setup first buffer. */
>>>> +    broken_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>>>> +    if (broken_buf == MAP_FAILED) {
>>>> +        perror("mmap for 'broken_buf'");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    /* Unmap "hole" in buffer. */
>>>> +    if (munmap(broken_buf + page_size, page_size)) {
>>>> +        perror("'broken_buf' setup");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    valid_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
>>>> +    if (valid_buf == MAP_FAILED) {
>>>> +        perror("mmap for 'valid_buf'");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    /* Try to fill buffer with unmapped middle. */
>>>> +    res = read(fd, broken_buf, buf_size);
>>>> +    if (res != -1) {
>>>> +        perror("invalid read result of 'broken_buf'");
>>>
>>> if `res` is valid, errno is not set, better to use fprintf(stderr, ...) printing the expected and received result.
>>> Take a look at test_stream_connection_reset()
>>
>> Ack, fix it in v2
>>
>>>
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    if (errno != ENOMEM) {
>>>> +        perror("invalid errno of 'broken_buf'");
>>>
>>> Instead of "invalid", I would say "unexpected".
>>
>> Ack
>>
>>>
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>
>>>
>>>> +
>>>> +    /* Try to fill valid buffer. */
>>>> +    res = read(fd, valid_buf, buf_size);
>>>> +    if (res != buf_size) {
>>>> +        perror("invalid read result of 'valid_buf'");
>>>
>>> I would split in 2 checks:
>>> - (res < 0) then use perror()
>>> - (res != buf_size) then use fprintf(stderr, ...) printing the expected   and received result.
>>
>> Ack
>>
>>>
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>> +
>>>> +    for (i = 0; i < buf_size; i++) {
>>>> +        if (valid_buf[i] != BUF_PATTERN_2) {
>>>> +            perror("invalid pattern for valid buf");
>>>
>>> errno is not set here, better to use fprintf(stderr, ...)
>>
>> Ack
>>
>>>
>>>> +            exit(EXIT_FAILURE);
>>>> +        }
>>>> +    }
>>>
>>> What about replace this for with a memcmp()?
>
>memcmp() will require special buffer with BUF_PATTERN_2 data as
>second argument. I think 'if()' condition is better here, as we
>compare each element of buffer with single byte. Anyway, 'memcmp()'
>does the same things inside itself.

Ah, I see. Okay, I agree on for()/if(), maybe we can also print more 
info (index, expected value, current value).

>
>>
>> Ack
>>
>>>
>>>> +
>>>> +
>>>> +    /* Unmap buffers. */
>>>> +    munmap(broken_buf, page_size);
>>>> +    munmap(broken_buf + page_size * 2, page_size);
>>>> +    munmap(valid_buf, buf_size);
>>>> +    close(fd);
>>>> +}
>>>> +
>>>> static struct test_case test_cases[] = {
>>>>     {
>>>>         .name = "SOCK_STREAM connection reset",
>>>> @@ -480,6 +596,11 @@ static struct test_case test_cases[] = {
>>>>         .run_client = test_seqpacket_timeout_client,
>>>>         .run_server = test_seqpacket_timeout_server,
>>>>     },
>>>> +    {
>>>> +        .name = "SOCK_SEQPACKET invalid receive buffer",
>>>> +        .run_client = test_seqpacket_invalid_rec_buffer_client,
>>>> +        .run_server = test_seqpacket_invalid_rec_buffer_server,
>>>> +    },
>>>
>>>
>>> Is this the right behavior? If read() fails because the buffer is invalid, do we throw out the whole packet?
>>>
>>> I was expecting the packet not to be consumed, have you tried AF_UNIX, does it have the same behavior?
>>
>> I've just checked AF_UNIX implementation of SEQPACKET receive in net/unix/af_unix.c. So, if 'skb_copy_datagram_msg()'
>> fails, it calls 'skb_free_datagram()'. I think this means that whole sk buff will be dropped, but anyway, i'll check
>> this behaviour in practice. See '__unix_dgram_recvmsg()' in net/unix/af_unix.c.
>
>So i've checked that assumption for SEQPACKET + AF_UNIX: when user passes broken buffer to
>the kernel(for example with unmapped page in the mid), rest of message will be dropped. Next
>read will never get tail of the dropped message.

Thanks for checking, so it seems the same behaviour.
Let's go ahead with this test :-)

Stefano

Re: [RFC PATCH v1 1/3] af_vsock: add two new tests for SOCK_SEQPACKET

[Stefano Garzarella]

Hi Arseniy,

On Fri, Mar 11, 2022 at 10:52:36AM +0000, Krasnov Arseniy Vladimirovich wrote:
>This adds two tests: for receive timeout and reading to invalid
>buffer provided by user. I forgot to put both patches to main
>patchset.
>
>Arseniy Krasnov(2):
>
>af_vsock: SOCK_SEQPACKET receive timeout test
>af_vsock: SOCK_SEQPACKET broken buffer test
>
>tools/testing/vsock/vsock_test.c | 170 +++++++++++++++++++++++++++++++++++++++
>1 file changed, 170 insertions(+)

Thank you for these tests!

I left a few comments and I'm not sure about the 'broken buffer test' 
behavior.

About the series, it sounds like something is wrong with your setup, 
usually the cover letter is "patch" 0. In this case I would have 
expected:

     [0/2] af_vsock: add two new tests for SOCK_SEQPACKET
     [1/2] af_vsock: SOCK_SEQPACKET receive timeout test
     [2/2] af_vsock: SOCK_SEQPACKET broken buffer test

Are you using `git send-email` or `git publish`?


When you will remove the RFC, please add `net-next` label:
[PATCH net-next 0/2], etc..

Thanks,
Stefano

Re: [RFC PATCH v1 1/3] af_vsock: add two new tests for SOCK_SEQPACKET

[Krasnov Arseniy Vladimirovich]

On 15.03.2022 11:42, Stefano Garzarella wrote:
> Hi Arseniy,
> 
> On Fri, Mar 11, 2022 at 10:52:36AM +0000, Krasnov Arseniy Vladimirovich wrote:
>> This adds two tests: for receive timeout and reading to invalid
>> buffer provided by user. I forgot to put both patches to main
>> patchset.
>>
>> Arseniy Krasnov(2):
>>
>> af_vsock: SOCK_SEQPACKET receive timeout test
>> af_vsock: SOCK_SEQPACKET broken buffer test
>>
>> tools/testing/vsock/vsock_test.c | 170 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 170 insertions(+)
> 
> Thank you for these tests!

Great! Thank You

> 
> I left a few comments and I'm not sure about the 'broken buffer test' behavior.

Ack

> 
> About the series, it sounds like something is wrong with your setup, usually the cover letter is "patch" 0. In this case I would have expected:
> 
>     [0/2] af_vsock: add two new tests for SOCK_SEQPACKET
>     [1/2] af_vsock: SOCK_SEQPACKET receive timeout test
>     [2/2] af_vsock: SOCK_SEQPACKET broken buffer test

Ack

> 
> Are you using `git send-email` or `git publish`?
> 

I'm using thunderbird to send patches><, because we don't have SMTP server(exchange only). 

> 
> When you will remove the RFC, please add `net-next` label:
> [PATCH net-next 0/2], etc..
> 
> Thanks,
> Stefano
>