All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API
@ 2022-03-10 10:59 Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c " Andrea Cervesato
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Rewritten all sysvipc tests and removed libclone dependency.
Removed TST_CHECKPOINT_INIT from all tests and replaced it with the
.checkpoints support.

Andrea Cervesato (10):
  Rewrite mesgq_nstest.c using new LTP API
  Rewrite msg_comm.c using new LTP API
  Rewrite sem_comm.c using new LTP API
  Rewrite sem_nstest.c using new LTP API
  Rewrite semtest_2ns.c using new LTP API
  Rewrite shm_comm.c using new LTP API
  Rewrite shmem_2nstest.c using new LTP API
  Rewrite shmnstest.c using new LTP API
  Remove libclone dependency from sysvipc
  Delete obsolete ipcns_helper.h in sysvipc suite

 testcases/kernel/containers/sysvipc/Makefile  |  26 +-
 testcases/kernel/containers/sysvipc/common.h  | 157 ++++++++++
 .../kernel/containers/sysvipc/ipcns_helper.h  |  41 ---
 .../kernel/containers/sysvipc/mesgq_nstest.c  | 247 ++++++---------
 .../kernel/containers/sysvipc/msg_comm.c      | 180 ++++-------
 .../kernel/containers/sysvipc/sem_comm.c      | 180 ++++-------
 .../kernel/containers/sysvipc/sem_nstest.c    | 212 +++++--------
 .../kernel/containers/sysvipc/semtest_2ns.c   | 295 +++++++-----------
 .../kernel/containers/sysvipc/shm_comm.c      | 167 +++-------
 .../kernel/containers/sysvipc/shmem_2nstest.c | 239 +++++---------
 .../kernel/containers/sysvipc/shmnstest.c     | 182 ++++-------
 11 files changed, 746 insertions(+), 1180 deletions(-)
 create mode 100644 testcases/kernel/containers/sysvipc/common.h
 delete mode 100644 testcases/kernel/containers/sysvipc/ipcns_helper.h

-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 14:16   ` Cyril Hrubis
  2022-03-10 10:59 ` [LTP] [PATCH v2 02/10] Rewrite msg_comm.c " Andrea Cervesato
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/sysvipc/common.h  | 157 +++++++++++
 .../kernel/containers/sysvipc/mesgq_nstest.c  | 247 +++++++-----------
 2 files changed, 254 insertions(+), 150 deletions(-)
 create mode 100644 testcases/kernel/containers/sysvipc/common.h

diff --git a/testcases/kernel/containers/sysvipc/common.h b/testcases/kernel/containers/sysvipc/common.h
new file mode 100644
index 000000000..1fea6fafa
--- /dev/null
+++ b/testcases/kernel/containers/sysvipc/common.h
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) International Business Machines Corp., 2007
+ *               Rishikesh K Rajak <risrajak@in.ibm.com>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+#ifndef COMMON_H
+#define COMMON_H
+
+#include <stdlib.h>
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+#include "lapi/namespaces_constants.h"
+
+enum {
+	T_CLONE,
+	T_UNSHARE,
+	T_NONE,
+};
+
+static int dummy_child(void *v)
+{
+	(void)v;
+	return 0;
+}
+
+static void check_newipc(void)
+{
+	int pid, status;
+
+	if (tst_kvercmp(2, 6, 19) < 0)
+		tst_brk(TCONF, "CLONE_NEWIPC not supported");
+
+	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child, NULL);
+	if (pid < 0)
+		tst_brk(TCONF | TERRNO, "CLONE_NEWIPC not supported");
+
+	SAFE_WAITPID(pid, &status, 0);
+}
+
+static inline int get_clone_unshare_enum(const char* str_op)
+{
+	int use_clone;
+
+	if (strcmp(str_op, "clone") &&
+		strcmp(str_op, "unshare") &&
+		strcmp(str_op, "none"))
+		tst_brk(TBROK, "Test execution mode <clone|unshare|none>");
+
+	if (!strcmp(str_op, "clone"))
+		use_clone = T_CLONE;
+	else if (!strcmp(str_op, "unshare"))
+		use_clone = T_UNSHARE;
+	else
+		use_clone = T_NONE;
+
+	return use_clone;
+}
+
+static int clone_test(unsigned long clone_flags, int (*fn1)(void *arg),
+		      void *arg1)
+{
+	int ret;
+
+	ret = ltp_clone_quick(clone_flags | SIGCHLD, fn1, arg1);
+
+	if (ret != -1) {
+		/* no errors: we'll get the PID id that means success */
+		ret = 0;
+	}
+
+	return ret;
+}
+
+static int unshare_test(unsigned long clone_flags, int (*fn1)(void *arg),
+			void *arg1)
+{
+	int pid, ret = 0;
+	int retpipe[2];
+	char buf[2];
+
+	SAFE_PIPE(retpipe);
+
+	pid = fork();
+	if (pid < 0) {
+		SAFE_CLOSE(retpipe[0]);
+		SAFE_CLOSE(retpipe[1]);
+		tst_brk(TBROK, "fork");
+	}
+
+	if (!pid) {
+		SAFE_CLOSE(retpipe[0]);
+
+		ret = tst_syscall(SYS_unshare, clone_flags);
+		if (ret == -1) {
+			SAFE_WRITE(1, retpipe[1], "0", 2);
+			SAFE_CLOSE(retpipe[1]);
+			exit(1);
+		} else {
+			SAFE_WRITE(1, retpipe[1], "1", 2);
+		}
+
+		SAFE_CLOSE(retpipe[1]);
+
+		ret = fn1(arg1);
+		exit(ret);
+	}
+
+	SAFE_CLOSE(retpipe[1]);
+	SAFE_READ(1, retpipe[0], &buf, 2);
+	SAFE_CLOSE(retpipe[0]);
+
+	if (*buf == '0')
+		return -1;
+
+	return ret;
+}
+
+static int plain_test(int (*fn1)(void *arg), void *arg1)
+{
+	int ret = 0, pid;
+
+	pid = SAFE_FORK();
+	if (!pid)
+		exit(fn1(arg1));
+
+	return ret;
+}
+
+static void clone_unshare_test(int use_clone, unsigned long clone_flags,
+			       int (*fn1)(void *arg), void *arg1)
+{
+	int ret = 0;
+
+	switch (use_clone) {
+	case T_NONE:
+		ret = plain_test(fn1, arg1);
+		break;
+	case T_CLONE:
+		ret = clone_test(clone_flags, fn1, arg1);
+		break;
+	case T_UNSHARE:
+		ret = unshare_test(clone_flags, fn1, arg1);
+		break;
+	default:
+		ret = -1;
+		tst_brk(TBROK, "%s: bad use_clone option: %d", __FUNCTION__,
+			use_clone);
+		break;
+	}
+
+	if (ret == -1)
+		tst_brk(TBROK, "child2 clone failed");
+}
+
+#endif
diff --git a/testcases/kernel/containers/sysvipc/mesgq_nstest.c b/testcases/kernel/containers/sysvipc/mesgq_nstest.c
index dbf311dc8..bb58b7211 100644
--- a/testcases/kernel/containers/sysvipc/mesgq_nstest.c
+++ b/testcases/kernel/containers/sysvipc/mesgq_nstest.c
@@ -1,175 +1,122 @@
-/* *************************************************************************
-* Copyright (c) International Business Machines Corp., 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Veerendra C <vechandr@in.ibm.com>
-*
-* In Parent Process , create mesgq with key 154326L
-* Now create container by passing 1 of the flag values..
-*	Flag = clone, clone(CLONE_NEWIPC), or unshare(CLONE_NEWIPC)
-* In cloned process, try to access the created mesgq
-* Test PASS: If the mesgq is readable when flag is None.
-* Test FAIL: If the mesgq is readable when flag is Unshare or Clone.
-***************************************************************************/
-
-#define _GNU_SOURCE 1
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <sys/ipc.h>
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) International Business Machines Corp., 2009
+ *				Veerendra C <vechandr@in.ibm.com>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test SysV IPC message passing through different namespaces.
+ *
+ * [Algorithm]
+ *
+ * In parent process create a new mesgq with a specific key.
+ * In cloned process try to access the created mesgq.
+ *
+ * Test will PASS if the mesgq is readable when flag is None.
+ * Test will FAIL if the mesgq is readable when flag is Unshare or Clone or
+ * the message received is wrong.
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/wait.h>
 #include <sys/msg.h>
-#include <libclone.h>
-#include "test.h"
-#include "ipcns_helper.h"
-
-#define KEY_VAL		154326L
-#define UNSHARESTR	"unshare"
-#define CLONESTR	"clone"
-#define NONESTR		"none"
-
-char *TCID = "mesgq_nstest";
-int TST_TOTAL = 1;
-int p1[2];
-int p2[2];
-struct msg_buf {
-	long int mtype;		/* type of received/sent message */
-	char mtext[80];		/* text of the message */
+#include <sys/types.h>
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
+#include "common.h"
+
+#define KEY_VAL 154326L
+#define MSG_TYPE 5
+#define MSG_TEXT "My message!"
+
+static char *str_op = "clone";
+static int use_clone;
+static int ipc_id = -1;
+
+static struct msg_buf {
+	long mtype;
+	char mtext[80];
 } msg;
 
-void mesgq_read(int id)
+static int check_mesgq(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	int READMAX = 80;
-	int n;
-	/* read msg type 5 on the Q; msgtype, flags are last 2 params.. */
+	int id, n;
 
-	n = msgrcv(id, &msg, READMAX, 5, 0);
-	if (n == -1)
-		perror("msgrcv"), tst_exit();
-
-	tst_resm(TINFO, "Mesg read of %d bytes; Type %ld: Msg: %.*s",
-		 n, msg.mtype, n, msg.mtext);
-}
+	id = msgget(KEY_VAL, 0);
 
-int check_mesgq(void *vtest)
-{
-	char buf[3];
-	int id;
+	if (id < 0) {
+		if (use_clone == T_NONE)
+			tst_res(TFAIL, "Plain cloned process didn't find mesgq");
+		else
+			tst_res(TPASS, "%s: container didn't find mesgq", str_op);
+	} else {
+		if (use_clone == T_NONE)
+			tst_res(TPASS, "Plain cloned process found mesgq inside container");
+		else
+			tst_res(TFAIL, "%s: container init process found mesgq", str_op);
 
-	(void) vtest;
+		n = SAFE_MSGRCV(id, &msg, sizeof(msg.mtext), MSG_TYPE, 0);
 
-	close(p1[1]);
-	close(p2[0]);
+		tst_res(TINFO, "Mesg read of %d bytes, Type %ld, Msg: %s", n, msg.mtype, msg.mtext);
 
-	read(p1[0], buf, 3);
-	id = msgget(KEY_VAL, 0);
-	if (id == -1)
-		write(p2[1], "notfnd", 7);
-	else {
-		write(p2[1], "exists", 7);
-		mesgq_read(id);
+		if (strcmp(msg.mtext, MSG_TEXT))
+			tst_res(TFAIL, "Received the wrong text message");
 	}
-	tst_exit();
-}
 
-static void setup(void)
-{
-	tst_require_root();
-	check_newipc();
+	TST_CHECKPOINT_WAKE(0);
+
+	return 0;
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int ret, use_clone = T_NONE, id, n;
-	char *tsttype = NONESTR;
-	char buf[7];
+	msg.mtype = MSG_TYPE;
+	strcpy(msg.mtext, "My message!");
 
-	setup();
+	SAFE_MSGSND(ipc_id, &msg, strlen(msg.mtext), 0);
 
-	if (argc != 2) {
-		tst_resm(TFAIL, "Usage: %s <clone|unshare|none>", argv[0]);
-		tst_brkm(TFAIL, NULL, " where clone, unshare, or fork specifies"
-			 " unshare method.");
-	}
+	tst_res(TINFO, "mesgq namespaces test: %s", str_op);
 
-	/* Using PIPE's to sync between container and Parent */
-	if (pipe(p1) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
-	if (pipe(p2) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_mesgq, NULL);
 
-	tsttype = NONESTR;
+	TST_CHECKPOINT_WAIT(0);
+}
 
-	if (strcmp(argv[1], "clone") == 0) {
-		use_clone = T_CLONE;
-		tsttype = CLONESTR;
-	} else if (strcmp(argv[1], "unshare") == 0) {
-		use_clone = T_UNSHARE;
-		tsttype = UNSHARESTR;
-	}
+static void setup(void)
+{
+	use_clone = get_clone_unshare_enum(str_op);
 
-	id = msgget(KEY_VAL, IPC_CREAT | IPC_EXCL | 0600);
-	if (id == -1) {
-		perror("msgget");
-		/* Retry without attempting to create the MQ */
-		id = msgget(KEY_VAL, 0);
-		if (id == -1)
-			perror("msgget failure"), exit(1);
-	}
+	if (use_clone != T_NONE)
+		check_newipc();
 
-	msg.mtype = 5;
-	strcpy(msg.mtext, "Message of type 5!");
-	n = msgsnd(id, &msg, strlen(msg.mtext), 0);
-	if (n == -1)
-		perror("msgsnd"), tst_exit();
-
-	tst_resm(TINFO, "mesgq namespaces test : %s", tsttype);
-	/* fire off the test */
-	ret = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mesgq, NULL);
-	if (ret < 0) {
-		tst_brkm(TFAIL, NULL, "%s failed", tsttype);
+	ipc_id = msgget(KEY_VAL, IPC_CREAT | IPC_EXCL | 0600);
+	if (ipc_id < 0) {
+		tst_res(TINFO, "Message already exists");
+		ipc_id = SAFE_MSGGET(KEY_VAL, 0);
 	}
+}
 
-	close(p1[0]);
-	close(p2[1]);
-	write(p1[1], "go", 3);
-
-	read(p2[0], buf, 7);
-	if (strcmp(buf, "exists") == 0) {
-		if (use_clone == T_NONE)
-			tst_resm(TPASS, "Plain cloned process found mesgq "
-				 "inside container");
-		else
-			tst_resm(TFAIL,
-				 "%s: Container init process found mesgq",
-				 tsttype);
-	} else {
-		if (use_clone == T_NONE)
-			tst_resm(TFAIL,
-				 "Plain cloned process didn't find mesgq");
-		else
-			tst_resm(TPASS, "%s: Container didn't find mesgq",
-				 tsttype);
+static void cleanup(void)
+{
+	if (ipc_id != -1) {
+		tst_res(TINFO, "Destroy message");
+		SAFE_MSGCTL(ipc_id, IPC_RMID, NULL);
 	}
-
-	/* Delete the mesgQ */
-	id = msgget(KEY_VAL, 0);
-	msgctl(id, IPC_RMID, NULL);
-
-	tst_exit();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Test execution mode <clone|unshare|none>" },
+		{},
+	},
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 02/10] Rewrite msg_comm.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 15:19   ` Cyril Hrubis
  2022-03-10 10:59 ` [LTP] [PATCH v2 03/10] Rewrite sem_comm.c " Andrea Cervesato
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/msg_comm.c      | 180 ++++++------------
 1 file changed, 55 insertions(+), 125 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/msg_comm.c b/testcases/kernel/containers/sysvipc/msg_comm.c
index 0da328997..0dce423f0 100644
--- a/testcases/kernel/containers/sysvipc/msg_comm.c
+++ b/testcases/kernel/containers/sysvipc/msg_comm.c
@@ -1,20 +1,16 @@
-/* Copyright (c) 2014 Red Hat, Inc.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of version 2 the GNU General Public License as
- * published by the Free Software Foundation.
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
  *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
+ * Test SysV IPC message passing through different processes.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- ***********************************************************************
- * File: msg_comm.c
+ * [Algorithm]
  *
- * Description:
  * 1. Clones two child processes with CLONE_NEWIPC flag, each child
  *    gets System V message queue (msg) with the _identical_ key.
  * 2. Child1 appends a message with identifier #1 to the message queue.
@@ -27,152 +23,86 @@
  */
 
 #define _GNU_SOURCE
-#include <sys/ipc.h>
+
+#include <sys/wait.h>
 #include <sys/msg.h>
 #include <sys/types.h>
-#include <sys/wait.h>
-#include <stdio.h>
-#include <errno.h>
-#include "ipcns_helper.h"
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
+#include "common.h"
 
 #define TESTKEY 124426L
-#define MSGSIZE 50
-char *TCID	= "msg_comm";
-int TST_TOTAL	= 1;
 
 struct sysv_msg {
 	long mtype;
-	char mtext[MSGSIZE];
+	char mtext[1];
 };
 
-static void cleanup(void)
+static int chld1_msg(LTP_ATTRIBUTE_UNUSED void *arg)
 {
-	tst_rmdir();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-	check_newipc();
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(tst_rmdir);
-}
-
-int chld1_msg(void *arg)
-{
-	int id, n, rval = 0;
+	int id;
 	struct sysv_msg m;
 	struct sysv_msg rec;
 
-	id = msgget(TESTKEY, IPC_CREAT | 0600);
-	if (id == -1) {
-		perror("msgget");
-		return 2;
-	}
+	id = SAFE_MSGGET(TESTKEY, IPC_CREAT | 0600);
 
 	m.mtype = 1;
 	m.mtext[0] = 'A';
-	if (msgsnd(id, &m, sizeof(struct sysv_msg) - sizeof(long), 0) == -1) {
-		perror("msgsnd");
-		msgctl(id, IPC_RMID, NULL);
-		return 2;
-	}
 
-	/* wait for child2 to write into the message queue */
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	SAFE_MSGSND(id, &m, sizeof(struct sysv_msg) - sizeof(long), 0);
+
+	TST_CHECKPOINT_WAIT(0);
+
+	TEST(msgrcv(id, &rec, sizeof(struct sysv_msg) - sizeof(long), 2, IPC_NOWAIT));
+	if (TST_RET < 0 && TST_ERR != ENOMSG)
+		tst_brk(TBROK | TERRNO, "msgrcv error");
 
 	/* if child1 message queue has changed (by child2) report fail */
-	n = msgrcv(id, &rec, sizeof(struct sysv_msg) - sizeof(long),
-		   2, IPC_NOWAIT);
-	if (n == -1 && errno != ENOMSG) {
-		perror("msgrcv");
-		msgctl(id, IPC_RMID, NULL);
-		return 2;
-	}
-	/* if mtype #2 was found in the message queue, it is fail */
-	if (n > 0) {
-		rval = 1;
-	}
-
-	/* tell child2 to continue */
-	TST_SAFE_CHECKPOINT_WAKE(NULL, 0);
-
-	msgctl(id, IPC_RMID, NULL);
-	return rval;
+	if (TST_RET > 0)
+		tst_res(TFAIL, "SysV msg: communication with non-identical keys between namespaces");
+	else
+		tst_res(TPASS, "SysV msg: communication with identical keys between namespaces");
+
+	TST_CHECKPOINT_WAKE(0);
+
+	SAFE_MSGCTL(id, IPC_RMID, NULL);
+
+	return 0;
 }
 
-int chld2_msg(void *arg)
+static int chld2_msg(LTP_ATTRIBUTE_UNUSED void *arg)
 {
 	int id;
 	struct sysv_msg m;
 
-	id = msgget(TESTKEY, IPC_CREAT | 0600);
-	if (id == -1) {
-		perror("msgget");
-		return 2;
-	}
+	id = SAFE_MSGGET(TESTKEY, IPC_CREAT | 0600);
 
 	m.mtype = 2;
 	m.mtext[0] = 'B';
-	if (msgsnd(id, &m, sizeof(struct sysv_msg) - sizeof(long), 0) == -1) {
-		perror("msgsnd");
-		msgctl(id, IPC_RMID, NULL);
-		return 2;
-	}
 
-	/* tell child1 to continue and wait for it */
-	TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 0);
+	SAFE_MSGSND(id, &m, sizeof(struct sysv_msg) - sizeof(long), 0);
+
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
+
+	SAFE_MSGCTL(id, IPC_RMID, NULL);
 
-	msgctl(id, IPC_RMID, NULL);
 	return 0;
 }
 
-static void test(void)
+static void run(void)
 {
-	int status, ret = 0;
-
-	ret = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld1_msg, NULL);
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
-
-	ret = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld2_msg, NULL);
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
-
-
-	while (wait(&status) > 0) {
-		if (WIFEXITED(status) && WEXITSTATUS(status) == 1)
-			ret = 1;
-		if (WIFEXITED(status) && WEXITSTATUS(status) == 2)
-			tst_brkm(TBROK | TERRNO, cleanup, "error in child");
-		if (WIFSIGNALED(status)) {
-			tst_resm(TFAIL, "child was killed with signal %s",
-					tst_strsig(WTERMSIG(status)));
-			return;
-		}
-	}
-
-	if (ret)
-		tst_resm(TFAIL, "SysV msg: communication with identical keys"
-				" between namespaces");
-	else
-		tst_resm(TPASS, "SysV msg: communication with identical keys"
-				" between namespaces");
+	clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld1_msg, NULL);
+	clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld2_msg, NULL);
 }
 
-int main(int argc, char *argv[])
+static void setup(void)
 {
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++)
-		test();
-
-	cleanup();
-	tst_exit();
+	check_newipc();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 03/10] Rewrite sem_comm.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c " Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 02/10] Rewrite msg_comm.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 15:43   ` Cyril Hrubis
  2022-03-10 10:59 ` [LTP] [PATCH v2 04/10] Rewrite sem_nstest.c " Andrea Cervesato
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/sem_comm.c      | 180 ++++++------------
 1 file changed, 56 insertions(+), 124 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/sem_comm.c b/testcases/kernel/containers/sysvipc/sem_comm.c
index a2c354a08..a57a1fd1a 100644
--- a/testcases/kernel/containers/sysvipc/sem_comm.c
+++ b/testcases/kernel/containers/sysvipc/sem_comm.c
@@ -1,20 +1,16 @@
-/* Copyright (c) 2014 Red Hat, Inc.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of version 2 the GNU General Public License as
- * published by the Free Software Foundation.
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
  *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
+ * Test SysV IPC semaphore usage between cloned processes.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- ***********************************************************************
- * File: sem_comm.c
+ * [Algorithm]
  *
- * Description:
  * 1. Clones two child processes with CLONE_NEWIPC flag, each child
  *    creates System V semaphore (sem) with the _identical_ key.
  * 2. Child1 locks the semaphore.
@@ -26,166 +22,102 @@
  */
 
 #define _GNU_SOURCE
-#include <sys/ipc.h>
-#include <sys/types.h>
+
 #include <sys/wait.h>
-#include <stdio.h>
-#include <errno.h>
-#include "ipcns_helper.h"
-#include "test.h"
-#include "safe_macros.h"
+#include <sys/msg.h>
+#include <sys/types.h>
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
 #include "lapi/sem.h"
+#include "common.h"
 
 #define TESTKEY 124426L
-char *TCID	= "sem_comm";
-int TST_TOTAL	= 1;
-
-static void cleanup(void)
-{
-	tst_rmdir();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-	check_newipc();
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(tst_rmdir);
-}
 
-int chld1_sem(void *arg)
+static int chld1_sem(LTP_ATTRIBUTE_UNUSED void *arg)
 {
 	int id;
 	union semun su;
 	struct sembuf sm;
 
-	id = semget(TESTKEY, 1, IPC_CREAT);
-	if (id == -1) {
-		perror("semget");
-		return 2;
-	}
+	id = SAFE_SEMGET(TESTKEY, 1, IPC_CREAT);
 
 	su.val = 1;
-	if (semctl(id, 0, SETVAL, su) == -1) {
-		perror("semctl");
-		semctl(id, 0, IPC_RMID);
-		return 2;
-	}
+	SAFE_SEMCTL(id, 0, SETVAL, su);
 
 	/* tell child2 to continue and wait for it to create the semaphore */
-	TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
 
 	sm.sem_num = 0;
 	sm.sem_op = -1;
 	sm.sem_flg = IPC_NOWAIT;
-	if (semop(id, &sm, 1) == -1) {
-		perror("semop");
-		semctl(id, 0, IPC_RMID);
-		return 2;
-	}
+	SAFE_SEMOP(id, &sm, 1);
 
 	/* tell child2 to continue and wait for it to lock the semaphore */
-	TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
 
 	sm.sem_op = 1;
-	semop(id, &sm, 1);
+	SAFE_SEMOP(id, &sm, 1);
+
+	SAFE_SEMCTL(id, 0, IPC_RMID);
 
-	semctl(id, 0, IPC_RMID);
 	return 0;
 }
 
-int chld2_sem(void *arg)
+static int chld2_sem(LTP_ATTRIBUTE_UNUSED void *arg)
 {
-	int id, rval = 0;
+	int id;
 	struct sembuf sm;
 	union semun su;
 
 	/* wait for child1 to create the semaphore */
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
 
-	id = semget(TESTKEY, 1, IPC_CREAT);
-	if (id == -1) {
-		perror("semget");
-		return 2;
-	}
+	id = SAFE_SEMGET(TESTKEY, 1, IPC_CREAT);
 
 	su.val = 1;
-	if (semctl(id, 0, SETVAL, su) == -1) {
-		perror("semctl");
-		semctl(id, 0, IPC_RMID);
-		return 2;
-	}
+	SAFE_SEMCTL(id, 0, SETVAL, su);
 
 	/* tell child1 to continue and wait for it to lock the semaphore */
-	TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
 
 	sm.sem_num = 0;
 	sm.sem_op = -1;
 	sm.sem_flg = IPC_NOWAIT;
-	if (semop(id, &sm, 1) == -1) {
-		if (errno == EAGAIN) {
-			rval = 1;
-		} else {
-			perror("semop");
-			semctl(id, 0, IPC_RMID);
-			return 2;
-		}
+	TEST(semop(id, &sm, 1));
+	if (TST_RET < 0) {
+		if (TST_ERR != EAGAIN)
+			tst_brk(TBROK | TERRNO, "semop error");
+
+		tst_res(TFAIL, "SysV sem: communication with identical keys between namespaces");
+	} else {
+		tst_res(TPASS, "SysV sem: communication with identical keys between namespaces");
 	}
 
 	/* tell child1 to continue */
-	TST_SAFE_CHECKPOINT_WAKE(NULL, 0);
+	TST_CHECKPOINT_WAKE(0);
 
 	sm.sem_op = 1;
-	semop(id, &sm, 1);
+	SAFE_SEMOP(id, &sm, 1);
+
+	SAFE_SEMCTL(id, 0, IPC_RMID);
 
-	semctl(id, 0, IPC_RMID);
-	return rval;
+	return 0;
 }
 
-static void test(void)
+static void run(void)
 {
-	int status, ret = 0;
-
-	ret = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld1_sem, NULL);
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
-
-	ret = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld2_sem, NULL);
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
-
-
-	while (wait(&status) > 0) {
-		if (WIFEXITED(status) && WEXITSTATUS(status) == 1)
-			ret = 1;
-		if (WIFEXITED(status) && WEXITSTATUS(status) == 2)
-			tst_brkm(TBROK | TERRNO, cleanup, "error in child");
-		if (WIFSIGNALED(status)) {
-			tst_resm(TFAIL, "child was killed with signal %s",
-					tst_strsig(WTERMSIG(status)));
-			return;
-		}
-	}
-
-	if (ret)
-		tst_resm(TFAIL, "SysV sem: communication with identical keys"
-				" between namespaces");
-	else
-		tst_resm(TPASS, "SysV sem: communication with identical keys"
-				" between namespaces");
+	clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld1_sem, NULL);
+	clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld2_sem, NULL);
 }
 
-int main(int argc, char *argv[])
+static void setup(void)
 {
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++)
-		test();
-
-	cleanup();
-	tst_exit();
+	check_newipc();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 04/10] Rewrite sem_nstest.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
                   ` (2 preceding siblings ...)
  2022-03-10 10:59 ` [LTP] [PATCH v2 03/10] Rewrite sem_comm.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 15:54   ` Cyril Hrubis
  2022-03-10 10:59 ` [LTP] [PATCH v2 05/10] Rewrite semtest_2ns.c " Andrea Cervesato
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/sem_nstest.c    | 212 +++++++-----------
 1 file changed, 76 insertions(+), 136 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/sem_nstest.c b/testcases/kernel/containers/sysvipc/sem_nstest.c
index 72e04fe4f..514af6c4f 100644
--- a/testcases/kernel/containers/sysvipc/sem_nstest.c
+++ b/testcases/kernel/containers/sysvipc/sem_nstest.c
@@ -1,158 +1,98 @@
-/* *************************************************************************
-* Copyright (c) International Business Machines Corp., 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Veerendra C <vechandr@in.ibm.com>
-*
-* In Parent Process , create semaphore with key 154326L
-* Now create container by passing 1 of the below flag values..
-*	clone(NONE), clone(CLONE_NEWIPC), or unshare(CLONE_NEWIPC)
-* In cloned process, try to access the created semaphore
-* Test PASS: If the semaphore is readable when flag is None.
-* Test FAIL: If the semaphore is readable when flag is Unshare or Clone.
-***************************************************************************/
-
-#define _GNU_SOURCE 1
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <sys/ipc.h>
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) International Business Machines Corp., 2009
+ *				Veerendra C <vechandr@in.ibm.com>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test SysV IPC semaphore usage between namespaces.
+ *
+ * [Algorithm]
+ *
+ * In parent process create a new semaphore with a specific key.
+ * In cloned process, try to access the created semaphore
+ *
+ * Test PASS if the semaphore is readable when flag is None.
+ * Test FAIL if the semaphore is readable when flag is Unshare or Clone.
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/wait.h>
+#include <sys/msg.h>
+#include <sys/types.h>
 #include <sys/sem.h>
-#include <libclone.h>
-#include "test.h"
-#include "ipcns_helper.h"
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
+#include "common.h"
 
-#define MY_KEY     154326L
-#define UNSHARESTR "unshare"
-#define CLONESTR   "clone"
-#define NONESTR    "none"
+#define MY_KEY 154326L
 
-char *TCID = "sem_nstest";
-int TST_TOTAL = 1;
-int p1[2];
-int p2[2];
+static char *str_op = "clone";
+static int use_clone;
+static int ipc_id = -1;
 
-int check_semaphore(void *vtest)
+static int check_semaphore(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	char buf[3];
 	int id;
 
-	(void) vtest;
+	id = semget(MY_KEY, 1, 0);
 
-	close(p1[1]);
-	close(p2[0]);
+	if (id < 0) {
+		if (use_clone == T_NONE)
+			tst_res(TFAIL, "Plain cloned process didn't find semaphore");
+		else
+			tst_res(TPASS, "%s: container didn't find semaphore", str_op);
+	} else {
+		tst_res(TINFO, "PID %d: fetched existing semaphore..id = %d", getpid(), id);
 
-	read(p1[0], buf, 3);
-	id = semget(MY_KEY, 1, 0);
-	if (id == -1)
-		write(p2[1], "notfnd", 7);
-	else {
-		write(p2[1], "exists", 7);
-		tst_resm(TINFO, "PID %d: Fetched existing semaphore..id = %d",
-			 getpid(), id);
+		if (use_clone == T_NONE)
+			tst_res(TPASS, "Plain cloned process found semaphore inside container");
+		else
+			tst_res(TFAIL, "%s: Container init process found semaphore", str_op);
 	}
-	tst_exit();
+
+	return 0;
 }
 
-static void setup(void)
+static void run(void)
 {
-	tst_require_root();
-	check_newipc();
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_semaphore, NULL);
 }
 
-int main(int argc, char *argv[])
+static void setup(void)
 {
-	int ret, use_clone = T_NONE, id;
-	char *tsttype = NONESTR;
-	char buf[7];
-
-	setup();
-
-	if (argc != 2) {
-		tst_resm(TFAIL, "Usage: %s <clone| unshare| none>", argv[0]);
-		tst_brkm(TFAIL, NULL, " where clone, unshare, or fork specifies"
-			 " unshare method.");
-	}
+	use_clone = get_clone_unshare_enum(str_op);
 
-	/* Using PIPE's to sync between container and Parent */
-	if (pipe(p1) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
-	if (pipe(p2) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
+	if (use_clone != T_NONE)
+		check_newipc();
 
-	if (strcmp(argv[1], "clone") == 0) {
-		use_clone = T_CLONE;
-		tsttype = CLONESTR;
-	} else if (strcmp(argv[1], "unshare") == 0) {
-		use_clone = T_UNSHARE;
-		tsttype = UNSHARESTR;
+	ipc_id = semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666);
+	if (ipc_id < 0) {
+		tst_res(TINFO, "Semaphore already exists");
+		ipc_id = SAFE_SEMGET(MY_KEY, 1, 0);
 	}
+}
 
-	/* 1. Create (or fetch if existing) the binary semaphore */
-	id = semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666);
-	if (id == -1) {
-		perror("Semaphore create");
-		if (errno != EEXIST) {
-			perror("semget failure");
-			tst_brkm(TBROK, NULL, "Semaphore creation failed");
-		}
-		id = semget(MY_KEY, 1, 0);
-		if (id == -1) {
-			perror("Semaphore create");
-			tst_brkm(TBROK, NULL, "Semaphore operation failed");
-		}
-	}
-
-	tst_resm(TINFO, "Semaphore namespaces Isolation test : %s", tsttype);
-	/* fire off the test */
-	ret =
-	    do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_semaphore,
-				  NULL);
-	if (ret < 0) {
-		tst_brkm(TFAIL, NULL, "%s failed", tsttype);
-	}
-
-	close(p1[0]);
-	close(p2[1]);
-	write(p1[1], "go", 3);
-	read(p2[0], buf, 7);
-
-	if (strcmp(buf, "exists") == 0) {
-		if (use_clone == T_NONE)
-			tst_resm(TPASS, "Plain cloned process found semaphore "
-				 "inside container");
-		else
-			tst_resm(TFAIL,
-				 "%s: Container init process found semaphore",
-				 tsttype);
-	} else {
-		if (use_clone == T_NONE)
-			tst_resm(TFAIL,
-				 "Plain cloned process didn't find semaphore");
-		else
-			tst_resm(TPASS, "%s: Container didn't find semaphore",
-				 tsttype);
+static void cleanup(void)
+{
+	if (ipc_id != -1) {
+		tst_res(TINFO, "Delete semaphore");
+		SAFE_SEMCTL(ipc_id, IPC_RMID, 0);
 	}
-
-	/* Delete the semaphore */
-	id = semget(MY_KEY, 1, 0);
-	semctl(id, IPC_RMID, 0);
-
-	tst_exit();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Test execution mode <clone|unshare|none>" },
+		{},
+	},
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 05/10] Rewrite semtest_2ns.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
                   ` (3 preceding siblings ...)
  2022-03-10 10:59 ` [LTP] [PATCH v2 04/10] Rewrite sem_nstest.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 16:37   ` Cyril Hrubis
  2022-03-10 10:59 ` [LTP] [PATCH v2 06/10] Rewrite shm_comm.c " Andrea Cervesato
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/semtest_2ns.c   | 295 +++++++-----------
 1 file changed, 107 insertions(+), 188 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/semtest_2ns.c b/testcases/kernel/containers/sysvipc/semtest_2ns.c
index c3483b675..c0c6f445e 100644
--- a/testcases/kernel/containers/sysvipc/semtest_2ns.c
+++ b/testcases/kernel/containers/sysvipc/semtest_2ns.c
@@ -1,230 +1,149 @@
-/* *************************************************************************
-* Copyright (c) International Business Machines Corp., 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Veerendra C <vechandr@in.ibm.com>
-*
-* Test Assertion:
-* This testcase verifies the semaphore isoloation in 2 diff containers.
-* It tries to create/access a semaphore created with the same KEY.
-*
-* Description:
-* Create 2 'containers' with the below flag value
-*   Flag = clone, clone(CLONE_NEWIPC), or unshare(CLONE_NEWIPC)
-* In Cont1, create semaphore with key 124326L
-* In Cont2, try to access the semaphore created in Cont1.
-* PASS :
-*		If flag = None and the semaphore is accessible in Cont2.
-*		If flag = unshare/clone and the semaphore is not accessible in Cont2.
-*		If semaphore is not accessible in Cont2, creates new semaphore with
-*		the same key to double check isloation in IPCNS.
-*
-* FAIL :
-*		If flag = none and the semaphore is not accessible.
-*		If flag = unshare/clone and semaphore is accessible in Cont2.
-*		If the new semaphore creation Fails.
-***************************************************************************/
-
-#define _GNU_SOURCE 1
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <sys/ipc.h>
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) International Business Machines Corp., 2009
+ *				Veerendra C <vechandr@in.ibm.com>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test SysV IPC semaphore usage between namespaces.
+ *
+ * [Algorithm]
+ *
+ * Create 2 'containers'
+ * In container1 create semaphore with a specific key
+ * In container2 try to access the semaphore created in container1
+ *
+ * Test is PASS if flag = none and the semaphore is accessible in container2 or
+ * if flag = unshare/clone and the semaphore is not accessible in container2.
+ * If semaphore is not accessible in container2, creates new semaphore with the
+ * same key to double check isloation in IPCNS.
+ *
+ * Test is FAIL if flag = none and the semaphore is not accessible, if
+ * flag = unshare/clone and semaphore is accessible in container2 or if the new
+ * semaphore creation Fails.
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/wait.h>
+#include <sys/msg.h>
+#include <sys/types.h>
 #include <sys/sem.h>
-#include <libclone.h>
-#include "test.h"
-#include "ipcns_helper.h"
-
-#define MY_KEY     124326L
-#define UNSHARESTR "unshare"
-#define CLONESTR   "clone"
-#define NONESTR    "none"
-
-char *TCID = "semtest_2ns";
-int TST_TOTAL = 1;
-int p1[2];
-int p2[2];
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
+#include "common.h"
+
+#define MY_KEY 124326L
+
+static char *str_op = "clone";
+static int use_clone;
+
 static struct sembuf semop_lock[2] = {
 	/* sem_num, sem_op, flag */
-	{0, 0, 0},		/* wait for sem#0 to become 0 */
-	{0, 1, SEM_UNDO}	/* then increment sem#0 by 1 */
+	{ 0, 0, 0 }, /* wait for sem#0 to become 0 */
+	{ 0, 1, SEM_UNDO } /* then increment sem#0 by 1 */
 };
 
 static struct sembuf semop_unlock[1] = {
 	/* sem_num, sem_op, flag */
-	{0, -1, (IPC_NOWAIT | SEM_UNDO)}	/* decrement sem#0 by 1 (sets it to 0) */
+	{ 0, -1, (IPC_NOWAIT | SEM_UNDO) } /* decrement sem#0 by 1 (sets it to 0) */
 };
 
 /*
  * sem_lock() - Locks the semaphore for crit-sec updation, and unlocks it later
  */
-void sem_lock(int id)
+static void sem_lock(int id)
 {
-	/* Checking the semlock and simulating as if the crit-sec is updated */
-	if (semop(id, &semop_lock[0], 2) < 0) {
-		perror("sem lock error");
-		tst_brkm(TBROK, NULL, "semop failed");
-	}
-	tst_resm(TINFO, "Sem1: File locked, Critical section is updated...");
+	SAFE_SEMOP(id, &semop_lock[0], 2);
+
+	tst_res(TINFO, "semaphore1: File locked, Critical section is updated...");
+
 	sleep(2);
-	if (semop(id, &semop_unlock[0], 1) < 0) {
-		perror("sem unlock error");
-		tst_brkm(TBROK, NULL, "semop failed");
-	}
+
+	SAFE_SEMOP(id, &semop_unlock[0], 1);
 }
 
 /*
  * check_sem1 -  does not read -- it writes to check_sem2() when it's done.
  */
-int check_sem1(void *vtest)
+static int check_sem1(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	int id1;
-
-	(void) vtest;
-
-	close(p1[0]);
-	/* 1. Create (or fetch if existing) the binary semaphore */
-	id1 = semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666);
-	if (id1 == -1) {
-		perror("Semaphore create");
-		if (errno != EEXIST) {
-			perror("semget failure");
-			tst_brkm(TBROK, NULL, "semget failure");
-		}
-		id1 = semget(MY_KEY, 1, 0);
-		if (id1 == -1) {
-			perror("Semaphore create");
-			tst_brkm(TBROK, NULL, "semget failure");
-		}
+	TEST(semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666));
+	if (TST_RET < 0) {
+		tst_res(TINFO, "semget failure. Checking existing semaphore");
+
+		if (TST_ERR != EEXIST)
+			tst_brk(TBROK | TRERRNO, "Semaphore creation failed");
+
+		SAFE_SEMGET(MY_KEY, 1, 0);
 	}
 
-	write(p1[1], "go", 3);
-	tst_resm(TINFO, "Cont1: Able to create semaphore");
-	tst_exit();
+	tst_res(TINFO, "container1: Able to create semaphore");
+
+	return 0;
 }
 
 /*
  * check_sem2() reads from check_sem1() and writes to main() when it's done.
  */
-
-int check_sem2(void *vtest)
+static int check_sem2(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	char buf[3];
-	int id2;
-
-	(void) vtest;
+	int id;
 
-	close(p1[1]);
-	close(p2[0]);
-	read(p1[0], buf, 3);
+	id = semget(MY_KEY, 1, 0);
+	if (id >= 0) {
+		sem_lock(id);
 
-	id2 = semget(MY_KEY, 1, 0);
-	if (id2 != -1) {
-		sem_lock(id2);
-		write(p2[1], "exists", 7);
+		if (use_clone == T_NONE)
+			tst_res(TPASS, "Plain cloned process able to access the semaphore created");
+		else
+			tst_res(TFAIL, "%s : In namespace2 found the semaphore created in Namespace1", str_op);
 	} else {
 		/* Trying to create a new semaphore, if semaphore is not existing */
-		id2 = semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666);
-		if (id2 == -1) {
-			perror("Semaphore create");
-			if (errno != EEXIST) {
-				perror("semget failure");
-				tst_resm(TBROK, "semget failure");
-			}
-		} else
-			tst_resm(TINFO,
-				 "Cont2: Able to create semaphore with sameKey");
-		/* Passing the pipe Not-found mesg */
-		write(p2[1], "notfnd", 7);
+		TEST(semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666));
+		if (TST_RET < 0) {
+			if (TST_ERR != EEXIST)
+				tst_brk(TBROK | TERRNO, "semget error");
+		} else {
+			tst_res(TINFO, "container2: Able to create semaphore with sameKey");
+		}
+
+		if (use_clone == T_NONE)
+			tst_res(TFAIL, "Plain cloned process didn't find semaphore");
+		else
+			tst_res(TPASS, "%s : In namespace2 unable to access the semaphore created in namespace1", str_op);
 	}
 
-	tst_exit();
+	id = SAFE_SEMGET(MY_KEY, 1, 0);
+	SAFE_SEMCTL(id, IPC_RMID, 0);
+
+	return 0;
 }
 
-static void setup(void)
+static void run(void)
 {
-	tst_require_root();
-	check_newipc();
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_sem1, NULL);
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_sem2, NULL);
 }
 
-int main(int argc, char *argv[])
+static void setup(void)
 {
-	int ret, id, use_clone = T_NONE;
-	char *tsttype = NONESTR;
-	char buf[7];
-
-	setup();
-
-	if (argc != 2) {
-		tst_resm(TINFO, "Usage: %s <clone| unshare| none>", argv[0]);
-		tst_resm(TINFO, " where clone, unshare, or fork specifies"
-			 " unshare method.");
-		tst_exit();
-	}
-
-	/* Using PIPE's to sync between container and Parent */
-	if (pipe(p1) == -1) {
-		perror("pipe1");
-		tst_exit();
-	}
-	if (pipe(p2) == -1) {
-		perror("pipe2");
-		tst_exit();
-	}
+	use_clone = get_clone_unshare_enum(str_op);
 
-	if (strcmp(argv[1], "clone") == 0) {
-		use_clone = T_CLONE;
-		tsttype = CLONESTR;
-	} else if (strcmp(argv[1], "unshare") == 0) {
-		use_clone = T_UNSHARE;
-		tsttype = UNSHARESTR;
-	}
-
-	tst_resm(TINFO, "Semaphore Namespaces Test : %s", tsttype);
-
-	/* Create 2 containers */
-	ret = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_sem1, NULL);
-	if (ret < 0) {
-		tst_brkm(TFAIL, NULL, "clone/unshare failed");
-	}
-
-	ret = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_sem2, NULL);
-	if (ret < 0) {
-		tst_brkm(TFAIL, NULL, "clone/unshare failed");
-	}
-	close(p2[1]);
-	read(p2[0], buf, 7);
-
-	if (strcmp(buf, "exists") == 0)
-		if (use_clone == T_NONE)
-			tst_resm(TPASS,
-				 "Plain cloned process able to access the semaphore "
-				 "created");
-		else
-			tst_resm(TFAIL,
-				 "%s : In namespace2 found the semaphore "
-				 "created in Namespace1", tsttype);
-	else if (use_clone == T_NONE)
-		tst_resm(TFAIL, "Plain cloned process didn't find semaphore");
-	else
-		tst_resm(TPASS,
-			 "%s : In namespace2 unable to access the semaphore "
-			 "created in Namespace1", tsttype);
-
-	/* Delete the semaphore */
-	id = semget(MY_KEY, 1, 0);
-	semctl(id, IPC_RMID, 0);
-	tst_exit();
+	if (use_clone != T_NONE)
+		check_newipc();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Test execution mode <clone|unshare|none>" },
+		{},
+	},
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 06/10] Rewrite shm_comm.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
                   ` (4 preceding siblings ...)
  2022-03-10 10:59 ` [LTP] [PATCH v2 05/10] Rewrite semtest_2ns.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 17:01   ` Cyril Hrubis
  2022-03-10 10:59 ` [LTP] [PATCH v2 07/10] Rewrite shmem_2nstest.c " Andrea Cervesato
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/shm_comm.c      | 167 ++++++------------
 1 file changed, 50 insertions(+), 117 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/shm_comm.c b/testcases/kernel/containers/sysvipc/shm_comm.c
index 4b3bbfaa8..fc4f067e9 100644
--- a/testcases/kernel/containers/sysvipc/shm_comm.c
+++ b/testcases/kernel/containers/sysvipc/shm_comm.c
@@ -1,20 +1,17 @@
-/* Copyright (c) 2014 Red Hat, Inc.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of version 2 the GNU General Public License as
- * published by the Free Software Foundation.
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
  *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
+ * Test if SysV IPC shared memory is properly working between two different
+ * namespaces.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- ***********************************************************************
- * File: shm_comm.c
+ * [Algorithm]
  *
- * Description:
  * 1. Clones two child processes with CLONE_NEWIPC flag, each child
  *    allocates System V shared memory segment (shm) with the _identical_
  *    key and attaches that segment into its address space.
@@ -27,141 +24,77 @@
  */
 
 #define _GNU_SOURCE
-#include <sys/ipc.h>
-#include <sys/shm.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <stdio.h>
-#include <errno.h>
-#include "ipcns_helper.h"
-#include "test.h"
-#include "safe_macros.h"
 
+#include <sys/wait.h>
+#include <sys/msg.h>
+#include <sys/types.h>
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
+#include "common.h"
 
 #define TESTKEY 124426L
 #define SHMSIZE 50
-char *TCID	= "shm_comm";
-int TST_TOTAL	= 1;
-
-static void cleanup(void)
-{
-	tst_rmdir();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-	check_newipc();
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(tst_rmdir);
-}
 
-int chld1_shm(void *arg)
+static int chld1_shm(LTP_ATTRIBUTE_UNUSED void *arg)
 {
-	int id, rval = 0;
+	int id;
 	char *shmem;
 
-	id = shmget(TESTKEY, SHMSIZE, IPC_CREAT);
-	if (id == -1) {
-		perror("shmget");
-		return 2;
-	}
-
-	if ((shmem = shmat(id, NULL, 0)) == (char *) -1) {
-		perror("shmat");
-		shmctl(id, IPC_RMID, NULL);
-		return 2;
-	}
+	id = SAFE_SHMGET(TESTKEY, SHMSIZE, IPC_CREAT);
 
+	shmem = SAFE_SHMAT(id, NULL, 0);
 	*shmem = 'A';
 
-	TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
 
-	/* if child1 shared segment has changed (by child2) report fail */
 	if (*shmem != 'A')
-		rval = 1;
+		tst_res(TFAIL, "SysV shm: communication with non-identical keys between namespaces");
+	else
+		tst_res(TPASS, "SysV shm: communication with identical keys between namespaces");
+
+	TST_CHECKPOINT_WAKE(0);
 
-	/* tell child2 to continue */
-	TST_SAFE_CHECKPOINT_WAKE(NULL, 0);
+	SAFE_SHMDT(shmem);
+	SAFE_SHMCTL(id, IPC_RMID, NULL);
 
-	shmdt(shmem);
-	shmctl(id, IPC_RMID, NULL);
-	return rval;
+	return 0;
 }
 
-int chld2_shm(void *arg)
+static int chld2_shm(LTP_ATTRIBUTE_UNUSED void *arg)
 {
 	int id;
 	char *shmem;
 
-	id = shmget(TESTKEY, SHMSIZE, IPC_CREAT);
-	if (id == -1) {
-		perror("shmget");
-		return 2;
-	}
+	id = SAFE_SHMGET(TESTKEY, SHMSIZE, IPC_CREAT);
 
-	if ((shmem = shmat(id, NULL, 0)) == (char *) -1) {
-		perror("shmat");
-		shmctl(id, IPC_RMID, NULL);
-		return 2;
-	}
+	shmem = SAFE_SHMAT(id, NULL, 0);
 
-	/* wait for child1 to write to his segment */
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
 
 	*shmem = 'B';
 
-	TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
+
+	SAFE_SHMDT(shmem);
+	SAFE_SHMCTL(id, IPC_RMID, NULL);
 
-	shmdt(shmem);
-	shmctl(id, IPC_RMID, NULL);
 	return 0;
 }
 
-static void test(void)
+static void run(void)
 {
-	int status, ret = 0;
-
-	ret = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld1_shm, NULL);
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
-
-	ret = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld2_shm, NULL);
-	if (ret == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
-
-
-	while (wait(&status) > 0) {
-		if (WIFEXITED(status) && WEXITSTATUS(status) == 1)
-			ret = 1;
-		if (WIFEXITED(status) && WEXITSTATUS(status) == 2)
-			tst_brkm(TBROK | TERRNO, cleanup, "error in child");
-		if (WIFSIGNALED(status)) {
-			tst_resm(TFAIL, "child was killed with signal %s",
-					tst_strsig(WTERMSIG(status)));
-			return;
-		}
-	}
-
-	if (ret)
-		tst_resm(TFAIL, "SysV shm: communication with identical keys"
-				" between namespaces");
-	else
-		tst_resm(TPASS, "SysV shm: communication with identical keys"
-				" between namespaces");
+	clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld1_shm, NULL);
+	clone_unshare_test(T_CLONE, CLONE_NEWIPC, chld2_shm, NULL);
 }
 
-int main(int argc, char *argv[])
+static void setup(void)
 {
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++)
-		test();
-
-	cleanup();
-	tst_exit();
+	check_newipc();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 07/10] Rewrite shmem_2nstest.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
                   ` (5 preceding siblings ...)
  2022-03-10 10:59 ` [LTP] [PATCH v2 06/10] Rewrite shm_comm.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 08/10] Rewrite shmnstest.c " Andrea Cervesato
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/shmem_2nstest.c | 239 ++++++------------
 1 file changed, 83 insertions(+), 156 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/shmem_2nstest.c b/testcases/kernel/containers/sysvipc/shmem_2nstest.c
index b172ee07c..fe3715509 100644
--- a/testcases/kernel/containers/sysvipc/shmem_2nstest.c
+++ b/testcases/kernel/containers/sysvipc/shmem_2nstest.c
@@ -1,187 +1,114 @@
-/* *************************************************************************
-* Copyright (c) International Business Machines Corp., 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Veerendra C <vechandr@in.ibm.com>
-*
-* Test Assertion:
-* This testcase verifies the Shared Memory isoloation in 2 containers.
-* It tries to create/access a Shared Memory created with the same KEY.
-*
-* Description:
-* Create 2 'containers' with the below flag value
-*   Flag = clone, clone(CLONE_NEWIPC), or unshare(CLONE_NEWIPC)
-* In Cont1, create Shared Memory segment with key 124426L
-* In Cont2, try to access the MQ created in Cont1.
-* PASS :
-* 		If flag = None and the shmem seg is accessible in Cont2.
-*		If flag = unshare/clone and the shmem seg is not accessible in Cont2.
-* 		If shmem seg is not accessible in Cont2,
-*		creates new shmem with same key to double check isloation in IPCNS.
-*
-* FAIL :
-* 		If flag = none and the shmem seg is not accessible.
-* 		If flag = unshare/clone and shmem seg is accessible in Cont2.
-*		If the new shmem seg creation Fails.
-***************************************************************************/
-
-#define _GNU_SOURCE 1
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <sys/ipc.h>
-#include <sys/shm.h>
-#include <libclone.h>
-#include "test.h"
-#include "safe_macros.h"
-#include "ipcns_helper.h"
-
-#define TESTKEY    124426L
-#define UNSHARESTR "unshare"
-#define CLONESTR   "clone"
-#define NONESTR    "none"
-
-char *TCID = "shmem_2nstest";
-int TST_TOTAL = 1;
-int p2[2];
-int p1[2];
-
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * check_shmem1() does not read -- it writes to check_shmem2() when it's done.
+ * Copyright (c) International Business Machines Corp., 2009
+ *				Veerendra C <vechandr@in.ibm.com>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test if SysV IPC shared memory is properly used between two namespaces.
+ *
+ * [Algorithm]
+ *
+ * Create two 'containers'.
+ * In container1, create Shared Memory segment with a specific key.
+ * In container2, try to access the MQ created in container1.
+ *
+ * Test is PASS if flag = none and the shmem seg is accessible in container2.
+ * If flag = unshare/clone and the shmem seg is not accessible in container2.
+ * If shmem seg is not accessible in container2, creates new shmem with same
+ * key to double check isloation in IPCNS.
+ *
+ * Test is FAIL if flag = none and the shmem seg is not accessible, if
+ * flag = unshare/clone and shmem seg is accessible in container2 or if the new
+ * shmem seg creation Fails.
  */
-int check_shmem1(void *vtest)
-{
-	int id1;
 
-	(void) vtest;
+#define _GNU_SOURCE
 
-	close(p1[0]);
+#include <sys/wait.h>
+#include <sys/msg.h>
+#include <sys/types.h>
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
+#include "common.h"
 
-	/* first create the key */
-	id1 = shmget(TESTKEY, 100, IPC_CREAT);
-	if (id1 == -1)
-		tst_brkm(TFAIL | TERRNO, NULL, "shmget failed");
+#define TESTKEY 124426L
 
-	tst_resm(TINFO, "Cont1: Able to create shared mem segment");
-	write(p1[1], "done", 5);
-	tst_exit();
-}
+static char *str_op = "clone";
+static int use_clone;
 
 /*
- * check_shmem2() reads from check_shmem1() and writes to main() when it's done.
+ * check_shmem1() does not read -- it writes to check_shmem2() when it's done.
  */
-int check_shmem2(void *vtest)
+static int check_shmem1(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	char buf[3];
-	int id2;
+	SAFE_SHMGET(TESTKEY, 100, IPC_CREAT);
 
-	(void) vtest;
+	tst_res(TINFO, "container1: able to create shared mem segment");
 
-	close(p1[1]);
-	close(p2[0]);
+	TST_CHECKPOINT_WAKE(0);
 
-	read(p1[0], buf, 3);
-	/* Trying to access shmem, if not existing create new shmem */
-	id2 = shmget(TESTKEY, 100, 0);
-	if (id2 == -1) {
-		id2 = shmget(TESTKEY, 100, IPC_CREAT);
-		if (id2 == -1)
-			tst_resm(TFAIL | TERRNO, "shmget failed");
-		else
-			tst_resm(TINFO,
-				 "Cont2: Able to allocate shmem seg with "
-				 "the same key");
-		write(p2[1], "notfnd", 7);
-	} else
-		write(p2[1], "exists", 7);
-
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-	check_newipc();
+	return 0;
 }
 
-int main(int argc, char *argv[])
+/*
+ * check_shmem2() reads from check_shmem1() and writes to main() when it's done.
+ */
+static int check_shmem2(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	int ret, use_clone = T_NONE;
-	char *tsttype = NONESTR;
-	char buf[7];
 	int id;
 
-	setup();
+	TST_CHECKPOINT_WAIT(0);
 
-	if (argc != 2) {
-		tst_resm(TINFO, "Usage: %s <clone| unshare| none>", argv[0]);
-		tst_resm(TINFO, " where clone, unshare, or fork specifies"
-			 " unshare method.");
-		tst_exit();
-	}
-
-	/* Using PIPE's to sync between containers and Parent */
-	SAFE_PIPE(NULL, p1);
-	SAFE_PIPE(NULL, p2);
-
-	if (strcmp(argv[1], "clone") == 0) {
-		use_clone = T_CLONE;
-		tsttype = CLONESTR;
-	} else if (strcmp(argv[1], "unshare") == 0) {
-		use_clone = T_UNSHARE;
-		tsttype = UNSHARESTR;
-	}
+	TEST(shmget(TESTKEY, 100, 0));
 
-	tst_resm(TINFO, "Shared Memory namespace test : %s", tsttype);
+	if (TST_RET < 0) {
+		SAFE_SHMGET(TESTKEY, 100, IPC_CREAT);
 
-	/* Create 2 containers */
-	ret =
-	    do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_shmem1, NULL);
-	if (ret < 0)
-		tst_brkm(TFAIL, NULL, "clone/unshare failed");
+		tst_res(TINFO, "container2: able to allocate shmem seg with the same key");
 
-	ret =
-	    do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_shmem2, NULL);
-	if (ret < 0)
-		tst_brkm(TFAIL, NULL, "clone/unshare failed");
-
-	close(p2[1]);
-	read(p2[0], buf, 7);
-
-	if (strcmp(buf, "exists") == 0) {
 		if (use_clone == T_NONE)
-			tst_resm(TPASS,
-				 "Plain cloned process able to access shmem "
-				 "segment created");
+			tst_res(TFAIL, "Plain cloned process didn't find shmem segment");
 		else
-			tst_resm(TFAIL,
-				 "%s : In namespace2 found the shmem segment "
-				 "created in Namespace1", tsttype);
+			tst_res(TPASS, "%s: in namespace2 unable to access the shmem seg created in namespace1", str_op);
 	} else {
 		if (use_clone == T_NONE)
-			tst_resm(TFAIL,
-				 "Plain cloned process didn't find shmem seg");
+			tst_res(TPASS, "Plain cloned process able to access shmem segment created");
 		else
-			tst_resm(TPASS,
-				 "%s : In namespace2 unable to access the shmem seg "
-				 "created in Namespace1", tsttype);
+			tst_res(TFAIL, "%s: in namespace2 found the shmem segment created in namespace1", str_op);
 	}
-	/* destroy the key */
 
-	id = shmget(TESTKEY, 100, 0);
-	shmctl(id, IPC_RMID, NULL);
+	id = SAFE_SHMGET(TESTKEY, 100, 0);
+	SAFE_SHMCTL(id, IPC_RMID, NULL);
 
-	tst_exit();
+	return 0;
 }
+
+static void run(void)
+{
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_shmem1, NULL);
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_shmem2, NULL);
+}
+
+static void setup(void)
+{
+	use_clone = get_clone_unshare_enum(str_op);
+
+	if (use_clone != T_NONE)
+		check_newipc();
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.forks_child = 1,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Test execution mode <clone|unshare|none>" },
+		{},
+	},
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 08/10] Rewrite shmnstest.c using new LTP API
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
                   ` (6 preceding siblings ...)
  2022-03-10 10:59 ` [LTP] [PATCH v2 07/10] Rewrite shmem_2nstest.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 09/10] Remove libclone dependency from sysvipc Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 10/10] Delete obsolete ipcns_helper.h in sysvipc suite Andrea Cervesato
  9 siblings, 0 replies; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/shmnstest.c     | 182 ++++++------------
 1 file changed, 62 insertions(+), 120 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/shmnstest.c b/testcases/kernel/containers/sysvipc/shmnstest.c
index cf69cab21..0b0b4a3b7 100644
--- a/testcases/kernel/containers/sysvipc/shmnstest.c
+++ b/testcases/kernel/containers/sysvipc/shmnstest.c
@@ -1,144 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
-* Copyright (c) International Business Machines Corp., 2007
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Serge Hallyn <serue@us.ibm.com>
-*
-* Create shm with key 0xEAEAEA
-* clone, clone(CLONE_NEWIPC), or unshare(CLONE_NEWIPC)
-* In cloned process, try to get the created shm
+ * Copyright (c) International Business Machines Corp., 2007
+ *				Serge Hallyn <serue@us.ibm.com>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
 
-***************************************************************************/
+/*\
+ * [Description]
+ *
+ * Test if SysV IPC shared memory with a specific key is shared between
+ * processes and namespaces.
+ */
+
+#define _GNU_SOURCE
 
-#define _GNU_SOURCE 1
 #include <sys/wait.h>
-#include <assert.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include <sys/ipc.h>
-#include <sys/shm.h>
-#include "ipcns_helper.h"
-#include "test.h"
+#include <sys/msg.h>
+#include <sys/types.h>
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
+#include "common.h"
 
-char *TCID = "sysvipc_namespace";
-int TST_TOTAL = 1;
 #define TESTKEY 0xEAEAEA
 
-int p1[2];
-int p2[2];
+static char *str_op = "clone";
+static int use_clone;
+static int ipc_id = -1;
 
-int check_shmid(void *vtest)
+static int check_shmid(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	char buf[3];
-	int id;
-
-	(void) vtest;
-
-	close(p1[1]);
-	close(p2[0]);
-
-	read(p1[0], buf, 3);
-	id = shmget(TESTKEY, 100, 0);
-	if (id == -1) {
-		write(p2[1], "notfnd", 7);
+	TEST(shmget(TESTKEY, 100, 0));
+	if (TST_RET < 0) {
+		if (use_clone == T_NONE)
+			tst_res(TFAIL, "plain cloned process didn't find shmid");
+		else
+			tst_res(TPASS, "%s: child process didn't find shmid", str_op);
 	} else {
-		write(p2[1], "exists", 7);
-		shmctl(id, IPC_RMID, NULL);
+		if (use_clone == T_NONE)
+			tst_res(TPASS, "plain cloned process found shmid");
+		else
+			tst_res(TFAIL, "%s: child process found shmid", str_op);
 	}
 
-	tst_exit();
+	return 0;
 }
 
-static void setup(void)
+static void run(void)
 {
-	tst_require_root();
-	check_newipc();
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_shmid, NULL);
 }
 
-#define UNSHARESTR "unshare"
-#define CLONESTR "clone"
-#define NONESTR "none"
-int main(int argc, char *argv[])
+static void setup(void)
 {
-	int r, use_clone = T_NONE;
-	int id;
-	char *tsttype = NONESTR;
-	char buf[7];
+	use_clone = get_clone_unshare_enum(str_op);
 
-	setup();
+	if (use_clone != T_NONE)
+		check_newipc();
 
-	if (argc != 2) {
-		tst_resm(TFAIL, "Usage: %s <clone|unshare|none>", argv[0]);
-		tst_brkm(TFAIL,
-			 NULL,
-			 " where clone, unshare, or fork specifies unshare method.");
-	}
-	if (pipe(p1) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
-	if (pipe(p2) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
-	tsttype = NONESTR;
-	if (strcmp(argv[1], "clone") == 0) {
-		use_clone = T_CLONE;
-		tsttype = CLONESTR;
-	} else if (strcmp(argv[1], "unshare") == 0) {
-		use_clone = T_UNSHARE;
-		tsttype = UNSHARESTR;
-	}
-
-	/* first create the key */
-	id = shmget(TESTKEY, 100, IPC_CREAT);
-	if (id == -1) {
-		perror("shmget");
-		tst_brkm(TFAIL, NULL, "shmget failed");
-	}
-
-	tst_resm(TINFO, "shmid namespaces test : %s", tsttype);
-	/* fire off the test */
-	r = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_shmid, NULL);
-	if (r < 0) {
-		tst_brkm(TFAIL, NULL, "%s failed", tsttype);
+	ipc_id = shmget(TESTKEY, 100, IPC_CREAT);
+	if (ipc_id < 0) {
+		tst_res(TINFO, "Shared memory already exist");
+		ipc_id = SAFE_SHMGET(TESTKEY, 100, 0);
 	}
+}
 
-	close(p1[0]);
-	close(p2[1]);
-	write(p1[1], "go", 3);
-	read(p2[0], buf, 7);
-	if (strcmp(buf, "exists") == 0) {
-		if (use_clone == T_NONE)
-			tst_resm(TPASS, "plain cloned process found shmid");
-		else
-			tst_resm(TFAIL, "%s: child process found shmid",
-				 tsttype);
-	} else {
-		if (use_clone == T_NONE)
-			tst_resm(TFAIL,
-				 "plain cloned process didn't find shmid");
-		else
-			tst_resm(TPASS, "%s: child process didn't find shmid",
-				 tsttype);
+static void cleanup(void)
+{
+	if (ipc_id != -1) {
+		tst_res(TINFO, "Delete shared memory");
+		SAFE_SHMCTL(ipc_id, IPC_RMID, NULL);
 	}
-
-	/* destroy the key */
-	shmctl(id, IPC_RMID, NULL);
-
-	tst_exit();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Test execution mode <clone|unshare|none>" },
+		{},
+	},
+};
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 09/10] Remove libclone dependency from sysvipc
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
                   ` (7 preceding siblings ...)
  2022-03-10 10:59 ` [LTP] [PATCH v2 08/10] Rewrite shmnstest.c " Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  2022-03-10 10:59 ` [LTP] [PATCH v2 10/10] Delete obsolete ipcns_helper.h in sysvipc suite Andrea Cervesato
  9 siblings, 0 replies; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/sysvipc/Makefile | 26 +++-----------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/testcases/kernel/containers/sysvipc/Makefile b/testcases/kernel/containers/sysvipc/Makefile
index 00b537f6a..426fe5292 100644
--- a/testcases/kernel/containers/sysvipc/Makefile
+++ b/testcases/kernel/containers/sysvipc/Makefile
@@ -1,28 +1,8 @@
-################################################################################
-##                                                                            ##
-## Copyright (c) International Business Machines  Corp., 2007                 ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-################################################################################
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) International Business Machines  Corp., 2007
+# Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
 
 top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
-include $(abs_srcdir)/../Makefile.inc
-
-LDLIBS			:= -lclone $(LDLIBS)
-
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [LTP] [PATCH v2 10/10] Delete obsolete ipcns_helper.h in sysvipc suite
  2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
                   ` (8 preceding siblings ...)
  2022-03-10 10:59 ` [LTP] [PATCH v2 09/10] Remove libclone dependency from sysvipc Andrea Cervesato
@ 2022-03-10 10:59 ` Andrea Cervesato
  9 siblings, 0 replies; 19+ messages in thread
From: Andrea Cervesato @ 2022-03-10 10:59 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 .../kernel/containers/sysvipc/ipcns_helper.h  | 41 -------------------
 1 file changed, 41 deletions(-)
 delete mode 100644 testcases/kernel/containers/sysvipc/ipcns_helper.h

diff --git a/testcases/kernel/containers/sysvipc/ipcns_helper.h b/testcases/kernel/containers/sysvipc/ipcns_helper.h
deleted file mode 100644
index 01ad0fff6..000000000
--- a/testcases/kernel/containers/sysvipc/ipcns_helper.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
-* Copyright (c) International Business Machines Corp., 2007
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Rishikesh K Rajak <risrajak@in.ibm.com>
-***************************************************************************/
-#include <sched.h>
-#include "../libclone/libclone.h"
-#include "test.h"
-#include "safe_macros.h"
-
-static int dummy_child(void *v)
-{
-	(void) v;
-	return 0;
-}
-
-static void check_newipc(void)
-{
-	int pid, status;
-
-	if (tst_kvercmp(2, 6, 19) < 0)
-		tst_brkm(TCONF, NULL, "CLONE_NEWIPC not supported");
-
-	pid = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, dummy_child, NULL);
-	if (pid == -1)
-		tst_brkm(TCONF | TERRNO, NULL, "CLONE_NEWIPC not supported");
-
-	SAFE_WAIT(NULL, &status);
-}
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c using new LTP API
  2022-03-10 10:59 ` [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c " Andrea Cervesato
@ 2022-03-10 14:16   ` Cyril Hrubis
  2022-03-14 12:23     ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 19+ messages in thread
From: Cyril Hrubis @ 2022-03-10 14:16 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> ---
>  testcases/kernel/containers/sysvipc/common.h  | 157 +++++++++++
>  .../kernel/containers/sysvipc/mesgq_nstest.c  | 247 +++++++-----------
>  2 files changed, 254 insertions(+), 150 deletions(-)
>  create mode 100644 testcases/kernel/containers/sysvipc/common.h
> 
> diff --git a/testcases/kernel/containers/sysvipc/common.h b/testcases/kernel/containers/sysvipc/common.h
> new file mode 100644
> index 000000000..1fea6fafa
> --- /dev/null
> +++ b/testcases/kernel/containers/sysvipc/common.h
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) International Business Machines Corp., 2007
> + *               Rishikesh K Rajak <risrajak@in.ibm.com>
> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +#ifndef COMMON_H
> +#define COMMON_H
> +
> +#include <stdlib.h>
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +#include "lapi/namespaces_constants.h"
> +
> +enum {
> +	T_CLONE,
> +	T_UNSHARE,
> +	T_NONE,
> +};
> +
> +static int dummy_child(void *v)
> +{
> +	(void)v;
> +	return 0;
> +}
> +
> +static void check_newipc(void)
> +{
> +	int pid, status;
> +
> +	if (tst_kvercmp(2, 6, 19) < 0)
> +		tst_brk(TCONF, "CLONE_NEWIPC not supported");

2.6.19 was released in 2006 it should be safe enough to drop this check
now

> +	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child, NULL);
> +	if (pid < 0)
> +		tst_brk(TCONF | TERRNO, "CLONE_NEWIPC not supported");
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +}
> +
> +static inline int get_clone_unshare_enum(const char* str_op)
> +{
> +	int use_clone;
> +
> +	if (strcmp(str_op, "clone") &&
> +		strcmp(str_op, "unshare") &&
> +		strcmp(str_op, "none"))
> +		tst_brk(TBROK, "Test execution mode <clone|unshare|none>");
> +
> +	if (!strcmp(str_op, "clone"))
> +		use_clone = T_CLONE;
> +	else if (!strcmp(str_op, "unshare"))
> +		use_clone = T_UNSHARE;
> +	else
> +		use_clone = T_NONE;

Why do we have to parse the string twice?

Why not just:

	if (!strcmp(...)
		...
	else if (!strcmp(...))
		...
	else if (!strcmp(...))
		...
	else
		tst_brk(TBROK, "Invalid op '%s'", str_op);


> +	return use_clone;
> +}
> +
> +static int clone_test(unsigned long clone_flags, int (*fn1)(void *arg),
> +		      void *arg1)
> +{
> +	int ret;
> +
> +	ret = ltp_clone_quick(clone_flags | SIGCHLD, fn1, arg1);
> +
> +	if (ret != -1) {
> +		/* no errors: we'll get the PID id that means success */
> +		ret = 0;
> +	}
> +
> +	return ret;
> +}
> +
> +static int unshare_test(unsigned long clone_flags, int (*fn1)(void *arg),
> +			void *arg1)
> +{
> +	int pid, ret = 0;
> +	int retpipe[2];
> +	char buf[2];
> +
> +	SAFE_PIPE(retpipe);
> +
> +	pid = fork();
> +	if (pid < 0) {
> +		SAFE_CLOSE(retpipe[0]);
> +		SAFE_CLOSE(retpipe[1]);
> +		tst_brk(TBROK, "fork");
> +	}
> +
> +	if (!pid) {
> +		SAFE_CLOSE(retpipe[0]);
> +
> +		ret = tst_syscall(SYS_unshare, clone_flags);
> +		if (ret == -1) {
> +			SAFE_WRITE(1, retpipe[1], "0", 2);
> +			SAFE_CLOSE(retpipe[1]);
> +			exit(1);
> +		} else {
> +			SAFE_WRITE(1, retpipe[1], "1", 2);
> +		}
> +
> +		SAFE_CLOSE(retpipe[1]);
> +
> +		ret = fn1(arg1);
> +		exit(ret);
> +	}
> +
> +	SAFE_CLOSE(retpipe[1]);
> +	SAFE_READ(1, retpipe[0], &buf, 2);
> +	SAFE_CLOSE(retpipe[0]);
> +
> +	if (*buf == '0')
> +		return -1;
> +
> +	return ret;

Can we please clean up this mess as well? We can easily use
SAFE_MACROS() in the new library and we do not need to propagate errors
via pipes anymore. So this should really be just:

static void unshare_test(unsigned long clone_flags, int (*fn1)(void *arg),
                        void *arg1)
{
	int pid;

	pid = SAFE_FORK();

	if (!pid) {
		int ret;

		SAFE_UNSHARE(clone_flags);

		ret = fn1(arg);
		exit(ret);
	}
}

And once all tests are converted we can even drop some more of the code
since the fn1() can be declared to return void as well.

> +}
> +
> +static int plain_test(int (*fn1)(void *arg), void *arg1)
> +{
> +	int ret = 0, pid;
> +
> +	pid = SAFE_FORK();
> +	if (!pid)
> +		exit(fn1(arg1));
> +
> +	return ret;
> +}
> +
> +static void clone_unshare_test(int use_clone, unsigned long clone_flags,
> +			       int (*fn1)(void *arg), void *arg1)
> +{
> +	int ret = 0;
> +
> +	switch (use_clone) {
> +	case T_NONE:
> +		ret = plain_test(fn1, arg1);
> +		break;
> +	case T_CLONE:
> +		ret = clone_test(clone_flags, fn1, arg1);
> +		break;
> +	case T_UNSHARE:
> +		ret = unshare_test(clone_flags, fn1, arg1);
> +		break;
> +	default:
> +		ret = -1;
> +		tst_brk(TBROK, "%s: bad use_clone option: %d", __FUNCTION__,
> +			use_clone);
> +		break;
> +	}
> +
> +	if (ret == -1)
> +		tst_brk(TBROK, "child2 clone failed");
> +}
> +
> +#endif
> diff --git a/testcases/kernel/containers/sysvipc/mesgq_nstest.c b/testcases/kernel/containers/sysvipc/mesgq_nstest.c
> index dbf311dc8..bb58b7211 100644
> --- a/testcases/kernel/containers/sysvipc/mesgq_nstest.c
> +++ b/testcases/kernel/containers/sysvipc/mesgq_nstest.c
> @@ -1,175 +1,122 @@
> -/* *************************************************************************
> -* Copyright (c) International Business Machines Corp., 2009
> -* This program is free software; you can redistribute it and/or modify
> -* it under the terms of the GNU General Public License as published by
> -* the Free Software Foundation; either version 2 of the License, or
> -* (at your option) any later version.
> -*
> -* This program is distributed in the hope that it will be useful,
> -* but WITHOUT ANY WARRANTY; without even the implied warranty of
> -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> -* the GNU General Public License for more details.
> -* You should have received a copy of the GNU General Public License
> -* along with this program; if not, write to the Free Software
> -* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> -*
> -* Author: Veerendra C <vechandr@in.ibm.com>
> -*
> -* In Parent Process , create mesgq with key 154326L
> -* Now create container by passing 1 of the flag values..
> -*	Flag = clone, clone(CLONE_NEWIPC), or unshare(CLONE_NEWIPC)
> -* In cloned process, try to access the created mesgq
> -* Test PASS: If the mesgq is readable when flag is None.
> -* Test FAIL: If the mesgq is readable when flag is Unshare or Clone.
> -***************************************************************************/
> -
> -#define _GNU_SOURCE 1
> -#include <stdio.h>
> -#include <stdlib.h>
> -#include <unistd.h>
> -#include <string.h>
> -#include <sys/ipc.h>
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) International Business Machines Corp., 2009
> + *				Veerendra C <vechandr@in.ibm.com>
> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Test SysV IPC message passing through different namespaces.
> + *
> + * [Algorithm]
> + *
> + * In parent process create a new mesgq with a specific key.
> + * In cloned process try to access the created mesgq.
> + *
> + * Test will PASS if the mesgq is readable when flag is None.
> + * Test will FAIL if the mesgq is readable when flag is Unshare or Clone or
> + * the message received is wrong.
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <sys/wait.h>
>  #include <sys/msg.h>
> -#include <libclone.h>
> -#include "test.h"
> -#include "ipcns_helper.h"
> -
> -#define KEY_VAL		154326L
> -#define UNSHARESTR	"unshare"
> -#define CLONESTR	"clone"
> -#define NONESTR		"none"
> -
> -char *TCID = "mesgq_nstest";
> -int TST_TOTAL = 1;
> -int p1[2];
> -int p2[2];
> -struct msg_buf {
> -	long int mtype;		/* type of received/sent message */
> -	char mtext[80];		/* text of the message */
> +#include <sys/types.h>
> +#include "tst_safe_sysv_ipc.h"
> +#include "tst_test.h"
> +#include "common.h"
> +
> +#define KEY_VAL 154326L
> +#define MSG_TYPE 5
> +#define MSG_TEXT "My message!"
> +
> +static char *str_op = "clone";
> +static int use_clone;
> +static int ipc_id = -1;
> +
> +static struct msg_buf {
> +	long mtype;
> +	char mtext[80];
>  } msg;
>  
> -void mesgq_read(int id)
> +static int check_mesgq(LTP_ATTRIBUTE_UNUSED void *vtest)
>  {
> -	int READMAX = 80;
> -	int n;
> -	/* read msg type 5 on the Q; msgtype, flags are last 2 params.. */
> +	int id, n;
>  
> -	n = msgrcv(id, &msg, READMAX, 5, 0);
> -	if (n == -1)
> -		perror("msgrcv"), tst_exit();
> -
> -	tst_resm(TINFO, "Mesg read of %d bytes; Type %ld: Msg: %.*s",
> -		 n, msg.mtype, n, msg.mtext);
> -}
> +	id = msgget(KEY_VAL, 0);
>  
> -int check_mesgq(void *vtest)
> -{
> -	char buf[3];
> -	int id;
> +	if (id < 0) {
> +		if (use_clone == T_NONE)
> +			tst_res(TFAIL, "Plain cloned process didn't find mesgq");
> +		else
> +			tst_res(TPASS, "%s: container didn't find mesgq", str_op);
> +	} else {
> +		if (use_clone == T_NONE)
> +			tst_res(TPASS, "Plain cloned process found mesgq inside container");
> +		else
> +			tst_res(TFAIL, "%s: container init process found mesgq", str_op);
>  
> -	(void) vtest;
> +		n = SAFE_MSGRCV(id, &msg, sizeof(msg.mtext), MSG_TYPE, 0);
>  
> -	close(p1[1]);
> -	close(p2[0]);
> +		tst_res(TINFO, "Mesg read of %d bytes, Type %ld, Msg: %s", n, msg.mtype, msg.mtext);
>  
> -	read(p1[0], buf, 3);
> -	id = msgget(KEY_VAL, 0);
> -	if (id == -1)
> -		write(p2[1], "notfnd", 7);
> -	else {
> -		write(p2[1], "exists", 7);
> -		mesgq_read(id);
> +		if (strcmp(msg.mtext, MSG_TEXT))
> +			tst_res(TFAIL, "Received the wrong text message");
>  	}
> -	tst_exit();
> -}
>  
> -static void setup(void)
> -{
> -	tst_require_root();
> -	check_newipc();
> +	TST_CHECKPOINT_WAKE(0);
> +
> +	return 0;
>  }
>  
> -int main(int argc, char *argv[])
> +static void run(void)
>  {
> -	int ret, use_clone = T_NONE, id, n;
> -	char *tsttype = NONESTR;
> -	char buf[7];
> +	msg.mtype = MSG_TYPE;
> +	strcpy(msg.mtext, "My message!");
>  
> -	setup();
> +	SAFE_MSGSND(ipc_id, &msg, strlen(msg.mtext), 0);

With large enough -i this will block sooner or late in the case of clone
or unshare because nobody reads the messages in this case. I guess that
the easiest solution would be:

	if (use_clone == T_NONE)
		SAFE_MSGSND(...);

> -	if (argc != 2) {
> -		tst_resm(TFAIL, "Usage: %s <clone|unshare|none>", argv[0]);
> -		tst_brkm(TFAIL, NULL, " where clone, unshare, or fork specifies"
> -			 " unshare method.");
> -	}
> +	tst_res(TINFO, "mesgq namespaces test: %s", str_op);
>  
> -	/* Using PIPE's to sync between container and Parent */
> -	if (pipe(p1) == -1) {
> -		perror("pipe");
> -		exit(EXIT_FAILURE);
> -	}
> -	if (pipe(p2) == -1) {
> -		perror("pipe");
> -		exit(EXIT_FAILURE);
> -	}
> +	clone_unshare_test(use_clone, CLONE_NEWIPC, check_mesgq, NULL);
>  
> -	tsttype = NONESTR;
> +	TST_CHECKPOINT_WAIT(0);

I guess that we can make even get rid of the checkpoints at this point
and do just tst_reap_children() here instead.

> +}
>  
> -	if (strcmp(argv[1], "clone") == 0) {
> -		use_clone = T_CLONE;
> -		tsttype = CLONESTR;
> -	} else if (strcmp(argv[1], "unshare") == 0) {
> -		use_clone = T_UNSHARE;
> -		tsttype = UNSHARESTR;
> -	}
> +static void setup(void)
> +{
> +	use_clone = get_clone_unshare_enum(str_op);
>  
> -	id = msgget(KEY_VAL, IPC_CREAT | IPC_EXCL | 0600);
> -	if (id == -1) {
> -		perror("msgget");
> -		/* Retry without attempting to create the MQ */
> -		id = msgget(KEY_VAL, 0);
> -		if (id == -1)
> -			perror("msgget failure"), exit(1);
> -	}
> +	if (use_clone != T_NONE)
> +		check_newipc();
>  
> -	msg.mtype = 5;
> -	strcpy(msg.mtext, "Message of type 5!");
> -	n = msgsnd(id, &msg, strlen(msg.mtext), 0);
> -	if (n == -1)
> -		perror("msgsnd"), tst_exit();
> -
> -	tst_resm(TINFO, "mesgq namespaces test : %s", tsttype);
> -	/* fire off the test */
> -	ret = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mesgq, NULL);
> -	if (ret < 0) {
> -		tst_brkm(TFAIL, NULL, "%s failed", tsttype);
> +	ipc_id = msgget(KEY_VAL, IPC_CREAT | IPC_EXCL | 0600);
> +	if (ipc_id < 0) {
> +		tst_res(TINFO, "Message already exists");
> +		ipc_id = SAFE_MSGGET(KEY_VAL, 0);
>  	}

I think that it's outright wrong to reuse a queue that does already
exist, we hardcode a key in the test which means that if we are unlucky
enough we will attempt to use a queue that is used by a different
processes. It would make more sense to exit the test loudly with an
error instead and let the user deal with the situation.

And technically it would be best to call the msgget() with IPC_PRIVATE
and then get the key by msgctl() IPC_STAT.

> +}
>  
> -	close(p1[0]);
> -	close(p2[1]);
> -	write(p1[1], "go", 3);
> -
> -	read(p2[0], buf, 7);
> -	if (strcmp(buf, "exists") == 0) {
> -		if (use_clone == T_NONE)
> -			tst_resm(TPASS, "Plain cloned process found mesgq "
> -				 "inside container");
> -		else
> -			tst_resm(TFAIL,
> -				 "%s: Container init process found mesgq",
> -				 tsttype);
> -	} else {
> -		if (use_clone == T_NONE)
> -			tst_resm(TFAIL,
> -				 "Plain cloned process didn't find mesgq");
> -		else
> -			tst_resm(TPASS, "%s: Container didn't find mesgq",
> -				 tsttype);
> +static void cleanup(void)
> +{
> +	if (ipc_id != -1) {
> +		tst_res(TINFO, "Destroy message");
> +		SAFE_MSGCTL(ipc_id, IPC_RMID, NULL);
>  	}
> -
> -	/* Delete the mesgQ */
> -	id = msgget(KEY_VAL, 0);
> -	msgctl(id, IPC_RMID, NULL);
> -
> -	tst_exit();
>  }
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.needs_checkpoints = 1,
> +	.options = (struct tst_option[]) {
> +		{ "m:", &str_op, "Test execution mode <clone|unshare|none>" },
> +		{},
> +	},
> +};
> -- 
> 2.35.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 02/10] Rewrite msg_comm.c using new LTP API
  2022-03-10 10:59 ` [LTP] [PATCH v2 02/10] Rewrite msg_comm.c " Andrea Cervesato
@ 2022-03-10 15:19   ` Cyril Hrubis
  0 siblings, 0 replies; 19+ messages in thread
From: Cyril Hrubis @ 2022-03-10 15:19 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> +	if (TST_RET > 0)
> +		tst_res(TFAIL, "SysV msg: communication with non-identical keys between namespaces");
> +	else
> +		tst_res(TPASS, "SysV msg: communication with identical keys between namespaces");

I would have changed these message to something more meaningful:

		tst_res(TFAIL, "messages leak between namespaces");
	else
		tst_res(TPASS, "messages does not leak between namespaces");


The rest looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 03/10] Rewrite sem_comm.c using new LTP API
  2022-03-10 10:59 ` [LTP] [PATCH v2 03/10] Rewrite sem_comm.c " Andrea Cervesato
@ 2022-03-10 15:43   ` Cyril Hrubis
  0 siblings, 0 replies; 19+ messages in thread
From: Cyril Hrubis @ 2022-03-10 15:43 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> +	TEST(semop(id, &sm, 1));
> +	if (TST_RET < 0) {
> +		if (TST_ERR != EAGAIN)
> +			tst_brk(TBROK | TERRNO, "semop error");
> +
> +		tst_res(TFAIL, "SysV sem: communication with identical keys between namespaces");
> +	} else {
> +		tst_res(TPASS, "SysV sem: communication with identical keys between namespaces");
>  	}

Same here, the messages can be better than that, otherwise looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 04/10] Rewrite sem_nstest.c using new LTP API
  2022-03-10 10:59 ` [LTP] [PATCH v2 04/10] Rewrite sem_nstest.c " Andrea Cervesato
@ 2022-03-10 15:54   ` Cyril Hrubis
  0 siblings, 0 replies; 19+ messages in thread
From: Cyril Hrubis @ 2022-03-10 15:54 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> +	ipc_id = semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666);
> +	if (ipc_id < 0) {
> +		tst_res(TINFO, "Semaphore already exists");
> +		ipc_id = SAFE_SEMGET(MY_KEY, 1, 0);
>  	}
> +}

Same here, I do not think that we should blindly reuse existing
semaphore.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 05/10] Rewrite semtest_2ns.c using new LTP API
  2022-03-10 10:59 ` [LTP] [PATCH v2 05/10] Rewrite semtest_2ns.c " Andrea Cervesato
@ 2022-03-10 16:37   ` Cyril Hrubis
  0 siblings, 0 replies; 19+ messages in thread
From: Cyril Hrubis @ 2022-03-10 16:37 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> +/*\
> + * [Description]
> + *
> + * Test SysV IPC semaphore usage between namespaces.
> + *
> + * [Algorithm]
> + *
> + * Create 2 'containers'
> + * In container1 create semaphore with a specific key
> + * In container2 try to access the semaphore created in container1
> + *
> + * Test is PASS if flag = none and the semaphore is accessible in container2 or
> + * if flag = unshare/clone and the semaphore is not accessible in container2.
> + * If semaphore is not accessible in container2, creates new semaphore with the
> + * same key to double check isloation in IPCNS.
> + *
> + * Test is FAIL if flag = none and the semaphore is not accessible, if
> + * flag = unshare/clone and semaphore is accessible in container2 or if the new
> + * semaphore creation Fails.
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <sys/wait.h>
> +#include <sys/msg.h>
> +#include <sys/types.h>
>  #include <sys/sem.h>
> -#include <libclone.h>
> -#include "test.h"
> -#include "ipcns_helper.h"
> -
> -#define MY_KEY     124326L
> -#define UNSHARESTR "unshare"
> -#define CLONESTR   "clone"
> -#define NONESTR    "none"
> -
> -char *TCID = "semtest_2ns";
> -int TST_TOTAL = 1;
> -int p1[2];
> -int p2[2];
> +#include "tst_safe_sysv_ipc.h"
> +#include "tst_test.h"
> +#include "common.h"
> +
> +#define MY_KEY 124326L
> +
> +static char *str_op = "clone";
> +static int use_clone;
> +
>  static struct sembuf semop_lock[2] = {
>  	/* sem_num, sem_op, flag */
> -	{0, 0, 0},		/* wait for sem#0 to become 0 */
> -	{0, 1, SEM_UNDO}	/* then increment sem#0 by 1 */
> +	{ 0, 0, 0 }, /* wait for sem#0 to become 0 */
> +	{ 0, 1, SEM_UNDO } /* then increment sem#0 by 1 */
>  };
>  
>  static struct sembuf semop_unlock[1] = {
>  	/* sem_num, sem_op, flag */
> -	{0, -1, (IPC_NOWAIT | SEM_UNDO)}	/* decrement sem#0 by 1 (sets it to 0) */
> +	{ 0, -1, (IPC_NOWAIT | SEM_UNDO) } /* decrement sem#0 by 1 (sets it to 0) */
>  };
>  
>  /*
>   * sem_lock() - Locks the semaphore for crit-sec updation, and unlocks it later
>   */
> -void sem_lock(int id)
> +static void sem_lock(int id)
>  {
> -	/* Checking the semlock and simulating as if the crit-sec is updated */
> -	if (semop(id, &semop_lock[0], 2) < 0) {
> -		perror("sem lock error");
> -		tst_brkm(TBROK, NULL, "semop failed");
> -	}
> -	tst_resm(TINFO, "Sem1: File locked, Critical section is updated...");
> +	SAFE_SEMOP(id, &semop_lock[0], 2);
> +
> +	tst_res(TINFO, "semaphore1: File locked, Critical section is updated...");
> +
>  	sleep(2);
> -	if (semop(id, &semop_unlock[0], 1) < 0) {
> -		perror("sem unlock error");
> -		tst_brkm(TBROK, NULL, "semop failed");
> -	}
> +
> +	SAFE_SEMOP(id, &semop_unlock[0], 1);
>  }
>  
>  /*
>   * check_sem1 -  does not read -- it writes to check_sem2() when it's done.
>   */
> -int check_sem1(void *vtest)
> +static int check_sem1(LTP_ATTRIBUTE_UNUSED void *vtest)
>  {
> -	int id1;
> -
> -	(void) vtest;
> -
> -	close(p1[0]);
> -	/* 1. Create (or fetch if existing) the binary semaphore */
> -	id1 = semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666);
> -	if (id1 == -1) {
> -		perror("Semaphore create");
> -		if (errno != EEXIST) {
> -			perror("semget failure");
> -			tst_brkm(TBROK, NULL, "semget failure");
> -		}
> -		id1 = semget(MY_KEY, 1, 0);
> -		if (id1 == -1) {
> -			perror("Semaphore create");
> -			tst_brkm(TBROK, NULL, "semget failure");
> -		}
> +	TEST(semget(MY_KEY, 1, IPC_CREAT | IPC_EXCL | 0666));
> +	if (TST_RET < 0) {
> +		tst_res(TINFO, "semget failure. Checking existing semaphore");
> +
> +		if (TST_ERR != EEXIST)
> +			tst_brk(TBROK | TRERRNO, "Semaphore creation failed");
> +
> +		SAFE_SEMGET(MY_KEY, 1, 0);
>  	}
>  
> -	write(p1[1], "go", 3);
> -	tst_resm(TINFO, "Cont1: Able to create semaphore");
> -	tst_exit();
> +	tst_res(TINFO, "container1: Able to create semaphore");
> +
> +	return 0;
>  }
>  
>  /*
>   * check_sem2() reads from check_sem1() and writes to main() when it's done.
>   */
> -
> -int check_sem2(void *vtest)
> +static int check_sem2(LTP_ATTRIBUTE_UNUSED void *vtest)
>  {
> -	char buf[3];
> -	int id2;
> -
> -	(void) vtest;
> +	int id;
>  
> -	close(p1[1]);
> -	close(p2[0]);
> -	read(p1[0], buf, 3);
> +	id = semget(MY_KEY, 1, 0);
> +	if (id >= 0) {
> +		sem_lock(id);

I wonder why we even call the sem_lock() here, it does not add any
value to the test and only slows it down for two seconds for no reason
in the case of -m none.


Looking at the code the whole test looks strange and does not make much
sense.

Looking at the check_sem1() I guess that it's supposed to create a
semaphore in one namespace, while the check_sem2() is trying to check if
that leaks into the second namespace. But without the checkpoint
synchronization the order is not guaranteed at all.

I guess that this should really be rewritten so that:

* check_sem1() creates the semaphore, then calls CHECKPOINT_WAKE()

* check_sem2() does CHEKPOINT_WAIT() then check if the semaphore does
  exists

That is the very basic test we should do.

On the top of that we can make the check_sem1() to also lock the
semaphore between the call to wake the other process. And the other
process check_sem2() can attempt to lock it as well with IPC_NOWAIT. If
that fails the reference to the semaphore somehow leaked even if the
namespaces are isolated.


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 06/10] Rewrite shm_comm.c using new LTP API
  2022-03-10 10:59 ` [LTP] [PATCH v2 06/10] Rewrite shm_comm.c " Andrea Cervesato
@ 2022-03-10 17:01   ` Cyril Hrubis
  0 siblings, 0 replies; 19+ messages in thread
From: Cyril Hrubis @ 2022-03-10 17:01 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> +		tst_res(TFAIL, "SysV shm: communication with non-identical keys between namespaces");
> +	else
> +		tst_res(TPASS, "SysV shm: communication with identical keys between namespaces");

Again these messages could be better, other than that the code looks
good.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c using new LTP API
  2022-03-10 14:16   ` Cyril Hrubis
@ 2022-03-14 12:23     ` Andrea Cervesato via ltp
  2022-03-15 12:38       ` Cyril Hrubis
  0 siblings, 1 reply; 19+ messages in thread
From: Andrea Cervesato via ltp @ 2022-03-14 12:23 UTC (permalink / raw)
  To: Cyril Hrubis, Andrea Cervesato; +Cc: ltp


[-- Attachment #1.1: Type: text/plain, Size: 14519 bytes --]

Hi!

On 3/10/22 15:16, Cyril Hrubis wrote:
> Hi!
>> ---
>>   testcases/kernel/containers/sysvipc/common.h  | 157 +++++++++++
>>   .../kernel/containers/sysvipc/mesgq_nstest.c  | 247 +++++++-----------
>>   2 files changed, 254 insertions(+), 150 deletions(-)
>>   create mode 100644 testcases/kernel/containers/sysvipc/common.h
>>
>> diff --git a/testcases/kernel/containers/sysvipc/common.h b/testcases/kernel/containers/sysvipc/common.h
>> new file mode 100644
>> index 000000000..1fea6fafa
>> --- /dev/null
>> +++ b/testcases/kernel/containers/sysvipc/common.h
>> @@ -0,0 +1,157 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) International Business Machines Corp., 2007
>> + *               Rishikesh K Rajak<risrajak@in.ibm.com>
>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato<andrea.cervesato@suse.com>
>> + */
>> +
>> +#ifndef COMMON_H
>> +#define COMMON_H
>> +
>> +#include <stdlib.h>
>> +#include "tst_test.h"
>> +#include "lapi/syscalls.h"
>> +#include "lapi/namespaces_constants.h"
>> +
>> +enum {
>> +	T_CLONE,
>> +	T_UNSHARE,
>> +	T_NONE,
>> +};
>> +
>> +static int dummy_child(void *v)
>> +{
>> +	(void)v;
>> +	return 0;
>> +}
>> +
>> +static void check_newipc(void)
>> +{
>> +	int pid, status;
>> +
>> +	if (tst_kvercmp(2, 6, 19) < 0)
>> +		tst_brk(TCONF, "CLONE_NEWIPC not supported");
> 2.6.19 was released in 2006 it should be safe enough to drop this check
> now
>
>> +	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child, NULL);
>> +	if (pid < 0)
>> +		tst_brk(TCONF | TERRNO, "CLONE_NEWIPC not supported");
>> +
>> +	SAFE_WAITPID(pid, &status, 0);
>> +}
>> +
>> +static inline int get_clone_unshare_enum(const char* str_op)
>> +{
>> +	int use_clone;
>> +
>> +	if (strcmp(str_op, "clone") &&
>> +		strcmp(str_op, "unshare") &&
>> +		strcmp(str_op, "none"))
>> +		tst_brk(TBROK, "Test execution mode <clone|unshare|none>");
>> +
>> +	if (!strcmp(str_op, "clone"))
>> +		use_clone = T_CLONE;
>> +	else if (!strcmp(str_op, "unshare"))
>> +		use_clone = T_UNSHARE;
>> +	else
>> +		use_clone = T_NONE;
> Why do we have to parse the string twice?
>
> Why not just:
>
> 	if (!strcmp(...)
> 		...
> 	else if (!strcmp(...))
> 		...
> 	else if (!strcmp(...))
> 		...
> 	else
> 		tst_brk(TBROK, "Invalid op '%s'", str_op);
>
>
>> +	return use_clone;
>> +}
>> +
>> +static int clone_test(unsigned long clone_flags, int (*fn1)(void *arg),
>> +		      void *arg1)
>> +{
>> +	int ret;
>> +
>> +	ret = ltp_clone_quick(clone_flags | SIGCHLD, fn1, arg1);
>> +
>> +	if (ret != -1) {
>> +		/* no errors: we'll get the PID id that means success */
>> +		ret = 0;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int unshare_test(unsigned long clone_flags, int (*fn1)(void *arg),
>> +			void *arg1)
>> +{
>> +	int pid, ret = 0;
>> +	int retpipe[2];
>> +	char buf[2];
>> +
>> +	SAFE_PIPE(retpipe);
>> +
>> +	pid = fork();
>> +	if (pid < 0) {
>> +		SAFE_CLOSE(retpipe[0]);
>> +		SAFE_CLOSE(retpipe[1]);
>> +		tst_brk(TBROK, "fork");
>> +	}
>> +
>> +	if (!pid) {
>> +		SAFE_CLOSE(retpipe[0]);
>> +
>> +		ret = tst_syscall(SYS_unshare, clone_flags);
>> +		if (ret == -1) {
>> +			SAFE_WRITE(1, retpipe[1], "0", 2);
>> +			SAFE_CLOSE(retpipe[1]);
>> +			exit(1);
>> +		} else {
>> +			SAFE_WRITE(1, retpipe[1], "1", 2);
>> +		}
>> +
>> +		SAFE_CLOSE(retpipe[1]);
>> +
>> +		ret = fn1(arg1);
>> +		exit(ret);
>> +	}
>> +
>> +	SAFE_CLOSE(retpipe[1]);
>> +	SAFE_READ(1, retpipe[0], &buf, 2);
>> +	SAFE_CLOSE(retpipe[0]);
>> +
>> +	if (*buf == '0')
>> +		return -1;
>> +
>> +	return ret;
> Can we please clean up this mess as well? We can easily use
> SAFE_MACROS() in the new library and we do not need to propagate errors
> via pipes anymore. So this should really be just:
>
> static void unshare_test(unsigned long clone_flags, int (*fn1)(void *arg),
>                          void *arg1)
> {
> 	int pid;
>
> 	pid = SAFE_FORK();
>
> 	if (!pid) {
> 		int ret;
>
> 		SAFE_UNSHARE(clone_flags);
>
> 		ret = fn1(arg);
> 		exit(ret);
> 	}
> }
>
> And once all tests are converted we can even drop some more of the code
> since the fn1() can be declared to return void as well.
>
>> +}
>> +
>> +static int plain_test(int (*fn1)(void *arg), void *arg1)
>> +{
>> +	int ret = 0, pid;
>> +
>> +	pid = SAFE_FORK();
>> +	if (!pid)
>> +		exit(fn1(arg1));
>> +
>> +	return ret;
>> +}
>> +
>> +static void clone_unshare_test(int use_clone, unsigned long clone_flags,
>> +			       int (*fn1)(void *arg), void *arg1)
>> +{
>> +	int ret = 0;
>> +
>> +	switch (use_clone) {
>> +	case T_NONE:
>> +		ret = plain_test(fn1, arg1);
>> +		break;
>> +	case T_CLONE:
>> +		ret = clone_test(clone_flags, fn1, arg1);
>> +		break;
>> +	case T_UNSHARE:
>> +		ret = unshare_test(clone_flags, fn1, arg1);
>> +		break;
>> +	default:
>> +		ret = -1;
>> +		tst_brk(TBROK, "%s: bad use_clone option: %d", __FUNCTION__,
>> +			use_clone);
>> +		break;
>> +	}
>> +
>> +	if (ret == -1)
>> +		tst_brk(TBROK, "child2 clone failed");
>> +}
>> +
>> +#endif
>> diff --git a/testcases/kernel/containers/sysvipc/mesgq_nstest.c b/testcases/kernel/containers/sysvipc/mesgq_nstest.c
>> index dbf311dc8..bb58b7211 100644
>> --- a/testcases/kernel/containers/sysvipc/mesgq_nstest.c
>> +++ b/testcases/kernel/containers/sysvipc/mesgq_nstest.c
>> @@ -1,175 +1,122 @@
>> -/* *************************************************************************
>> -* Copyright (c) International Business Machines Corp., 2009
>> -* This program is free software; you can redistribute it and/or modify
>> -* it under the terms of the GNU General Public License as published by
>> -* the Free Software Foundation; either version 2 of the License, or
>> -* (at your option) any later version.
>> -*
>> -* This program is distributed in the hope that it will be useful,
>> -* but WITHOUT ANY WARRANTY; without even the implied warranty of
>> -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
>> -* the GNU General Public License for more details.
>> -* You should have received a copy of the GNU General Public License
>> -* along with this program; if not, write to the Free Software
>> -* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> -*
>> -* Author: Veerendra C<vechandr@in.ibm.com>
>> -*
>> -* In Parent Process , create mesgq with key 154326L
>> -* Now create container by passing 1 of the flag values..
>> -*	Flag = clone, clone(CLONE_NEWIPC), or unshare(CLONE_NEWIPC)
>> -* In cloned process, try to access the created mesgq
>> -* Test PASS: If the mesgq is readable when flag is None.
>> -* Test FAIL: If the mesgq is readable when flag is Unshare or Clone.
>> -***************************************************************************/
>> -
>> -#define _GNU_SOURCE 1
>> -#include <stdio.h>
>> -#include <stdlib.h>
>> -#include <unistd.h>
>> -#include <string.h>
>> -#include <sys/ipc.h>
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) International Business Machines Corp., 2009
>> + *				Veerendra C<vechandr@in.ibm.com>
>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato<andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Test SysV IPC message passing through different namespaces.
>> + *
>> + * [Algorithm]
>> + *
>> + * In parent process create a new mesgq with a specific key.
>> + * In cloned process try to access the created mesgq.
>> + *
>> + * Test will PASS if the mesgq is readable when flag is None.
>> + * Test will FAIL if the mesgq is readable when flag is Unshare or Clone or
>> + * the message received is wrong.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +
>> +#include <sys/wait.h>
>>   #include <sys/msg.h>
>> -#include <libclone.h>
>> -#include "test.h"
>> -#include "ipcns_helper.h"
>> -
>> -#define KEY_VAL		154326L
>> -#define UNSHARESTR	"unshare"
>> -#define CLONESTR	"clone"
>> -#define NONESTR		"none"
>> -
>> -char *TCID = "mesgq_nstest";
>> -int TST_TOTAL = 1;
>> -int p1[2];
>> -int p2[2];
>> -struct msg_buf {
>> -	long int mtype;		/* type of received/sent message */
>> -	char mtext[80];		/* text of the message */
>> +#include <sys/types.h>
>> +#include "tst_safe_sysv_ipc.h"
>> +#include "tst_test.h"
>> +#include "common.h"
>> +
>> +#define KEY_VAL 154326L
>> +#define MSG_TYPE 5
>> +#define MSG_TEXT "My message!"
>> +
>> +static char *str_op = "clone";
>> +static int use_clone;
>> +static int ipc_id = -1;
>> +
>> +static struct msg_buf {
>> +	long mtype;
>> +	char mtext[80];
>>   } msg;
>>   
>> -void mesgq_read(int id)
>> +static int check_mesgq(LTP_ATTRIBUTE_UNUSED void *vtest)
>>   {
>> -	int READMAX = 80;
>> -	int n;
>> -	/* read msg type 5 on the Q; msgtype, flags are last 2 params.. */
>> +	int id, n;
>>   
>> -	n = msgrcv(id, &msg, READMAX, 5, 0);
>> -	if (n == -1)
>> -		perror("msgrcv"), tst_exit();
>> -
>> -	tst_resm(TINFO, "Mesg read of %d bytes; Type %ld: Msg: %.*s",
>> -		 n, msg.mtype, n, msg.mtext);
>> -}
>> +	id = msgget(KEY_VAL, 0);
>>   
>> -int check_mesgq(void *vtest)
>> -{
>> -	char buf[3];
>> -	int id;
>> +	if (id < 0) {
>> +		if (use_clone == T_NONE)
>> +			tst_res(TFAIL, "Plain cloned process didn't find mesgq");
>> +		else
>> +			tst_res(TPASS, "%s: container didn't find mesgq", str_op);
>> +	} else {
>> +		if (use_clone == T_NONE)
>> +			tst_res(TPASS, "Plain cloned process found mesgq inside container");
>> +		else
>> +			tst_res(TFAIL, "%s: container init process found mesgq", str_op);
>>   
>> -	(void) vtest;
>> +		n = SAFE_MSGRCV(id, &msg, sizeof(msg.mtext), MSG_TYPE, 0);
>>   
>> -	close(p1[1]);
>> -	close(p2[0]);
>> +		tst_res(TINFO, "Mesg read of %d bytes, Type %ld, Msg: %s", n, msg.mtype, msg.mtext);
>>   
>> -	read(p1[0], buf, 3);
>> -	id = msgget(KEY_VAL, 0);
>> -	if (id == -1)
>> -		write(p2[1], "notfnd", 7);
>> -	else {
>> -		write(p2[1], "exists", 7);
>> -		mesgq_read(id);
>> +		if (strcmp(msg.mtext, MSG_TEXT))
>> +			tst_res(TFAIL, "Received the wrong text message");
>>   	}
>> -	tst_exit();
>> -}
>>   
>> -static void setup(void)
>> -{
>> -	tst_require_root();
>> -	check_newipc();
>> +	TST_CHECKPOINT_WAKE(0);
>> +
>> +	return 0;
>>   }
>>   
>> -int main(int argc, char *argv[])
>> +static void run(void)
>>   {
>> -	int ret, use_clone = T_NONE, id, n;
>> -	char *tsttype = NONESTR;
>> -	char buf[7];
>> +	msg.mtype = MSG_TYPE;
>> +	strcpy(msg.mtext, "My message!");
>>   
>> -	setup();
>> +	SAFE_MSGSND(ipc_id, &msg, strlen(msg.mtext), 0);
> With large enough -i this will block sooner or late in the case of clone
> or unshare because nobody reads the messages in this case. I guess that
> the easiest solution would be:
>
> 	if (use_clone == T_NONE)
> 		SAFE_MSGSND(...);
>
>> -	if (argc != 2) {
>> -		tst_resm(TFAIL, "Usage: %s <clone|unshare|none>", argv[0]);
>> -		tst_brkm(TFAIL, NULL, " where clone, unshare, or fork specifies"
>> -			 " unshare method.");
>> -	}
>> +	tst_res(TINFO, "mesgq namespaces test: %s", str_op);
>>   
>> -	/* Using PIPE's to sync between container and Parent */
>> -	if (pipe(p1) == -1) {
>> -		perror("pipe");
>> -		exit(EXIT_FAILURE);
>> -	}
>> -	if (pipe(p2) == -1) {
>> -		perror("pipe");
>> -		exit(EXIT_FAILURE);
>> -	}
>> +	clone_unshare_test(use_clone, CLONE_NEWIPC, check_mesgq, NULL);
>>   
>> -	tsttype = NONESTR;
>> +	TST_CHECKPOINT_WAIT(0);
> I guess that we can make even get rid of the checkpoints at this point
> and do just tst_reap_children() here instead.
Yes, it was garbage code I let it there without reason. I usually get 
rid of tst_reap_children() because it's already called by default
>> +}
>>   
>> -	if (strcmp(argv[1], "clone") == 0) {
>> -		use_clone = T_CLONE;
>> -		tsttype = CLONESTR;
>> -	} else if (strcmp(argv[1], "unshare") == 0) {
>> -		use_clone = T_UNSHARE;
>> -		tsttype = UNSHARESTR;
>> -	}
>> +static void setup(void)
>> +{
>> +	use_clone = get_clone_unshare_enum(str_op);
>>   
>> -	id = msgget(KEY_VAL, IPC_CREAT | IPC_EXCL | 0600);
>> -	if (id == -1) {
>> -		perror("msgget");
>> -		/* Retry without attempting to create the MQ */
>> -		id = msgget(KEY_VAL, 0);
>> -		if (id == -1)
>> -			perror("msgget failure"), exit(1);
>> -	}
>> +	if (use_clone != T_NONE)
>> +		check_newipc();
>>   
>> -	msg.mtype = 5;
>> -	strcpy(msg.mtext, "Message of type 5!");
>> -	n = msgsnd(id, &msg, strlen(msg.mtext), 0);
>> -	if (n == -1)
>> -		perror("msgsnd"), tst_exit();
>> -
>> -	tst_resm(TINFO, "mesgq namespaces test : %s", tsttype);
>> -	/* fire off the test */
>> -	ret = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mesgq, NULL);
>> -	if (ret < 0) {
>> -		tst_brkm(TFAIL, NULL, "%s failed", tsttype);
>> +	ipc_id = msgget(KEY_VAL, IPC_CREAT | IPC_EXCL | 0600);
>> +	if (ipc_id < 0) {
>> +		tst_res(TINFO, "Message already exists");
>> +		ipc_id = SAFE_MSGGET(KEY_VAL, 0);
>>   	}
> I think that it's outright wrong to reuse a queue that does already
> exist, we hardcode a key in the test which means that if we are unlucky
> enough we will attempt to use a queue that is used by a different
> processes. It would make more sense to exit the test loudly with an
> error instead and let the user deal with the situation.
>
> And technically it would be best to call the msgget() with IPC_PRIVATE
> and then get the key by msgctl() IPC_STAT.
According with documentation and examples, msgget() is mostly used with 
IPC_CREAT . What's the advantage of using IPC_PRIVATE in this case?
>> +}
>>   
>> -	close(p1[0]);
>> -	close(p2[1]);
>> -	write(p1[1], "go", 3);
>> -
>> -	read(p2[0], buf, 7);
>> -	if (strcmp(buf, "exists") == 0) {
>> -		if (use_clone == T_NONE)
>> -			tst_resm(TPASS, "Plain cloned process found mesgq "
>> -				 "inside container");
>> -		else
>> -			tst_resm(TFAIL,
>> -				 "%s: Container init process found mesgq",
>> -				 tsttype);
>> -	} else {
>> -		if (use_clone == T_NONE)
>> -			tst_resm(TFAIL,
>> -				 "Plain cloned process didn't find mesgq");
>> -		else
>> -			tst_resm(TPASS, "%s: Container didn't find mesgq",
>> -				 tsttype);
>> +static void cleanup(void)
>> +{
>> +	if (ipc_id != -1) {
>> +		tst_res(TINFO, "Destroy message");
>> +		SAFE_MSGCTL(ipc_id, IPC_RMID, NULL);
>>   	}
>> -
>> -	/* Delete the mesgQ */
>> -	id = msgget(KEY_VAL, 0);
>> -	msgctl(id, IPC_RMID, NULL);
>> -
>> -	tst_exit();
>>   }
>> +
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.needs_checkpoints = 1,
>> +	.options = (struct tst_option[]) {
>> +		{ "m:", &str_op, "Test execution mode <clone|unshare|none>" },
>> +		{},
>> +	},
>> +};
>> -- 
>> 2.35.1
>>
>>
>> -- 
>> Mailing list info:https://lists.linux.it/listinfo/ltp

[-- Attachment #1.2: Type: text/html, Size: 15976 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c using new LTP API
  2022-03-14 12:23     ` Andrea Cervesato via ltp
@ 2022-03-15 12:38       ` Cyril Hrubis
  0 siblings, 0 replies; 19+ messages in thread
From: Cyril Hrubis @ 2022-03-15 12:38 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> > And technically it would be best to call the msgget() with IPC_PRIVATE
> > and then get the key by msgctl() IPC_STAT.
> According with documentation and examples, msgget() is mostly used with 
> IPC_CREAT . What's the advantage of using IPC_PRIVATE in this case?

If you pass IPC_PRIVATE instead of the hardcoded key kernel will pick an
unused key for you, that's all.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-03-15 12:38 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10 10:59 [LTP] [PATCH v2 00/10] Rewrite sysvipc testing suite using new LTP API Andrea Cervesato
2022-03-10 10:59 ` [LTP] [PATCH v2 01/10] Rewrite mesgq_nstest.c " Andrea Cervesato
2022-03-10 14:16   ` Cyril Hrubis
2022-03-14 12:23     ` Andrea Cervesato via ltp
2022-03-15 12:38       ` Cyril Hrubis
2022-03-10 10:59 ` [LTP] [PATCH v2 02/10] Rewrite msg_comm.c " Andrea Cervesato
2022-03-10 15:19   ` Cyril Hrubis
2022-03-10 10:59 ` [LTP] [PATCH v2 03/10] Rewrite sem_comm.c " Andrea Cervesato
2022-03-10 15:43   ` Cyril Hrubis
2022-03-10 10:59 ` [LTP] [PATCH v2 04/10] Rewrite sem_nstest.c " Andrea Cervesato
2022-03-10 15:54   ` Cyril Hrubis
2022-03-10 10:59 ` [LTP] [PATCH v2 05/10] Rewrite semtest_2ns.c " Andrea Cervesato
2022-03-10 16:37   ` Cyril Hrubis
2022-03-10 10:59 ` [LTP] [PATCH v2 06/10] Rewrite shm_comm.c " Andrea Cervesato
2022-03-10 17:01   ` Cyril Hrubis
2022-03-10 10:59 ` [LTP] [PATCH v2 07/10] Rewrite shmem_2nstest.c " Andrea Cervesato
2022-03-10 10:59 ` [LTP] [PATCH v2 08/10] Rewrite shmnstest.c " Andrea Cervesato
2022-03-10 10:59 ` [LTP] [PATCH v2 09/10] Remove libclone dependency from sysvipc Andrea Cervesato
2022-03-10 10:59 ` [LTP] [PATCH v2 10/10] Delete obsolete ipcns_helper.h in sysvipc suite Andrea Cervesato

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.