<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><font size="4">Hi!</font><br>
</p>
<div class="moz-cite-prefix">On 3/10/22 15:16, Cyril Hrubis wrote:<br>
</div>
<blockquote type="cite" cite="mid:YioISjHts5dstfLM@yuki">
<pre class="moz-quote-pre" wrap="">Hi!
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">---
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 <a class="moz-txt-link-rfc2396E" href="mailto:risrajak@in.ibm.com"><risrajak@in.ibm.com></a>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <a class="moz-txt-link-rfc2396E" href="mailto:andrea.cervesato@suse.com"><andrea.cervesato@suse.com></a>
+ */
+
+#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");
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
2.6.19 was released in 2006 it should be safe enough to drop this check
now
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ 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;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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);
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ 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;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+}
+
+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 <a class="moz-txt-link-rfc2396E" href="mailto:vechandr@in.ibm.com"><vechandr@in.ibm.com></a>
-*
-* 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 <a class="moz-txt-link-rfc2396E" href="mailto:vechandr@in.ibm.com"><vechandr@in.ibm.com></a>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <a class="moz-txt-link-rfc2396E" href="mailto:andrea.cervesato@suse.com"><andrea.cervesato@suse.com></a>
+ */
+
+/*\
+ * [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);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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(...);
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">- 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);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I guess that we can make even get rid of the checkpoints at this point
and do just tst_reap_children() here instead.
</pre>
</blockquote>
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
<blockquote type="cite" cite="mid:YioISjHts5dstfLM@yuki">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+}
- 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);
}
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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.
</pre>
</blockquote>
According with documentation and examples, msgget() is mostly used
with IPC_CREAT . What's the advantage of using IPC_PRIVATE in this
case?<br>
<blockquote type="cite" cite="mid:YioISjHts5dstfLM@yuki">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+}
- 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: <a class="moz-txt-link-freetext" href="https://lists.linux.it/listinfo/ltp">https://lists.linux.it/listinfo/ltp</a>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>