[RFC PATCH v2 0/2] af_vsock: add two new tests for SOCK_SEQPACKET

[RFC PATCH v2 0/2] 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 | 211 +++++++++++++++++++++++++++++++++++++++
1 file changed, 211 insertions(+)

v1 -> v2:
 see every patch after '---' line.

-- 
2.25.1

[RFC PATCH v2 1/2] 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>
---
 v1 -> v2:
 1) Check amount of time spent in 'read()'.

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

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 2a3638c0a008..6d7648cce5aa 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 <time.h>
 
 #include "timeout.h"
 #include "control.h"
@@ -391,6 +392,79 @@ static void test_seqpacket_msg_trunc_server(const struct test_opts *opts)
 	close(fd);
 }
 
+static time_t current_nsec(void)
+{
+	struct timespec ts;
+
+	if (clock_gettime(CLOCK_REALTIME, &ts)) {
+		perror("clock_gettime(3) failed");
+		exit(EXIT_FAILURE);
+	}
+
+	return (ts.tv_sec * 1000000000ULL) + ts.tv_nsec;
+}
+
+#define RCVTIMEO_TIMEOUT_SEC 1
+#define READ_OVERHEAD_NSEC 250000000 /* 0.25 sec */
+
+static void test_seqpacket_timeout_client(const struct test_opts *opts)
+{
+	int fd;
+	struct timeval tv;
+	char dummy;
+	time_t read_enter_ns;
+	time_t read_overhead_ns;
+
+	fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
+	if (fd < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	tv.tv_sec = RCVTIMEO_TIMEOUT_SEC;
+	tv.tv_usec = 0;
+
+	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)) == -1) {
+		perror("setsockopt 'SO_RCVTIMEO'");
+		exit(EXIT_FAILURE);
+	}
+
+	read_enter_ns = current_nsec();
+
+	if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
+	    (errno != EAGAIN)) {
+		perror("EAGAIN expected");
+		exit(EXIT_FAILURE);
+	}
+
+	read_overhead_ns = current_nsec() - read_enter_ns -
+			1000000000ULL * RCVTIMEO_TIMEOUT_SEC;
+
+	if (read_overhead_ns > READ_OVERHEAD_NSEC) {
+		fprintf(stderr,
+			"too much time in read(2) with SO_RCVTIMEO: %lu ns\n",
+			read_overhead_ns);
+		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 +505,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

[RFC PATCH v2 2/2] 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>
---
 v1 -> v2:
 1) Use 'fprintf()' instead of 'perror()' where 'errno' variable
    is not affected.
 2) Replace word "invalid" -> "unexpected".
    
 tools/testing/vsock/vsock_test.c | 132 +++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 6d7648cce5aa..1132bcd8ddb7 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -17,6 +17,7 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <time.h>
+#include <sys/mman.h>
 
 #include "timeout.h"
 #include "control.h"
@@ -465,6 +466,132 @@ 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) {
+		fprintf(stderr,
+			"expected 'broken_buf' read(2) failure, got %zi\n",
+			res);
+		exit(EXIT_FAILURE);
+	}
+
+	if (errno != ENOMEM) {
+		perror("unexpected errno of 'broken_buf'");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Try to fill valid buffer. */
+	res = read(fd, valid_buf, buf_size);
+	if (res < 0) {
+		perror("unexpected 'valid_buf' read(2) failure");
+		exit(EXIT_FAILURE);
+	}
+
+	if (res != buf_size) {
+		fprintf(stderr,
+			"invalid 'valid_buf' read(2), got %zi, expected %i\n",
+			res, buf_size);
+		exit(EXIT_FAILURE);
+	}
+
+	for (i = 0; i < buf_size; i++) {
+		if (valid_buf[i] != BUF_PATTERN_2) {
+			fprintf(stderr,
+				"invalid pattern for 'valid_buf' at %i, expected %hhX, got %hhX\n",
+				i, BUF_PATTERN_2, valid_buf[i]);
+			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",
@@ -510,6 +637,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 v2 1/2] af_vsock: SOCK_SEQPACKET receive timeout test

[Stefano Garzarella]

On Wed, Mar 16, 2022 at 07:27:45AM +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>
>---
> v1 -> v2:
> 1) Check amount of time spent in 'read()'.

The patch looks correct to me, but since it's an RFC and you have to 
send another version anyway, here are some minor suggestions :-)

>
> tools/testing/vsock/vsock_test.c | 79 ++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 2a3638c0a008..6d7648cce5aa 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 <time.h>
>
> #include "timeout.h"
> #include "control.h"
>@@ -391,6 +392,79 @@ static void test_seqpacket_msg_trunc_server(const struct test_opts *opts)
> 	close(fd);
> }
>
>+static time_t current_nsec(void)
>+{
>+	struct timespec ts;
>+
>+	if (clock_gettime(CLOCK_REALTIME, &ts)) {
>+		perror("clock_gettime(3) failed");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	return (ts.tv_sec * 1000000000ULL) + ts.tv_nsec;
>+}
>+
>+#define RCVTIMEO_TIMEOUT_SEC 1
>+#define READ_OVERHEAD_NSEC 250000000 /* 0.25 sec */
>+
>+static void test_seqpacket_timeout_client(const struct test_opts *opts)
>+{
>+	int fd;
>+	struct timeval tv;
>+	char dummy;
>+	time_t read_enter_ns;
>+	time_t read_overhead_ns;
>+
>+	fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	tv.tv_sec = RCVTIMEO_TIMEOUT_SEC;
>+	tv.tv_usec = 0;
>+
>+	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)) == -1) {
>+		perror("setsockopt 'SO_RCVTIMEO'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	read_enter_ns = current_nsec();
>+
>+	if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
>+	    (errno != EAGAIN)) {

Here we can split in 2 checks like in patch 2, since if read() return 
value is >= 0, errno is not set.

>+		perror("EAGAIN expected");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	read_overhead_ns = current_nsec() - read_enter_ns -
>+			1000000000ULL * RCVTIMEO_TIMEOUT_SEC;
>+
>+	if (read_overhead_ns > READ_OVERHEAD_NSEC) {
>+		fprintf(stderr,
>+			"too much time in read(2) with SO_RCVTIMEO: %lu ns\n",
>+			read_overhead_ns);

What about printing also the expected overhead?

>+		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 +505,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 v2 1/2] af_vsock: SOCK_SEQPACKET receive timeout test

[Stefano Garzarella]

On Wed, Mar 16, 2022 at 07:27:45AM +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>
>---
> v1 -> v2:
> 1) Check amount of time spent in 'read()'.

The patch looks correct to me, but since it's an RFC and you have to 
send another version anyway, here are some minor suggestions :-)

>
> tools/testing/vsock/vsock_test.c | 79 ++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 2a3638c0a008..6d7648cce5aa 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 <time.h>
>
> #include "timeout.h"
> #include "control.h"
>@@ -391,6 +392,79 @@ static void test_seqpacket_msg_trunc_server(const struct test_opts *opts)
> 	close(fd);
> }
>
>+static time_t current_nsec(void)
>+{
>+	struct timespec ts;
>+
>+	if (clock_gettime(CLOCK_REALTIME, &ts)) {
>+		perror("clock_gettime(3) failed");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	return (ts.tv_sec * 1000000000ULL) + ts.tv_nsec;
>+}
>+
>+#define RCVTIMEO_TIMEOUT_SEC 1
>+#define READ_OVERHEAD_NSEC 250000000 /* 0.25 sec */
>+
>+static void test_seqpacket_timeout_client(const struct test_opts *opts)
>+{
>+	int fd;
>+	struct timeval tv;
>+	char dummy;
>+	time_t read_enter_ns;
>+	time_t read_overhead_ns;
>+
>+	fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	tv.tv_sec = RCVTIMEO_TIMEOUT_SEC;
>+	tv.tv_usec = 0;
>+
>+	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)) == -1) {
>+		perror("setsockopt 'SO_RCVTIMEO'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	read_enter_ns = current_nsec();
>+
>+	if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
>+	    (errno != EAGAIN)) {

Here we can split in 2 checks like in patch 2, since if read() return 
value is >= 0, errno is not set.

>+		perror("EAGAIN expected");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	read_overhead_ns = current_nsec() - read_enter_ns -
>+			1000000000ULL * RCVTIMEO_TIMEOUT_SEC;
>+
>+	if (read_overhead_ns > READ_OVERHEAD_NSEC) {
>+		fprintf(stderr,
>+			"too much time in read(2) with SO_RCVTIMEO: %lu ns\n",
>+			read_overhead_ns);

What about printing also the expected overhead?

>+		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 +505,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

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH v2 2/2] af_vsock: SOCK_SEQPACKET broken buffer test

[Stefano Garzarella]

On Wed, Mar 16, 2022 at 07:29:28AM +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>
>---
> v1 -> v2:
> 1) Use 'fprintf()' instead of 'perror()' where 'errno' variable
>    is not affected.
> 2) Replace word "invalid" -> "unexpected".
>
> tools/testing/vsock/vsock_test.c | 132 +++++++++++++++++++++++++++++++
> 1 file changed, 132 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 6d7648cce5aa..1132bcd8ddb7 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -17,6 +17,7 @@
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <time.h>
>+#include <sys/mman.h>
>
> #include "timeout.h"
> #include "control.h"
>@@ -465,6 +466,132 @@ 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) {
>+		fprintf(stderr,
>+			"expected 'broken_buf' read(2) failure, got %zi\n",
>+			res);
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (errno != ENOMEM) {
>+		perror("unexpected errno of 'broken_buf'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	/* Try to fill valid buffer. */
>+	res = read(fd, valid_buf, buf_size);
>+	if (res < 0) {
>+		perror("unexpected 'valid_buf' read(2) failure");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (res != buf_size) {
>+		fprintf(stderr,
>+			"invalid 'valid_buf' read(2), got %zi, expected %i\n",
>+			res, buf_size);

I would suggest to use always the same pattern in the error messages:
"expected X, got Y".

The rest LGTM.

>+		exit(EXIT_FAILURE);
>+	}
>+
>+	for (i = 0; i < buf_size; i++) {
>+		if (valid_buf[i] != BUF_PATTERN_2) {
>+			fprintf(stderr,
>+				"invalid pattern for 'valid_buf' at %i, expected %hhX, got %hhX\n",
>+				i, BUF_PATTERN_2, valid_buf[i]);
>+			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",
>@@ -510,6 +637,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 v2 2/2] af_vsock: SOCK_SEQPACKET broken buffer test

[Stefano Garzarella]

On Wed, Mar 16, 2022 at 07:29:28AM +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>
>---
> v1 -> v2:
> 1) Use 'fprintf()' instead of 'perror()' where 'errno' variable
>    is not affected.
> 2) Replace word "invalid" -> "unexpected".
>
> tools/testing/vsock/vsock_test.c | 132 +++++++++++++++++++++++++++++++
> 1 file changed, 132 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 6d7648cce5aa..1132bcd8ddb7 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -17,6 +17,7 @@
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <time.h>
>+#include <sys/mman.h>
>
> #include "timeout.h"
> #include "control.h"
>@@ -465,6 +466,132 @@ 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) {
>+		fprintf(stderr,
>+			"expected 'broken_buf' read(2) failure, got %zi\n",
>+			res);
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (errno != ENOMEM) {
>+		perror("unexpected errno of 'broken_buf'");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	/* Try to fill valid buffer. */
>+	res = read(fd, valid_buf, buf_size);
>+	if (res < 0) {
>+		perror("unexpected 'valid_buf' read(2) failure");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (res != buf_size) {
>+		fprintf(stderr,
>+			"invalid 'valid_buf' read(2), got %zi, expected %i\n",
>+			res, buf_size);

I would suggest to use always the same pattern in the error messages:
"expected X, got Y".

The rest LGTM.

>+		exit(EXIT_FAILURE);
>+	}
>+
>+	for (i = 0; i < buf_size; i++) {
>+		if (valid_buf[i] != BUF_PATTERN_2) {
>+			fprintf(stderr,
>+				"invalid pattern for 'valid_buf' at %i, expected %hhX, got %hhX\n",
>+				i, BUF_PATTERN_2, valid_buf[i]);
>+			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",
>@@ -510,6 +637,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

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH v2 0/2] af_vsock: add two new tests for SOCK_SEQPACKET

[Stefano Garzarella]

On Wed, Mar 16, 2022 at 07:25:07AM +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 | 211 +++++++++++++++++++++++++++++++++++++++
>1 file changed, 211 insertions(+)

I think there are only small things to fix, so next series you can 
remove RFC (remember to use net-next).

I added the tests to my suite and everything is running correctly.

I also suggest you to solve these little issues that checkpatch has 
highlighted to have patches ready for submission :-)

Thanks,
Stefano

$ ./scripts/checkpatch.pl --strict -g  master..HEAD
---------------------------------------------------------------------
Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test")
---------------------------------------------------------------------
CHECK: Unnecessary parentheses around 'errno != EAGAIN'
#70: FILE: tools/testing/vsock/vsock_test.c:434:
+	if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
+	   (errno != EAGAIN)) {

WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich <AVKrasnov@sberdevices.ru>' != 'Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>'

total: 0 errors, 1 warnings, 1 checks, 97 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
       mechanically convert to the typical style using --fix or --fix-inplace.

Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test") has style problems, please review.
-------------------------------------------------------------------
Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test")
-------------------------------------------------------------------
CHECK: Comparison to NULL could be written "!buf1"
#51: FILE: tools/testing/vsock/vsock_test.c:486:
+	if (buf1 == NULL) {

CHECK: Comparison to NULL could be written "!buf2"
#57: FILE: tools/testing/vsock/vsock_test.c:492:
+	if (buf2 == NULL) {

CHECK: Please don't use multiple blank lines
#152: FILE: tools/testing/vsock/vsock_test.c:587:
+
+

WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich <AVKrasnov@sberdevices.ru>' != 'Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>'

total: 0 errors, 1 warnings, 3 checks, 150 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
       mechanically convert to the typical style using --fix or --fix-inplace.

Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test") has style problems, please review.

NOTE: If any of the errors are false positives, please report
       them to the maintainer, see CHECKPATCH in MAINTAINERS.

Re: [RFC PATCH v2 0/2] af_vsock: add two new tests for SOCK_SEQPACKET

[Stefano Garzarella]

On Wed, Mar 16, 2022 at 07:25:07AM +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 | 211 +++++++++++++++++++++++++++++++++++++++
>1 file changed, 211 insertions(+)

I think there are only small things to fix, so next series you can 
remove RFC (remember to use net-next).

I added the tests to my suite and everything is running correctly.

I also suggest you to solve these little issues that checkpatch has 
highlighted to have patches ready for submission :-)

Thanks,
Stefano

$ ./scripts/checkpatch.pl --strict -g  master..HEAD
---------------------------------------------------------------------
Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test")
---------------------------------------------------------------------
CHECK: Unnecessary parentheses around 'errno != EAGAIN'
#70: FILE: tools/testing/vsock/vsock_test.c:434:
+	if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
+	   (errno != EAGAIN)) {

WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich <AVKrasnov@sberdevices.ru>' != 'Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>'

total: 0 errors, 1 warnings, 1 checks, 97 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
       mechanically convert to the typical style using --fix or --fix-inplace.

Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test") has style problems, please review.
-------------------------------------------------------------------
Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test")
-------------------------------------------------------------------
CHECK: Comparison to NULL could be written "!buf1"
#51: FILE: tools/testing/vsock/vsock_test.c:486:
+	if (buf1 == NULL) {

CHECK: Comparison to NULL could be written "!buf2"
#57: FILE: tools/testing/vsock/vsock_test.c:492:
+	if (buf2 == NULL) {

CHECK: Please don't use multiple blank lines
#152: FILE: tools/testing/vsock/vsock_test.c:587:
+
+

WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich <AVKrasnov@sberdevices.ru>' != 'Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>'

total: 0 errors, 1 warnings, 3 checks, 150 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
       mechanically convert to the typical style using --fix or --fix-inplace.

Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test") has style problems, please review.

NOTE: If any of the errors are false positives, please report
       them to the maintainer, see CHECKPATCH in MAINTAINERS.




_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH v2 1/2] af_vsock: SOCK_SEQPACKET receive timeout test

[Krasnov Arseniy Vladimirovich]

On 16.03.2022 11:51, Stefano Garzarella wrote:
> On Wed, Mar 16, 2022 at 07:27:45AM +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>
>> ---
>> v1 -> v2:
>> 1) Check amount of time spent in 'read()'.
> 
> The patch looks correct to me, but since it's an RFC and you have to send another version anyway, here are some minor suggestions :-)
> 

Ok, i'll prepare next version with fixes :)

>>
>> tools/testing/vsock/vsock_test.c | 79 ++++++++++++++++++++++++++++++++
>> 1 file changed, 79 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index 2a3638c0a008..6d7648cce5aa 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 <time.h>
>>
>> #include "timeout.h"
>> #include "control.h"
>> @@ -391,6 +392,79 @@ static void test_seqpacket_msg_trunc_server(const struct test_opts *opts)
>>     close(fd);
>> }
>>
>> +static time_t current_nsec(void)
>> +{
>> +    struct timespec ts;
>> +
>> +    if (clock_gettime(CLOCK_REALTIME, &ts)) {
>> +        perror("clock_gettime(3) failed");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    return (ts.tv_sec * 1000000000ULL) + ts.tv_nsec;
>> +}
>> +
>> +#define RCVTIMEO_TIMEOUT_SEC 1
>> +#define READ_OVERHEAD_NSEC 250000000 /* 0.25 sec */
>> +
>> +static void test_seqpacket_timeout_client(const struct test_opts *opts)
>> +{
>> +    int fd;
>> +    struct timeval tv;
>> +    char dummy;
>> +    time_t read_enter_ns;
>> +    time_t read_overhead_ns;
>> +
>> +    fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>> +    if (fd < 0) {
>> +        perror("connect");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    tv.tv_sec = RCVTIMEO_TIMEOUT_SEC;
>> +    tv.tv_usec = 0;
>> +
>> +    if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)) == -1) {
>> +        perror("setsockopt 'SO_RCVTIMEO'");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    read_enter_ns = current_nsec();
>> +
>> +    if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
>> +        (errno != EAGAIN)) {
> 
> Here we can split in 2 checks like in patch 2, since if read() return value is >= 0, errno is not set.
> 
>> +        perror("EAGAIN expected");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    read_overhead_ns = current_nsec() - read_enter_ns -
>> +            1000000000ULL * RCVTIMEO_TIMEOUT_SEC;
>> +
>> +    if (read_overhead_ns > READ_OVERHEAD_NSEC) {
>> +        fprintf(stderr,
>> +            "too much time in read(2) with SO_RCVTIMEO: %lu ns\n",
>> +            read_overhead_ns);
> 
> What about printing also the expected overhead?
> 
>> +        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 +505,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 v2 0/2] af_vsock: add two new tests for SOCK_SEQPACKET

[Krasnov Arseniy Vladimirovich]

On 16.03.2022 11:58, Stefano Garzarella wrote:
> On Wed, Mar 16, 2022 at 07:25:07AM +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 | 211 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 211 insertions(+)
> 
> I think there are only small things to fix, so next series you can remove RFC (remember to use net-next).
> 
> I added the tests to my suite and everything is running correctly.
> 
> I also suggest you to solve these little issues that checkpatch has highlighted to have patches ready for submission :-)
> 
> Thanks,
> Stefano
> 
> $ ./scripts/checkpatch.pl --strict -g  master..HEAD
> ---------------------------------------------------------------------
> Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test")
> ---------------------------------------------------------------------
> CHECK: Unnecessary parentheses around 'errno != EAGAIN'
> #70: FILE: tools/testing/vsock/vsock_test.c:434:
> +    if ((read(fd, &dummy, sizeof(dummy)) != -1) ||
> +       (errno != EAGAIN)) {
> 
> WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich <AVKrasnov@sberdevices.ru>' != 'Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>'
> 
> total: 0 errors, 1 warnings, 1 checks, 97 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
> 
> Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test") has style problems, please review.
> -------------------------------------------------------------------
> Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test")
> -------------------------------------------------------------------
> CHECK: Comparison to NULL could be written "!buf1"
> #51: FILE: tools/testing/vsock/vsock_test.c:486:
> +    if (buf1 == NULL) {
> 
> CHECK: Comparison to NULL could be written "!buf2"
> #57: FILE: tools/testing/vsock/vsock_test.c:492:
> +    if (buf2 == NULL) {
> 
> CHECK: Please don't use multiple blank lines
> #152: FILE: tools/testing/vsock/vsock_test.c:587:
> +
> +
> 
> WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich <AVKrasnov@sberdevices.ru>' != 'Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>'
> 
> total: 0 errors, 1 warnings, 3 checks, 150 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
> 
> Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test") has style problems, please review.
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
Ack
> 
> 
> 
>