diff mbox series

[v2,1/2] Add shutdown01 test

Message ID 20240607-shutdown-v2-1-a09ce3290ee1@suse.com
State Accepted
Headers show
Series shutdown testing suite | expand

Commit Message

Andrea Cervesato June 7, 2024, 8:13 a.m. UTC 
From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies the following shutdown() functionalities:

- SHUT_RD should enable send() ops but disable recv() ops
- SHUT_WR should enable recv() ops but disable send() ops
- SHUT_RDWR should disable both recv() and send() ops

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                |   2 +
 testcases/kernel/syscalls/shutdown/.gitignore   |   1 +
 testcases/kernel/syscalls/shutdown/Makefile     |   7 ++
 testcases/kernel/syscalls/shutdown/shutdown01.c | 121 ++++++++++++++++++++++++
 4 files changed, 131 insertions(+)

Comments

Petr Vorel June 10, 2024, 1:13 p.m. UTC | #1 
Hi Andrea,

TL;DR: generally LGTM, I have few minor details, which I offer to fix before
merge (diff below).

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> +++ b/testcases/kernel/syscalls/shutdown/shutdown01.c
...
> +static struct tcase {
> +	int shutdown_op;
> +	int recv_flag;
> +	int recv_err;
> +	int send_flag;
> +	int send_err;
> +	char *flag_str;
> +} tcases[] = {
> +	{SHUT_RD, 0, 0, 0, 0, "SHUT_RD"},
> +	{SHUT_WR, MSG_DONTWAIT, EWOULDBLOCK, MSG_NOSIGNAL, EPIPE, "SHUT_WR"},
> +	{SHUT_RDWR, 0, 0, MSG_NOSIGNAL, EPIPE, "SHUT_RDWR"}

Would you mind to use
1) designated initializers (e.g. .recv_flag = MSG_DONTWAIT,)
2) stringify macro

IMHO it helps readability, that's why it's often used.

#define OP_DESC(x) .shutdown_op = x, .desc = #x
static struct tcase {
	int shutdown_op;
	int recv_flag;
	int recv_err;
	int send_flag;
	int send_err;
	char *desc;
} tcases[] = {
	{OP_DESC(SHUT_RD)},
	{OP_DESC(SHUT_WR), .recv_flag = MSG_DONTWAIT, .recv_err = EWOULDBLOCK,
		.send_flag = MSG_NOSIGNAL, .send_err = EPIPE},
	{OP_DESC(SHUT_RDWR), .send_flag = MSG_NOSIGNAL, .send_err = EPIPE}
};

> +};
> +
> +static struct sockaddr_un *sock_addr;
> +
> +static void run_server(void)
> +{
> +	int server_sock;
> +
> +	server_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
> +
> +	SAFE_BIND(server_sock,
> +		(struct sockaddr *)sock_addr,
> +		sizeof(struct sockaddr_un));
> +	SAFE_LISTEN(server_sock, 10);
> +
> +	tst_res(TINFO, "Running server on socket file");
> +
> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
> +
> +	SAFE_CLOSE(server_sock);
> +	SAFE_UNLINK(SOCKETFILE);
> +}
> +
> +static int start_test(void)
> +{
> +	int client_sock;
> +
> +	if (!SAFE_FORK()) {
> +		run_server();
> +		_exit(0);
> +	}
> +
> +	TST_CHECKPOINT_WAIT(0);
> +
> +	tst_res(TINFO, "Connecting to the server");
> +
> +	client_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
> +	SAFE_CONNECT(client_sock,
> +		(struct sockaddr *)sock_addr,
> +		sizeof(struct sockaddr_un));
> +
> +	return client_sock;
> +}
> +
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	int client_sock;
> +	char buff[MSGSIZE] = {0};
> +
> +	client_sock = start_test();
> +
> +	tst_res(TINFO, "Testing %s flag", tc->flag_str);
> +
> +	TST_EXP_PASS(shutdown(client_sock, tc->shutdown_op));
> +
> +	if (!tc->recv_err)
> +		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
> +	else
> +		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);

very nit:

	if (tc->recv_err)
		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
	else
		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);

If you agree, I can merge with following changes.

Kind regards,
Petr

diff --git testcases/kernel/syscalls/shutdown/shutdown01.c testcases/kernel/syscalls/shutdown/shutdown01.c
index ba3853d9c..8e58f23e6 100644
--- testcases/kernel/syscalls/shutdown/shutdown01.c
+++ testcases/kernel/syscalls/shutdown/shutdown01.c
@@ -19,17 +19,19 @@
 #define MSGSIZE 4
 #define SOCKETFILE "socket"
 
+#define OP_DESC(x) .shutdown_op = x, .desc = #x
 static struct tcase {
 	int shutdown_op;
 	int recv_flag;
 	int recv_err;
 	int send_flag;
 	int send_err;
-	char *flag_str;
+	char *desc;
 } tcases[] = {
-	{SHUT_RD, 0, 0, 0, 0, "SHUT_RD"},
-	{SHUT_WR, MSG_DONTWAIT, EWOULDBLOCK, MSG_NOSIGNAL, EPIPE, "SHUT_WR"},
-	{SHUT_RDWR, 0, 0, MSG_NOSIGNAL, EPIPE, "SHUT_RDWR"}
+	{OP_DESC(SHUT_RD)},
+	{OP_DESC(SHUT_WR), .recv_flag = MSG_DONTWAIT, .recv_err = EWOULDBLOCK,
+		.send_flag = MSG_NOSIGNAL, .send_err = EPIPE},
+	{OP_DESC(SHUT_RDWR), .send_flag = MSG_NOSIGNAL, .send_err = EPIPE}
 };
 
 static struct sockaddr_un *sock_addr;
@@ -82,23 +84,22 @@ static void run(unsigned int n)
 
 	client_sock = start_test();
 
-	tst_res(TINFO, "Testing %s flag", tc->flag_str);
+	tst_res(TINFO, "Testing %s flag", tc->desc);
 
 	TST_EXP_PASS(shutdown(client_sock, tc->shutdown_op));
 
-	if (!tc->recv_err)
-		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
-	else
+	if (tc->recv_err)
 		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
-
-	if (!tc->send_err)
-		SAFE_SEND(MSGSIZE, client_sock, buff, MSGSIZE, tc->send_flag);
 	else
+		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
+
+	if (tc->send_err)
 		TST_EXP_FAIL(send(client_sock, buff, MSGSIZE, tc->send_flag), tc->send_err);
+	else
+		SAFE_SEND(MSGSIZE, client_sock, buff, MSGSIZE, tc->send_flag);
 
 	SAFE_CLOSE(client_sock);
 	TST_CHECKPOINT_WAKE(0);
-
 }
 
 static void setup(void)
Andrea Cervesato June 10, 2024, 1:17 p.m. UTC | #2 
Hi Petr,

I agree with the proposed changes. Thanks, feel free to merge.

Acked-by: Andrea Cervesato <andrea.cervesato@suse.com>

On 6/10/24 15:13, Petr Vorel wrote:
> Hi Andrea,
>
> TL;DR: generally LGTM, I have few minor details, which I offer to fix before
> merge (diff below).
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
>> +++ b/testcases/kernel/syscalls/shutdown/shutdown01.c
> ...
>> +static struct tcase {
>> +	int shutdown_op;
>> +	int recv_flag;
>> +	int recv_err;
>> +	int send_flag;
>> +	int send_err;
>> +	char *flag_str;
>> +} tcases[] = {
>> +	{SHUT_RD, 0, 0, 0, 0, "SHUT_RD"},
>> +	{SHUT_WR, MSG_DONTWAIT, EWOULDBLOCK, MSG_NOSIGNAL, EPIPE, "SHUT_WR"},
>> +	{SHUT_RDWR, 0, 0, MSG_NOSIGNAL, EPIPE, "SHUT_RDWR"}
> Would you mind to use
> 1) designated initializers (e.g. .recv_flag = MSG_DONTWAIT,)
> 2) stringify macro
>
> IMHO it helps readability, that's why it's often used.
>
> #define OP_DESC(x) .shutdown_op = x, .desc = #x
> static struct tcase {
> 	int shutdown_op;
> 	int recv_flag;
> 	int recv_err;
> 	int send_flag;
> 	int send_err;
> 	char *desc;
> } tcases[] = {
> 	{OP_DESC(SHUT_RD)},
> 	{OP_DESC(SHUT_WR), .recv_flag = MSG_DONTWAIT, .recv_err = EWOULDBLOCK,
> 		.send_flag = MSG_NOSIGNAL, .send_err = EPIPE},
> 	{OP_DESC(SHUT_RDWR), .send_flag = MSG_NOSIGNAL, .send_err = EPIPE}
> };
>
>> +};
>> +
>> +static struct sockaddr_un *sock_addr;
>> +
>> +static void run_server(void)
>> +{
>> +	int server_sock;
>> +
>> +	server_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
>> +
>> +	SAFE_BIND(server_sock,
>> +		(struct sockaddr *)sock_addr,
>> +		sizeof(struct sockaddr_un));
>> +	SAFE_LISTEN(server_sock, 10);
>> +
>> +	tst_res(TINFO, "Running server on socket file");
>> +
>> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
>> +
>> +	SAFE_CLOSE(server_sock);
>> +	SAFE_UNLINK(SOCKETFILE);
>> +}
>> +
>> +static int start_test(void)
>> +{
>> +	int client_sock;
>> +
>> +	if (!SAFE_FORK()) {
>> +		run_server();
>> +		_exit(0);
>> +	}
>> +
>> +	TST_CHECKPOINT_WAIT(0);
>> +
>> +	tst_res(TINFO, "Connecting to the server");
>> +
>> +	client_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
>> +	SAFE_CONNECT(client_sock,
>> +		(struct sockaddr *)sock_addr,
>> +		sizeof(struct sockaddr_un));
>> +
>> +	return client_sock;
>> +}
>> +
>> +static void run(unsigned int n)
>> +{
>> +	struct tcase *tc = &tcases[n];
>> +	int client_sock;
>> +	char buff[MSGSIZE] = {0};
>> +
>> +	client_sock = start_test();
>> +
>> +	tst_res(TINFO, "Testing %s flag", tc->flag_str);
>> +
>> +	TST_EXP_PASS(shutdown(client_sock, tc->shutdown_op));
>> +
>> +	if (!tc->recv_err)
>> +		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
>> +	else
>> +		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
> very nit:
>
> 	if (tc->recv_err)
> 		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
> 	else
> 		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
>
> If you agree, I can merge with following changes.
>
> Kind regards,
> Petr
>
> diff --git testcases/kernel/syscalls/shutdown/shutdown01.c testcases/kernel/syscalls/shutdown/shutdown01.c
> index ba3853d9c..8e58f23e6 100644
> --- testcases/kernel/syscalls/shutdown/shutdown01.c
> +++ testcases/kernel/syscalls/shutdown/shutdown01.c
> @@ -19,17 +19,19 @@
>   #define MSGSIZE 4
>   #define SOCKETFILE "socket"
>   
> +#define OP_DESC(x) .shutdown_op = x, .desc = #x
>   static struct tcase {
>   	int shutdown_op;
>   	int recv_flag;
>   	int recv_err;
>   	int send_flag;
>   	int send_err;
> -	char *flag_str;
> +	char *desc;
>   } tcases[] = {
> -	{SHUT_RD, 0, 0, 0, 0, "SHUT_RD"},
> -	{SHUT_WR, MSG_DONTWAIT, EWOULDBLOCK, MSG_NOSIGNAL, EPIPE, "SHUT_WR"},
> -	{SHUT_RDWR, 0, 0, MSG_NOSIGNAL, EPIPE, "SHUT_RDWR"}
> +	{OP_DESC(SHUT_RD)},
> +	{OP_DESC(SHUT_WR), .recv_flag = MSG_DONTWAIT, .recv_err = EWOULDBLOCK,
> +		.send_flag = MSG_NOSIGNAL, .send_err = EPIPE},
> +	{OP_DESC(SHUT_RDWR), .send_flag = MSG_NOSIGNAL, .send_err = EPIPE}
>   };
>   
>   static struct sockaddr_un *sock_addr;
> @@ -82,23 +84,22 @@ static void run(unsigned int n)
>   
>   	client_sock = start_test();
>   
> -	tst_res(TINFO, "Testing %s flag", tc->flag_str);
> +	tst_res(TINFO, "Testing %s flag", tc->desc);
>   
>   	TST_EXP_PASS(shutdown(client_sock, tc->shutdown_op));
>   
> -	if (!tc->recv_err)
> -		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
> -	else
> +	if (tc->recv_err)
>   		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
> -
> -	if (!tc->send_err)
> -		SAFE_SEND(MSGSIZE, client_sock, buff, MSGSIZE, tc->send_flag);
>   	else
> +		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
> +
> +	if (tc->send_err)
>   		TST_EXP_FAIL(send(client_sock, buff, MSGSIZE, tc->send_flag), tc->send_err);
> +	else
> +		SAFE_SEND(MSGSIZE, client_sock, buff, MSGSIZE, tc->send_flag);
>   
>   	SAFE_CLOSE(client_sock);
>   	TST_CHECKPOINT_WAKE(0);
> -
>   }
>   
>   static void setup(void)

Andrea
Petr Vorel June 10, 2024, 1:52 p.m. UTC | #3 
Hi Andrea,

this patch merged, thank you!

Kind regards,
Petr
diff mbox series

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index cf06ee563..dc396415e 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1465,6 +1465,8 @@  shmget04 shmget04
 shmget05 shmget05
 shmget06 shmget06
 
+shutdown01 shutdown01
+
 sigaction01 sigaction01
 sigaction02 sigaction02
 
diff --git a/testcases/kernel/syscalls/shutdown/.gitignore b/testcases/kernel/syscalls/shutdown/.gitignore
new file mode 100644
index 000000000..2df24d1ab
--- /dev/null
+++ b/testcases/kernel/syscalls/shutdown/.gitignore
@@ -0,0 +1 @@ 
+shutdown01
diff --git a/testcases/kernel/syscalls/shutdown/Makefile b/testcases/kernel/syscalls/shutdown/Makefile
new file mode 100644
index 000000000..8cf1b9024
--- /dev/null
+++ b/testcases/kernel/syscalls/shutdown/Makefile
@@ -0,0 +1,7 @@ 
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/shutdown/shutdown01.c b/testcases/kernel/syscalls/shutdown/shutdown01.c
new file mode 100644
index 000000000..ba3853d9c
--- /dev/null
+++ b/testcases/kernel/syscalls/shutdown/shutdown01.c
@@ -0,0 +1,121 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies the following shutdown() functionalities:
+ *
+ * - SHUT_RD should enable send() ops but disable recv() ops
+ * - SHUT_WR should enable recv() ops but disable send() ops
+ * - SHUT_RDWR should disable both recv() and send() ops
+ */
+
+#include "tst_test.h"
+#include "tst_safe_net.h"
+
+#define MSGSIZE 4
+#define SOCKETFILE "socket"
+
+static struct tcase {
+	int shutdown_op;
+	int recv_flag;
+	int recv_err;
+	int send_flag;
+	int send_err;
+	char *flag_str;
+} tcases[] = {
+	{SHUT_RD, 0, 0, 0, 0, "SHUT_RD"},
+	{SHUT_WR, MSG_DONTWAIT, EWOULDBLOCK, MSG_NOSIGNAL, EPIPE, "SHUT_WR"},
+	{SHUT_RDWR, 0, 0, MSG_NOSIGNAL, EPIPE, "SHUT_RDWR"}
+};
+
+static struct sockaddr_un *sock_addr;
+
+static void run_server(void)
+{
+	int server_sock;
+
+	server_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
+
+	SAFE_BIND(server_sock,
+		(struct sockaddr *)sock_addr,
+		sizeof(struct sockaddr_un));
+	SAFE_LISTEN(server_sock, 10);
+
+	tst_res(TINFO, "Running server on socket file");
+
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
+
+	SAFE_CLOSE(server_sock);
+	SAFE_UNLINK(SOCKETFILE);
+}
+
+static int start_test(void)
+{
+	int client_sock;
+
+	if (!SAFE_FORK()) {
+		run_server();
+		_exit(0);
+	}
+
+	TST_CHECKPOINT_WAIT(0);
+
+	tst_res(TINFO, "Connecting to the server");
+
+	client_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
+	SAFE_CONNECT(client_sock,
+		(struct sockaddr *)sock_addr,
+		sizeof(struct sockaddr_un));
+
+	return client_sock;
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int client_sock;
+	char buff[MSGSIZE] = {0};
+
+	client_sock = start_test();
+
+	tst_res(TINFO, "Testing %s flag", tc->flag_str);
+
+	TST_EXP_PASS(shutdown(client_sock, tc->shutdown_op));
+
+	if (!tc->recv_err)
+		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
+	else
+		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
+
+	if (!tc->send_err)
+		SAFE_SEND(MSGSIZE, client_sock, buff, MSGSIZE, tc->send_flag);
+	else
+		TST_EXP_FAIL(send(client_sock, buff, MSGSIZE, tc->send_flag), tc->send_err);
+
+	SAFE_CLOSE(client_sock);
+	TST_CHECKPOINT_WAKE(0);
+
+}
+
+static void setup(void)
+{
+	sock_addr->sun_family = AF_UNIX;
+	memcpy(sock_addr->sun_path, SOCKETFILE, sizeof(SOCKETFILE));
+}
+
+static struct tst_test test = {
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.needs_tmpdir = 1,
+	.bufs = (struct tst_buffers []) {
+		{&sock_addr, .size = sizeof(struct sockaddr_un)},
+		{}
+	}
+};