* [LTP] [PATCH v3 1/3] Add more mqueue safe macros
2022-08-16 11:31 [LTP] [PATCH v3 0/3] Refactor pidns30 and pidns31 using new LTP API Andrea Cervesato via ltp
@ 2022-08-16 11:31 ` Andrea Cervesato via ltp
2022-11-01 11:48 ` Richard Palethorpe
2022-08-16 11:31 ` [LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API Andrea Cervesato via ltp
2022-08-16 11:31 ` [LTP] [PATCH v3 3/3] Refactor pidns31 " Andrea Cervesato via ltp
2 siblings, 1 reply; 9+ messages in thread
From: Andrea Cervesato via ltp @ 2022-08-16 11:31 UTC (permalink / raw)
To: ltp
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
include/tst_safe_posix_ipc.h | 73 ++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/include/tst_safe_posix_ipc.h b/include/tst_safe_posix_ipc.h
index b60c12c9e..78ce660b5 100644
--- a/include/tst_safe_posix_ipc.h
+++ b/include/tst_safe_posix_ipc.h
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* Copyright (C) 2017-2019 Petr Vorel pvorel@suse.cz
+ * Copyright (C) 2022 Andrea Cervesato andrea.cervesato@suse.com
*/
#ifndef TST_SAFE_POSIX_IPC_H__
@@ -12,6 +13,18 @@
#define SAFE_MQ_OPEN(pathname, oflags, ...) \
safe_mq_open(__FILE__, __LINE__, (pathname), (oflags), ##__VA_ARGS__)
+#define SAFE_MQ_NOTIFY(mqdes, sevp) \
+ safe_mq_notify(__FILE__, __LINE__, (mqdes), (sevp))
+
+#define SAFE_MQ_SEND(mqdes, msg_ptr, msg_len, msg_prio) \
+ safe_mq_send(__FILE__, __LINE__, (mqdes), (msg_ptr), (msg_len), (msg_prio))
+
+#define SAFE_MQ_CLOSE(mqdes) \
+ safe_mq_close(__FILE__, __LINE__, (mqdes))
+
+#define SAFE_MQ_UNLINK(name) \
+ safe_mq_unlink(__FILE__, __LINE__, (name))
+
static inline int safe_mq_open(const char *file, const int lineno,
const char *pathname, int oflags, ...)
{
@@ -46,4 +59,64 @@ static inline int safe_mq_open(const char *file, const int lineno,
return rval;
}
+static int safe_mq_notify(const char *file, const int lineno,
+ mqd_t mqdes, const struct sigevent *sevp)
+{
+ int rval;
+
+ rval = mq_notify(mqdes, sevp);
+
+ if (rval == -1)
+ tst_brk_(file, lineno, TBROK | TERRNO, "mq_notify() failed");
+
+ return rval;
+}
+
+static int safe_mq_send(const char *file, const int lineno,
+ mqd_t mqdes, const char *msg_ptr,
+ size_t msg_len, unsigned int msg_prio)
+{
+ int rval;
+
+ rval = mq_send(mqdes, msg_ptr, msg_len, msg_prio);
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "mq_send(%d,%s,%lu,%d) failed", mqdes, msg_ptr,
+ msg_len, msg_prio);
+ }
+
+ return rval;
+}
+
+static int safe_mq_close(const char *file, const int lineno,
+ mqd_t mqdes)
+{
+ int rval;
+
+ rval = mq_close(mqdes);
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "mq_close(%d) failed", mqdes);
+ }
+
+ return rval;
+}
+
+static int safe_mq_unlink(const char *file, const int lineno,
+ const char *name)
+{
+ int rval;
+
+ rval = mq_unlink(name);
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "mq_unlink(%s) failed", name);
+ }
+
+ return rval;
+}
+
#endif /* TST_SAFE_POSIX_IPC_H__ */
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH v3 1/3] Add more mqueue safe macros
2022-08-16 11:31 ` [LTP] [PATCH v3 1/3] Add more mqueue safe macros Andrea Cervesato via ltp
@ 2022-11-01 11:48 ` Richard Palethorpe
0 siblings, 0 replies; 9+ messages in thread
From: Richard Palethorpe @ 2022-11-01 11:48 UTC (permalink / raw)
To: Andrea Cervesato; +Cc: ltp
Hello,
Ah, I see you had two similar patches on the queue and merged the one
with less functions added.
Anyway, now merged this as well with inline added and trailing
whitespace removed.
Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> include/tst_safe_posix_ipc.h | 73 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
> diff --git a/include/tst_safe_posix_ipc.h b/include/tst_safe_posix_ipc.h
> index b60c12c9e..78ce660b5 100644
> --- a/include/tst_safe_posix_ipc.h
> +++ b/include/tst_safe_posix_ipc.h
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> * Copyright (C) 2017-2019 Petr Vorel pvorel@suse.cz
> + * Copyright (C) 2022 Andrea Cervesato andrea.cervesato@suse.com
> */
>
> #ifndef TST_SAFE_POSIX_IPC_H__
> @@ -12,6 +13,18 @@
> #define SAFE_MQ_OPEN(pathname, oflags, ...) \
> safe_mq_open(__FILE__, __LINE__, (pathname), (oflags), ##__VA_ARGS__)
>
> +#define SAFE_MQ_NOTIFY(mqdes, sevp) \
> + safe_mq_notify(__FILE__, __LINE__, (mqdes), (sevp))
> +
> +#define SAFE_MQ_SEND(mqdes, msg_ptr, msg_len, msg_prio) \
> + safe_mq_send(__FILE__, __LINE__, (mqdes), (msg_ptr), (msg_len), (msg_prio))
> +
> +#define SAFE_MQ_CLOSE(mqdes) \
> + safe_mq_close(__FILE__, __LINE__, (mqdes))
> +
> +#define SAFE_MQ_UNLINK(name) \
> + safe_mq_unlink(__FILE__, __LINE__, (name))
> +
> static inline int safe_mq_open(const char *file, const int lineno,
> const char *pathname, int oflags, ...)
> {
> @@ -46,4 +59,64 @@ static inline int safe_mq_open(const char *file, const int lineno,
> return rval;
> }
>
> +static int safe_mq_notify(const char *file, const int lineno,
> + mqd_t mqdes, const struct sigevent *sevp)
> +{
> + int rval;
> +
> + rval = mq_notify(mqdes, sevp);
> +
> + if (rval == -1)
> + tst_brk_(file, lineno, TBROK | TERRNO, "mq_notify() failed");
> +
> + return rval;
> +}
> +
> +static int safe_mq_send(const char *file, const int lineno,
> + mqd_t mqdes, const char *msg_ptr,
> + size_t msg_len, unsigned int msg_prio)
> +{
> + int rval;
> +
> + rval = mq_send(mqdes, msg_ptr, msg_len, msg_prio);
> +
> + if (rval == -1) {
> + tst_brk_(file, lineno, TBROK | TERRNO,
> + "mq_send(%d,%s,%lu,%d) failed", mqdes, msg_ptr,
> + msg_len, msg_prio);
> + }
> +
> + return rval;
> +}
> +
> +static int safe_mq_close(const char *file, const int lineno,
> + mqd_t mqdes)
> +{
> + int rval;
> +
> + rval = mq_close(mqdes);
> +
> + if (rval == -1) {
> + tst_brk_(file, lineno, TBROK | TERRNO,
> + "mq_close(%d) failed", mqdes);
> + }
> +
> + return rval;
> +}
> +
> +static int safe_mq_unlink(const char *file, const int lineno,
> + const char *name)
> +{
> + int rval;
> +
> + rval = mq_unlink(name);
> +
> + if (rval == -1) {
> + tst_brk_(file, lineno, TBROK | TERRNO,
> + "mq_unlink(%s) failed", name);
> + }
> +
> + return rval;
> +}
> +
> #endif /* TST_SAFE_POSIX_IPC_H__ */
> --
> 2.35.3
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API
2022-08-16 11:31 [LTP] [PATCH v3 0/3] Refactor pidns30 and pidns31 using new LTP API Andrea Cervesato via ltp
2022-08-16 11:31 ` [LTP] [PATCH v3 1/3] Add more mqueue safe macros Andrea Cervesato via ltp
@ 2022-08-16 11:31 ` Andrea Cervesato via ltp
2022-11-01 12:01 ` Richard Palethorpe
2022-08-16 11:31 ` [LTP] [PATCH v3 3/3] Refactor pidns31 " Andrea Cervesato via ltp
2 siblings, 1 reply; 9+ messages in thread
From: Andrea Cervesato via ltp @ 2022-08-16 11:31 UTC (permalink / raw)
To: ltp
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
testcases/kernel/containers/pidns/pidns30.c | 312 +++++---------------
1 file changed, 66 insertions(+), 246 deletions(-)
diff --git a/testcases/kernel/containers/pidns/pidns30.c b/testcases/kernel/containers/pidns/pidns30.c
index c8b0806c0..b01759a87 100644
--- a/testcases/kernel/containers/pidns/pidns30.c
+++ b/testcases/kernel/containers/pidns/pidns30.c
@@ -1,296 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
/*
-* Copyright (c) Bull S.A.S. 2008
-* 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
-*
-***************************************************************************
-* File: pidns30.c
-*
-* Description:
-* This testcase checks if the si_pid is correctly set when a process
-* that has registered for notification on a posix mqueue is in a
-* descendant namespace wrt the process that sends a message to that posix
-* mqueue.
-*
-* Test Assertion & Strategy:
-* Parent Child
-* --------------------------------------------------------------------------
-* Create a POSIX mqueue.
-* Create a PID namespace container.
-* Open that mqueue for reading
-* Register for notification when a
-* message arrives in that mqueue
-* Install a handler for SIGUSR1.
-* Write something to the mqueue.
-* Inside the handler, check that
-* si_pid is set to 0
-*
-* Usage: <for command-line>
-* pidns30
-*
-* History:
-* DATE NAME DESCRIPTION
-* 01/12/08 Nadia Derbey Creation of this test.
-* <Nadia.Derbey@bull.net>
-*
-******************************************************************************/
+ * Copyright (c) Bull S.A.S. 2008
+ * 01/12/08 Nadia Derbey <Nadia.Derbey@bull.net>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Clone a process with CLONE_NEWPID flag, register notification on a posix
+ * mqueue and send a mqueue message from the parent. Then check if signal
+ * notification contains si_pid of the parent.
+ */
+
#define _GNU_SOURCE 1
-#include <sys/wait.h>
-#include <sys/types.h>
#include <signal.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <stdio.h>
#include <mqueue.h>
-#include "lapi/syscalls.h"
-#include "pidns_helper.h"
-#include "test.h"
-
-char *TCID = "pidns30";
-int TST_TOTAL = 1;
-
-char *mqname = "mq1";
-int result = TFAIL;
+#include "tst_test.h"
+#include "tst_safe_posix_ipc.h"
+#include "lapi/namespaces_constants.h"
-int father_to_child[2];
-int child_to_father[2];
+#define MQNAME "/LTP_PIDNS30_MQ"
-#define CHILD_PID 1
-#define PARENT_PID 0
-
-#define MSG "HOW ARE YOU"
-#define MSG_PRIO 1
-
-#define NO_STEP -1
-#define F_STEP_0 0x00
-#define F_STEP_1 0x01
-#define F_STEP_2 0x02
-#define F_STEP_3 0x03
-#define C_STEP_0 0x10
-#define C_STEP_1 0x11
-#define C_STEP_2 0x12
-
-mqd_t rc = -1;
-mqd_t mqd = -1;
-
-static void remove_pipe(int *fd)
-{
- close(fd[0]);
- close(fd[1]);
-}
+static mqd_t mqd = -1;
+static int received;
static void remove_mqueue(mqd_t mqd)
{
- mq_close(mqd);
- tst_syscall(__NR_mq_unlink, mqname);
-}
-
-static void cleanup(void)
-{
- if (mqd != -1) {
- remove_mqueue(mqd);
- }
- if (rc != -1) {
- remove_mqueue(rc);
- }
- remove_pipe(father_to_child);
- remove_pipe(child_to_father);
-}
+ if (mqd != -1)
+ SAFE_MQ_CLOSE(mqd);
-static void cleanup_child(void)
-{
- if (mqd != -1) {
- tst_syscall(__NR_mq_notify, mqd, NULL);
- }
- cleanup();
+ mq_unlink(MQNAME);
}
-/*
- * child_signal_handler() - to handle SIGUSR1
- *
- * XXX (garrcoop): add calls to cleanup_child() -- or should this be handled
- * from the libltp signal handler?
- */
-static void child_signal_handler(int sig, siginfo_t * si, void *unused)
+static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
{
- char buf[256];
- struct mq_attr attr;
+ tst_res(TINFO, "Received signal %s from pid %d", tst_strsig(si->si_signo), si->si_pid);
- if (si->si_signo != SIGUSR1) {
- printf("received signal = %d unexpectedly\n", si->si_signo);
+ if (si->si_signo != SIGUSR1 || si->si_code != SI_MESGQ || si->si_pid != 0)
return;
- }
-
- if (si->si_code != SI_MESGQ) {
- printf("expected signal code SI_MESGQ; got %d instead\n",
- si->si_code);
- return;
- }
-
- if (si->si_pid) {
- printf("expected signal originator PID = 0; got %d instead\n",
- si->si_pid);
- return;
- } else {
- printf("signal originator PID = 0\n");
- result = TPASS;
- }
- /*
- * Now read the message - Be silent on errors since this is not the
- * test purpose.
- */
- rc = mq_getattr(si->si_int, &attr);
- if (rc != -1)
- mq_receive(si->si_int, buf, attr.mq_msgsize, NULL);
+ received++;
}
-/*
- * child_fn() - Inside container
- *
- * XXX (garrcoop): add more calls to cleanup_child()?
- */
-int child_fn(void *arg)
+static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
{
- pid_t pid, ppid;
+ pid_t cpid, ppid;
struct sigaction sa;
struct sigevent notif;
- char buf[5];
+ mqd_t mqd_child;
- /* Set process id and parent pid */
- pid = getpid();
+ cpid = getpid();
ppid = getppid();
- if (pid != CHILD_PID || ppid != PARENT_PID) {
- printf("pidns was not created\n");
- return 1;
+ if (cpid != 1 || ppid != 0) {
+ tst_res(TFAIL, "got unexpected result of cpid=%d ppid=%d", cpid, ppid);
+ return 0;
}
- /* Close the appropriate end of each pipe */
- close(child_to_father[0]);
- close(father_to_child[1]);
+ TST_CHECKPOINT_WAIT(0);
- while (read(father_to_child[0], buf, 1) != 1)
- sleep(1);
-
- mqd = tst_syscall(__NR_mq_open, mqname, O_RDONLY, 0, NULL);
- if (mqd == -1) {
- perror("mq_open failed");
- return 1;
- } else
- printf("mq_open succeeded\n");
-
- /* Register for notification on message arrival */
+ mqd_child = SAFE_MQ_OPEN(MQNAME, O_RDONLY, 0, NULL);
notif.sigev_notify = SIGEV_SIGNAL;
notif.sigev_signo = SIGUSR1;
- notif.sigev_value.sival_int = mqd;
- if (tst_syscall(__NR_mq_notify, mqd, ¬if) == -1) {
- perror("mq_notify failed");
- return 1;
- } else
- printf("successfully registered for notification\n");
+ notif.sigev_value.sival_int = mqd_child;
+
+ SAFE_MQ_NOTIFY(mqd_child, ¬if);
- /* Define handler for SIGUSR1 */
sa.sa_flags = SA_SIGINFO;
- sigemptyset(&sa.sa_mask);
+ SAFE_SIGEMPTYSET(&sa.sa_mask);
sa.sa_sigaction = child_signal_handler;
- if (sigaction(SIGUSR1, &sa, NULL) == -1) {
- perror("sigaction failed");
- return 1;
- } else
- printf("successfully registered handler for SIGUSR1\n");
+ SAFE_SIGACTION(SIGUSR1, &sa, NULL);
- /* Ask parent to send a message to the mqueue */
- if (write(child_to_father[1], "c:ok", 5) != 5) {
- perror("write failed");
- return 1;
- }
-
- sleep(3);
+ TST_CHECKPOINT_WAKE_AND_WAIT(0);
- /* Has parent sent a message? */
- read(father_to_child[0], buf, 5);
- if (strcmp(buf, "f:ok") != 0) {
- printf("parent did not send the message!\n");
- return 1;
+ if (received != 1) {
+ tst_res(TFAIL, "Signal hasn't been received after mqueue event");
+ return 0;
}
- printf("parent is done - cleaning up\n");
- cleanup_child();
+ tst_res(TPASS, "Signal has been received after mqueue event");
- exit(0);
+ return 0;
}
-static void setup(void)
+static void cleanup(void)
{
- tst_require_root();
- check_newpid();
+ remove_mqueue(mqd);
}
-int main(void)
+static void run(void)
{
- int status;
- char buf[5];
- pid_t cpid;
-
- setup();
-
- if (pipe(child_to_father) == -1 || pipe(father_to_child) == -1) {
- tst_brkm(TBROK | TERRNO, cleanup, "pipe failed");
- }
+ int ret;
- tst_syscall(__NR_mq_unlink, mqname);
+ remove_mqueue(mqd);
- /* container creation on PID namespace */
- cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
- if (cpid == -1)
- tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
+ ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
+ if (ret < 0)
+ tst_brk(TBROK | TERRNO, "clone failed");
- mqd =
- tst_syscall(__NR_mq_open, mqname, O_RDWR | O_CREAT | O_EXCL, 0777,
- NULL);
- if (mqd == -1)
- tst_brkm(TBROK | TERRNO, cleanup, "mq_open failed");
- else
- tst_resm(TINFO, "successfully created posix mqueue");
+ mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, 0);
- if (write(father_to_child[1], buf, 1) != 1)
- tst_brkm(TBROK | TERRNO, cleanup, "write failed");
+ TST_CHECKPOINT_WAKE_AND_WAIT(0);
- /* Close the appropriate end of each pipe */
- close(child_to_father[1]);
- close(father_to_child[0]);
+ SAFE_MQ_SEND(mqd, "pippo", 5, 1);
- /* Is container ready */
- read(child_to_father[0], buf, 5);
- if (strcmp(buf, "c:ok") != 0)
- tst_brkm(TBROK, cleanup,
- "container did not respond as expected!");
-
- rc = mq_send(mqd, MSG, strlen(MSG), MSG_PRIO);
- if (rc == -1)
- tst_brkm(TBROK | TERRNO, cleanup, "mq_send failed");
- else
- tst_resm(TINFO, "mq_send succeeded");
-
- /* Tell the child the message has been sent */
- if (write(father_to_child[1], "f:ok", 5) != 5)
- tst_brkm(TBROK | TERRNO, cleanup, "write failed");
-
- /* Wait for child to finish */
- if (wait(&status) == -1)
- tst_resm(TBROK | TERRNO, "wait failed");
-
- cleanup();
-
- tst_exit();
+ TST_CHECKPOINT_WAKE(0);
}
+
+static struct tst_test test = {
+ .test_all = run,
+ .cleanup = cleanup,
+ .needs_root = 1,
+ .needs_checkpoints = 1,
+};
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API
2022-08-16 11:31 ` [LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API Andrea Cervesato via ltp
@ 2022-11-01 12:01 ` Richard Palethorpe
2023-02-08 9:42 ` Andrea Cervesato via ltp
2023-02-08 9:51 ` Andrea Cervesato via ltp
0 siblings, 2 replies; 9+ messages in thread
From: Richard Palethorpe @ 2022-11-01 12:01 UTC (permalink / raw)
To: Andrea Cervesato; +Cc: ltp
Hello,
Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> testcases/kernel/containers/pidns/pidns30.c | 312 +++++---------------
> 1 file changed, 66 insertions(+), 246 deletions(-)
>
> diff --git a/testcases/kernel/containers/pidns/pidns30.c b/testcases/kernel/containers/pidns/pidns30.c
> index c8b0806c0..b01759a87 100644
> --- a/testcases/kernel/containers/pidns/pidns30.c
> +++ b/testcases/kernel/containers/pidns/pidns30.c
> @@ -1,296 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> -* Copyright (c) Bull S.A.S. 2008
> -* 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
> -*
> -***************************************************************************
> -* File: pidns30.c
> -*
> -* Description:
> -* This testcase checks if the si_pid is correctly set when a process
> -* that has registered for notification on a posix mqueue is in a
> -* descendant namespace wrt the process that sends a message to that posix
> -* mqueue.
> -*
> -* Test Assertion & Strategy:
> -* Parent Child
> -* --------------------------------------------------------------------------
> -* Create a POSIX mqueue.
> -* Create a PID namespace container.
> -* Open that mqueue for reading
> -* Register for notification when a
> -* message arrives in that mqueue
> -* Install a handler for SIGUSR1.
> -* Write something to the mqueue.
> -* Inside the handler, check that
> -* si_pid is set to 0
> -*
> -* Usage: <for command-line>
> -* pidns30
> -*
> -* History:
> -* DATE NAME DESCRIPTION
> -* 01/12/08 Nadia Derbey Creation of this test.
> -* <Nadia.Derbey@bull.net>
> -*
> -******************************************************************************/
> + * Copyright (c) Bull S.A.S. 2008
> + * 01/12/08 Nadia Derbey <Nadia.Derbey@bull.net>
> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Clone a process with CLONE_NEWPID flag, register notification on a posix
> + * mqueue and send a mqueue message from the parent. Then check if signal
> + * notification contains si_pid of the parent.
> + */
> +
> #define _GNU_SOURCE 1
> -#include <sys/wait.h>
> -#include <sys/types.h>
> #include <signal.h>
> -#include <stdlib.h>
> -#include <unistd.h>
> -#include <stdio.h>
> #include <mqueue.h>
> -#include "lapi/syscalls.h"
> -#include "pidns_helper.h"
> -#include "test.h"
> -
> -char *TCID = "pidns30";
> -int TST_TOTAL = 1;
> -
> -char *mqname = "mq1";
> -int result = TFAIL;
> +#include "tst_test.h"
> +#include "tst_safe_posix_ipc.h"
> +#include "lapi/namespaces_constants.h"
>
> -int father_to_child[2];
> -int child_to_father[2];
> +#define MQNAME "/LTP_PIDNS30_MQ"
>
> -#define CHILD_PID 1
> -#define PARENT_PID 0
> -
> -#define MSG "HOW ARE YOU"
> -#define MSG_PRIO 1
> -
> -#define NO_STEP -1
> -#define F_STEP_0 0x00
> -#define F_STEP_1 0x01
> -#define F_STEP_2 0x02
> -#define F_STEP_3 0x03
> -#define C_STEP_0 0x10
> -#define C_STEP_1 0x11
> -#define C_STEP_2 0x12
> -
> -mqd_t rc = -1;
> -mqd_t mqd = -1;
> -
> -static void remove_pipe(int *fd)
> -{
> - close(fd[0]);
> - close(fd[1]);
> -}
> +static mqd_t mqd = -1;
> +static int received;
This should be volatile because it is used in the sighandler.
>
> static void remove_mqueue(mqd_t mqd)
> {
> - mq_close(mqd);
> - tst_syscall(__NR_mq_unlink, mqname);
> -}
> -
> -static void cleanup(void)
> -{
> - if (mqd != -1) {
> - remove_mqueue(mqd);
> - }
> - if (rc != -1) {
> - remove_mqueue(rc);
> - }
> - remove_pipe(father_to_child);
> - remove_pipe(child_to_father);
> -}
> + if (mqd != -1)
> + SAFE_MQ_CLOSE(mqd);
>
> -static void cleanup_child(void)
> -{
> - if (mqd != -1) {
> - tst_syscall(__NR_mq_notify, mqd, NULL);
> - }
> - cleanup();
> + mq_unlink(MQNAME);
We don't know that mq_unlink ever succeeds as we never check the
result. This seems like lazyness because it is used at the beginning of
run where it is expected to fail.
> }
>
> -/*
> - * child_signal_handler() - to handle SIGUSR1
> - *
> - * XXX (garrcoop): add calls to cleanup_child() -- or should this be handled
> - * from the libltp signal handler?
> - */
> -static void child_signal_handler(int sig, siginfo_t * si, void *unused)
> +static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
> {
> - char buf[256];
> - struct mq_attr attr;
> + tst_res(TINFO, "Received signal %s from pid %d",
> tst_strsig(si->si_signo), si->si_pid);
Again we should avoid calling tst_res in the signal handler
>
> - if (si->si_signo != SIGUSR1) {
> - printf("received signal = %d unexpectedly\n", si->si_signo);
> + if (si->si_signo != SIGUSR1 || si->si_code != SI_MESGQ ||
> si->si_pid != 0)
If the wrong signal or details are sent then we won't get any debug
information. Unlike with the old version.
> return;
> - }
> -
> - if (si->si_code != SI_MESGQ) {
> - printf("expected signal code SI_MESGQ; got %d instead\n",
> - si->si_code);
> - return;
> - }
> -
> - if (si->si_pid) {
> - printf("expected signal originator PID = 0; got %d instead\n",
> - si->si_pid);
> - return;
> - } else {
> - printf("signal originator PID = 0\n");
> - result = TPASS;
> - }
>
> - /*
> - * Now read the message - Be silent on errors since this is not the
> - * test purpose.
> - */
> - rc = mq_getattr(si->si_int, &attr);
> - if (rc != -1)
> - mq_receive(si->si_int, buf, attr.mq_msgsize, NULL);
> + received++;
> }
>
> -/*
> - * child_fn() - Inside container
> - *
> - * XXX (garrcoop): add more calls to cleanup_child()?
> - */
> -int child_fn(void *arg)
> +static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
> {
> - pid_t pid, ppid;
> + pid_t cpid, ppid;
> struct sigaction sa;
> struct sigevent notif;
> - char buf[5];
> + mqd_t mqd_child;
>
> - /* Set process id and parent pid */
> - pid = getpid();
> + cpid = getpid();
> ppid = getppid();
>
> - if (pid != CHILD_PID || ppid != PARENT_PID) {
> - printf("pidns was not created\n");
> - return 1;
> + if (cpid != 1 || ppid != 0) {
> + tst_res(TFAIL, "got unexpected result of cpid=%d ppid=%d", cpid, ppid);
> + return 0;
Again TST_EXP_EQ*
> }
>
> - /* Close the appropriate end of each pipe */
> - close(child_to_father[0]);
> - close(father_to_child[1]);
> + TST_CHECKPOINT_WAIT(0);
>
> - while (read(father_to_child[0], buf, 1) != 1)
> - sleep(1);
> -
> - mqd = tst_syscall(__NR_mq_open, mqname, O_RDONLY, 0, NULL);
> - if (mqd == -1) {
> - perror("mq_open failed");
> - return 1;
> - } else
> - printf("mq_open succeeded\n");
> -
> - /* Register for notification on message arrival */
> + mqd_child = SAFE_MQ_OPEN(MQNAME, O_RDONLY, 0, NULL);
> notif.sigev_notify = SIGEV_SIGNAL;
> notif.sigev_signo = SIGUSR1;
> - notif.sigev_value.sival_int = mqd;
> - if (tst_syscall(__NR_mq_notify, mqd, ¬if) == -1) {
> - perror("mq_notify failed");
> - return 1;
> - } else
> - printf("successfully registered for notification\n");
> + notif.sigev_value.sival_int = mqd_child;
> +
> + SAFE_MQ_NOTIFY(mqd_child, ¬if);
>
> - /* Define handler for SIGUSR1 */
> sa.sa_flags = SA_SIGINFO;
> - sigemptyset(&sa.sa_mask);
> + SAFE_SIGEMPTYSET(&sa.sa_mask);
> sa.sa_sigaction = child_signal_handler;
> - if (sigaction(SIGUSR1, &sa, NULL) == -1) {
> - perror("sigaction failed");
> - return 1;
> - } else
> - printf("successfully registered handler for SIGUSR1\n");
> + SAFE_SIGACTION(SIGUSR1, &sa, NULL);
>
> - /* Ask parent to send a message to the mqueue */
> - if (write(child_to_father[1], "c:ok", 5) != 5) {
> - perror("write failed");
> - return 1;
> - }
> -
> - sleep(3);
> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>
> - /* Has parent sent a message? */
> - read(father_to_child[0], buf, 5);
> - if (strcmp(buf, "f:ok") != 0) {
> - printf("parent did not send the message!\n");
> - return 1;
> + if (received != 1) {
> + tst_res(TFAIL, "Signal hasn't been received after mqueue
> event");
If we receive more than one signal then this will also be printed. In
any case we can shorted this with TST_EXP_EQ_*.
> + return 0;
> }
> - printf("parent is done - cleaning up\n");
>
> - cleanup_child();
> + tst_res(TPASS, "Signal has been received after mqueue event");
>
> - exit(0);
> + return 0;
> }
>
> -static void setup(void)
> +static void cleanup(void)
> {
> - tst_require_root();
> - check_newpid();
> + remove_mqueue(mqd);
> }
>
> -int main(void)
> +static void run(void)
> {
> - int status;
> - char buf[5];
> - pid_t cpid;
> -
> - setup();
> -
> - if (pipe(child_to_father) == -1 || pipe(father_to_child) == -1) {
> - tst_brkm(TBROK | TERRNO, cleanup, "pipe failed");
> - }
> + int ret;
>
> - tst_syscall(__NR_mq_unlink, mqname);
> + remove_mqueue(mqd);
>
> - /* container creation on PID namespace */
> - cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
> - if (cpid == -1)
> - tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
> + ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
> + if (ret < 0)
> + tst_brk(TBROK | TERRNO, "clone failed");
>
> - mqd =
> - tst_syscall(__NR_mq_open, mqname, O_RDWR | O_CREAT | O_EXCL, 0777,
> - NULL);
> - if (mqd == -1)
> - tst_brkm(TBROK | TERRNO, cleanup, "mq_open failed");
> - else
> - tst_resm(TINFO, "successfully created posix mqueue");
> + mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, 0);
>
> - if (write(father_to_child[1], buf, 1) != 1)
> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>
> - /* Close the appropriate end of each pipe */
> - close(child_to_father[1]);
> - close(father_to_child[0]);
> + SAFE_MQ_SEND(mqd, "pippo", 5, 1);
>
> - /* Is container ready */
> - read(child_to_father[0], buf, 5);
> - if (strcmp(buf, "c:ok") != 0)
> - tst_brkm(TBROK, cleanup,
> - "container did not respond as expected!");
> -
> - rc = mq_send(mqd, MSG, strlen(MSG), MSG_PRIO);
> - if (rc == -1)
> - tst_brkm(TBROK | TERRNO, cleanup, "mq_send failed");
> - else
> - tst_resm(TINFO, "mq_send succeeded");
> -
> - /* Tell the child the message has been sent */
> - if (write(father_to_child[1], "f:ok", 5) != 5)
> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
> -
> - /* Wait for child to finish */
> - if (wait(&status) == -1)
> - tst_resm(TBROK | TERRNO, "wait failed");
> -
> - cleanup();
> -
> - tst_exit();
> + TST_CHECKPOINT_WAKE(0);
> }
> +
> +static struct tst_test test = {
> + .test_all = run,
> + .cleanup = cleanup,
> + .needs_root = 1,
> + .needs_checkpoints = 1,
> +};
> --
> 2.35.3
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API
2022-11-01 12:01 ` Richard Palethorpe
@ 2023-02-08 9:42 ` Andrea Cervesato via ltp
2023-02-08 9:51 ` Andrea Cervesato via ltp
1 sibling, 0 replies; 9+ messages in thread
From: Andrea Cervesato via ltp @ 2023-02-08 9:42 UTC (permalink / raw)
To: rpalethorpe; +Cc: ltp
Hi,
On 11/1/22 13:01, Richard Palethorpe wrote:
> Hello,
>
> Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>> testcases/kernel/containers/pidns/pidns30.c | 312 +++++---------------
>> 1 file changed, 66 insertions(+), 246 deletions(-)
>>
>> diff --git a/testcases/kernel/containers/pidns/pidns30.c b/testcases/kernel/containers/pidns/pidns30.c
>> index c8b0806c0..b01759a87 100644
>> --- a/testcases/kernel/containers/pidns/pidns30.c
>> +++ b/testcases/kernel/containers/pidns/pidns30.c
>> @@ -1,296 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> /*
>> -* Copyright (c) Bull S.A.S. 2008
>> -* 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
>> -*
>> -***************************************************************************
>> -* File: pidns30.c
>> -*
>> -* Description:
>> -* This testcase checks if the si_pid is correctly set when a process
>> -* that has registered for notification on a posix mqueue is in a
>> -* descendant namespace wrt the process that sends a message to that posix
>> -* mqueue.
>> -*
>> -* Test Assertion & Strategy:
>> -* Parent Child
>> -* --------------------------------------------------------------------------
>> -* Create a POSIX mqueue.
>> -* Create a PID namespace container.
>> -* Open that mqueue for reading
>> -* Register for notification when a
>> -* message arrives in that mqueue
>> -* Install a handler for SIGUSR1.
>> -* Write something to the mqueue.
>> -* Inside the handler, check that
>> -* si_pid is set to 0
>> -*
>> -* Usage: <for command-line>
>> -* pidns30
>> -*
>> -* History:
>> -* DATE NAME DESCRIPTION
>> -* 01/12/08 Nadia Derbey Creation of this test.
>> -* <Nadia.Derbey@bull.net>
>> -*
>> -******************************************************************************/
>> + * Copyright (c) Bull S.A.S. 2008
>> + * 01/12/08 Nadia Derbey <Nadia.Derbey@bull.net>
>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Clone a process with CLONE_NEWPID flag, register notification on a posix
>> + * mqueue and send a mqueue message from the parent. Then check if signal
>> + * notification contains si_pid of the parent.
>> + */
>> +
>> #define _GNU_SOURCE 1
>> -#include <sys/wait.h>
>> -#include <sys/types.h>
>> #include <signal.h>
>> -#include <stdlib.h>
>> -#include <unistd.h>
>> -#include <stdio.h>
>> #include <mqueue.h>
>> -#include "lapi/syscalls.h"
>> -#include "pidns_helper.h"
>> -#include "test.h"
>> -
>> -char *TCID = "pidns30";
>> -int TST_TOTAL = 1;
>> -
>> -char *mqname = "mq1";
>> -int result = TFAIL;
>> +#include "tst_test.h"
>> +#include "tst_safe_posix_ipc.h"
>> +#include "lapi/namespaces_constants.h"
>>
>> -int father_to_child[2];
>> -int child_to_father[2];
>> +#define MQNAME "/LTP_PIDNS30_MQ"
>>
>> -#define CHILD_PID 1
>> -#define PARENT_PID 0
>> -
>> -#define MSG "HOW ARE YOU"
>> -#define MSG_PRIO 1
>> -
>> -#define NO_STEP -1
>> -#define F_STEP_0 0x00
>> -#define F_STEP_1 0x01
>> -#define F_STEP_2 0x02
>> -#define F_STEP_3 0x03
>> -#define C_STEP_0 0x10
>> -#define C_STEP_1 0x11
>> -#define C_STEP_2 0x12
>> -
>> -mqd_t rc = -1;
>> -mqd_t mqd = -1;
>> -
>> -static void remove_pipe(int *fd)
>> -{
>> - close(fd[0]);
>> - close(fd[1]);
>> -}
>> +static mqd_t mqd = -1;
>> +static int received;
> This should be volatile because it is used in the sighandler.
>
>>
>> static void remove_mqueue(mqd_t mqd)
>> {
>> - mq_close(mqd);
>> - tst_syscall(__NR_mq_unlink, mqname);
>> -}
>> -
>> -static void cleanup(void)
>> -{
>> - if (mqd != -1) {
>> - remove_mqueue(mqd);
>> - }
>> - if (rc != -1) {
>> - remove_mqueue(rc);
>> - }
>> - remove_pipe(father_to_child);
>> - remove_pipe(child_to_father);
>> -}
>> + if (mqd != -1)
>> + SAFE_MQ_CLOSE(mqd);
>>
>> -static void cleanup_child(void)
>> -{
>> - if (mqd != -1) {
>> - tst_syscall(__NR_mq_notify, mqd, NULL);
>> - }
>> - cleanup();
>> + mq_unlink(MQNAME);
> We don't know that mq_unlink ever succeeds as we never check the
> result. This seems like lazyness because it is used at the beginning of
> run where it is expected to fail.
>
>> }
>>
>> -/*
>> - * child_signal_handler() - to handle SIGUSR1
>> - *
>> - * XXX (garrcoop): add calls to cleanup_child() -- or should this be handled
>> - * from the libltp signal handler?
>> - */
>> -static void child_signal_handler(int sig, siginfo_t * si, void *unused)
>> +static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
>> {
>> - char buf[256];
>> - struct mq_attr attr;
>> + tst_res(TINFO, "Received signal %s from pid %d",
>> tst_strsig(si->si_signo), si->si_pid);
> Again we should avoid calling tst_res in the signal handler
>
>>
>> - if (si->si_signo != SIGUSR1) {
>> - printf("received signal = %d unexpectedly\n", si->si_signo);
>> + if (si->si_signo != SIGUSR1 || si->si_code != SI_MESGQ ||
>> si->si_pid != 0)
> If the wrong signal or details are sent then we won't get any debug
> information. Unlike with the old version.
>
>> return;
>> - }
>> -
>> - if (si->si_code != SI_MESGQ) {
>> - printf("expected signal code SI_MESGQ; got %d instead\n",
>> - si->si_code);
>> - return;
>> - }
>> -
>> - if (si->si_pid) {
>> - printf("expected signal originator PID = 0; got %d instead\n",
>> - si->si_pid);
>> - return;
>> - } else {
>> - printf("signal originator PID = 0\n");
>> - result = TPASS;
>> - }
>>
>> - /*
>> - * Now read the message - Be silent on errors since this is not the
>> - * test purpose.
>> - */
>> - rc = mq_getattr(si->si_int, &attr);
>> - if (rc != -1)
>> - mq_receive(si->si_int, buf, attr.mq_msgsize, NULL);
>> + received++;
>> }
>>
>> -/*
>> - * child_fn() - Inside container
>> - *
>> - * XXX (garrcoop): add more calls to cleanup_child()?
>> - */
>> -int child_fn(void *arg)
>> +static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
>> {
>> - pid_t pid, ppid;
>> + pid_t cpid, ppid;
>> struct sigaction sa;
>> struct sigevent notif;
>> - char buf[5];
>> + mqd_t mqd_child;
>>
>> - /* Set process id and parent pid */
>> - pid = getpid();
>> + cpid = getpid();
>> ppid = getppid();
>>
>> - if (pid != CHILD_PID || ppid != PARENT_PID) {
>> - printf("pidns was not created\n");
>> - return 1;
>> + if (cpid != 1 || ppid != 0) {
>> + tst_res(TFAIL, "got unexpected result of cpid=%d ppid=%d", cpid, ppid);
>> + return 0;
> Again TST_EXP_EQ*
>
>> }
>>
>> - /* Close the appropriate end of each pipe */
>> - close(child_to_father[0]);
>> - close(father_to_child[1]);
>> + TST_CHECKPOINT_WAIT(0);
>>
>> - while (read(father_to_child[0], buf, 1) != 1)
>> - sleep(1);
>> -
>> - mqd = tst_syscall(__NR_mq_open, mqname, O_RDONLY, 0, NULL);
>> - if (mqd == -1) {
>> - perror("mq_open failed");
>> - return 1;
>> - } else
>> - printf("mq_open succeeded\n");
>> -
>> - /* Register for notification on message arrival */
>> + mqd_child = SAFE_MQ_OPEN(MQNAME, O_RDONLY, 0, NULL);
>> notif.sigev_notify = SIGEV_SIGNAL;
>> notif.sigev_signo = SIGUSR1;
>> - notif.sigev_value.sival_int = mqd;
>> - if (tst_syscall(__NR_mq_notify, mqd, ¬if) == -1) {
>> - perror("mq_notify failed");
>> - return 1;
>> - } else
>> - printf("successfully registered for notification\n");
>> + notif.sigev_value.sival_int = mqd_child;
>> +
>> + SAFE_MQ_NOTIFY(mqd_child, ¬if);
>>
>> - /* Define handler for SIGUSR1 */
>> sa.sa_flags = SA_SIGINFO;
>> - sigemptyset(&sa.sa_mask);
>> + SAFE_SIGEMPTYSET(&sa.sa_mask);
>> sa.sa_sigaction = child_signal_handler;
>> - if (sigaction(SIGUSR1, &sa, NULL) == -1) {
>> - perror("sigaction failed");
>> - return 1;
>> - } else
>> - printf("successfully registered handler for SIGUSR1\n");
>> + SAFE_SIGACTION(SIGUSR1, &sa, NULL);
>>
>> - /* Ask parent to send a message to the mqueue */
>> - if (write(child_to_father[1], "c:ok", 5) != 5) {
>> - perror("write failed");
>> - return 1;
>> - }
>> -
>> - sleep(3);
>> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>>
>> - /* Has parent sent a message? */
>> - read(father_to_child[0], buf, 5);
>> - if (strcmp(buf, "f:ok") != 0) {
>> - printf("parent did not send the message!\n");
>> - return 1;
>> + if (received != 1) {
>> + tst_res(TFAIL, "Signal hasn't been received after mqueue
>> event");
> If we receive more than one signal then this will also be printed. In
> any case we can shorted this with TST_EXP_EQ_*.
I would keep this message because it's easier to read when test fails.
>> + return 0;
>> }
>> - printf("parent is done - cleaning up\n");
>>
>> - cleanup_child();
>> + tst_res(TPASS, "Signal has been received after mqueue event");
>>
>> - exit(0);
>> + return 0;
>> }
>>
>> -static void setup(void)
>> +static void cleanup(void)
>> {
>> - tst_require_root();
>> - check_newpid();
>> + remove_mqueue(mqd);
>> }
>>
>> -int main(void)
>> +static void run(void)
>> {
>> - int status;
>> - char buf[5];
>> - pid_t cpid;
>> -
>> - setup();
>> -
>> - if (pipe(child_to_father) == -1 || pipe(father_to_child) == -1) {
>> - tst_brkm(TBROK | TERRNO, cleanup, "pipe failed");
>> - }
>> + int ret;
>>
>> - tst_syscall(__NR_mq_unlink, mqname);
>> + remove_mqueue(mqd);
>>
>> - /* container creation on PID namespace */
>> - cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
>> - if (cpid == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
>> + ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
>> + if (ret < 0)
>> + tst_brk(TBROK | TERRNO, "clone failed");
>>
>> - mqd =
>> - tst_syscall(__NR_mq_open, mqname, O_RDWR | O_CREAT | O_EXCL, 0777,
>> - NULL);
>> - if (mqd == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "mq_open failed");
>> - else
>> - tst_resm(TINFO, "successfully created posix mqueue");
>> + mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, 0);
>>
>> - if (write(father_to_child[1], buf, 1) != 1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
>> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>>
>> - /* Close the appropriate end of each pipe */
>> - close(child_to_father[1]);
>> - close(father_to_child[0]);
>> + SAFE_MQ_SEND(mqd, "pippo", 5, 1);
>>
>> - /* Is container ready */
>> - read(child_to_father[0], buf, 5);
>> - if (strcmp(buf, "c:ok") != 0)
>> - tst_brkm(TBROK, cleanup,
>> - "container did not respond as expected!");
>> -
>> - rc = mq_send(mqd, MSG, strlen(MSG), MSG_PRIO);
>> - if (rc == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "mq_send failed");
>> - else
>> - tst_resm(TINFO, "mq_send succeeded");
>> -
>> - /* Tell the child the message has been sent */
>> - if (write(father_to_child[1], "f:ok", 5) != 5)
>> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
>> -
>> - /* Wait for child to finish */
>> - if (wait(&status) == -1)
>> - tst_resm(TBROK | TERRNO, "wait failed");
>> -
>> - cleanup();
>> -
>> - tst_exit();
>> + TST_CHECKPOINT_WAKE(0);
>> }
>> +
>> +static struct tst_test test = {
>> + .test_all = run,
>> + .cleanup = cleanup,
>> + .needs_root = 1,
>> + .needs_checkpoints = 1,
>> +};
>> --
>> 2.35.3
>
Andrea
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API
2022-11-01 12:01 ` Richard Palethorpe
2023-02-08 9:42 ` Andrea Cervesato via ltp
@ 2023-02-08 9:51 ` Andrea Cervesato via ltp
1 sibling, 0 replies; 9+ messages in thread
From: Andrea Cervesato via ltp @ 2023-02-08 9:51 UTC (permalink / raw)
To: rpalethorpe; +Cc: ltp
Hi,
On 11/1/22 13:01, Richard Palethorpe wrote:
> Hello,
>
> Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>> testcases/kernel/containers/pidns/pidns30.c | 312 +++++---------------
>> 1 file changed, 66 insertions(+), 246 deletions(-)
>>
>> diff --git a/testcases/kernel/containers/pidns/pidns30.c b/testcases/kernel/containers/pidns/pidns30.c
>> index c8b0806c0..b01759a87 100644
>> --- a/testcases/kernel/containers/pidns/pidns30.c
>> +++ b/testcases/kernel/containers/pidns/pidns30.c
>> @@ -1,296 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> /*
>> -* Copyright (c) Bull S.A.S. 2008
>> -* 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
>> -*
>> -***************************************************************************
>> -* File: pidns30.c
>> -*
>> -* Description:
>> -* This testcase checks if the si_pid is correctly set when a process
>> -* that has registered for notification on a posix mqueue is in a
>> -* descendant namespace wrt the process that sends a message to that posix
>> -* mqueue.
>> -*
>> -* Test Assertion & Strategy:
>> -* Parent Child
>> -* --------------------------------------------------------------------------
>> -* Create a POSIX mqueue.
>> -* Create a PID namespace container.
>> -* Open that mqueue for reading
>> -* Register for notification when a
>> -* message arrives in that mqueue
>> -* Install a handler for SIGUSR1.
>> -* Write something to the mqueue.
>> -* Inside the handler, check that
>> -* si_pid is set to 0
>> -*
>> -* Usage: <for command-line>
>> -* pidns30
>> -*
>> -* History:
>> -* DATE NAME DESCRIPTION
>> -* 01/12/08 Nadia Derbey Creation of this test.
>> -* <Nadia.Derbey@bull.net>
>> -*
>> -******************************************************************************/
>> + * Copyright (c) Bull S.A.S. 2008
>> + * 01/12/08 Nadia Derbey <Nadia.Derbey@bull.net>
>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Clone a process with CLONE_NEWPID flag, register notification on a posix
>> + * mqueue and send a mqueue message from the parent. Then check if signal
>> + * notification contains si_pid of the parent.
>> + */
>> +
>> #define _GNU_SOURCE 1
>> -#include <sys/wait.h>
>> -#include <sys/types.h>
>> #include <signal.h>
>> -#include <stdlib.h>
>> -#include <unistd.h>
>> -#include <stdio.h>
>> #include <mqueue.h>
>> -#include "lapi/syscalls.h"
>> -#include "pidns_helper.h"
>> -#include "test.h"
>> -
>> -char *TCID = "pidns30";
>> -int TST_TOTAL = 1;
>> -
>> -char *mqname = "mq1";
>> -int result = TFAIL;
>> +#include "tst_test.h"
>> +#include "tst_safe_posix_ipc.h"
>> +#include "lapi/namespaces_constants.h"
>>
>> -int father_to_child[2];
>> -int child_to_father[2];
>> +#define MQNAME "/LTP_PIDNS30_MQ"
>>
>> -#define CHILD_PID 1
>> -#define PARENT_PID 0
>> -
>> -#define MSG "HOW ARE YOU"
>> -#define MSG_PRIO 1
>> -
>> -#define NO_STEP -1
>> -#define F_STEP_0 0x00
>> -#define F_STEP_1 0x01
>> -#define F_STEP_2 0x02
>> -#define F_STEP_3 0x03
>> -#define C_STEP_0 0x10
>> -#define C_STEP_1 0x11
>> -#define C_STEP_2 0x12
>> -
>> -mqd_t rc = -1;
>> -mqd_t mqd = -1;
>> -
>> -static void remove_pipe(int *fd)
>> -{
>> - close(fd[0]);
>> - close(fd[1]);
>> -}
>> +static mqd_t mqd = -1;
>> +static int received;
> This should be volatile because it is used in the sighandler.
>
>>
>> static void remove_mqueue(mqd_t mqd)
>> {
>> - mq_close(mqd);
>> - tst_syscall(__NR_mq_unlink, mqname);
>> -}
>> -
>> -static void cleanup(void)
>> -{
>> - if (mqd != -1) {
>> - remove_mqueue(mqd);
>> - }
>> - if (rc != -1) {
>> - remove_mqueue(rc);
>> - }
>> - remove_pipe(father_to_child);
>> - remove_pipe(child_to_father);
>> -}
>> + if (mqd != -1)
>> + SAFE_MQ_CLOSE(mqd);
>>
>> -static void cleanup_child(void)
>> -{
>> - if (mqd != -1) {
>> - tst_syscall(__NR_mq_notify, mqd, NULL);
>> - }
>> - cleanup();
>> + mq_unlink(MQNAME);
> We don't know that mq_unlink ever succeeds as we never check the
> result. This seems like lazyness because it is used at the beginning of
> run where it is expected to fail.
We need to unlink the msg when test is iterated multiple times. In case
there's not msg, syscall will return ENOENT and that's why there's no
check for it.
>
>> }
>>
>> -/*
>> - * child_signal_handler() - to handle SIGUSR1
>> - *
>> - * XXX (garrcoop): add calls to cleanup_child() -- or should this be handled
>> - * from the libltp signal handler?
>> - */
>> -static void child_signal_handler(int sig, siginfo_t * si, void *unused)
>> +static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
>> {
>> - char buf[256];
>> - struct mq_attr attr;
>> + tst_res(TINFO, "Received signal %s from pid %d",
>> tst_strsig(si->si_signo), si->si_pid);
> Again we should avoid calling tst_res in the signal handler
>
>>
>> - if (si->si_signo != SIGUSR1) {
>> - printf("received signal = %d unexpectedly\n", si->si_signo);
>> + if (si->si_signo != SIGUSR1 || si->si_code != SI_MESGQ ||
>> si->si_pid != 0)
> If the wrong signal or details are sent then we won't get any debug
> information. Unlike with the old version.
>
>> return;
>> - }
>> -
>> - if (si->si_code != SI_MESGQ) {
>> - printf("expected signal code SI_MESGQ; got %d instead\n",
>> - si->si_code);
>> - return;
>> - }
>> -
>> - if (si->si_pid) {
>> - printf("expected signal originator PID = 0; got %d instead\n",
>> - si->si_pid);
>> - return;
>> - } else {
>> - printf("signal originator PID = 0\n");
>> - result = TPASS;
>> - }
>>
>> - /*
>> - * Now read the message - Be silent on errors since this is not the
>> - * test purpose.
>> - */
>> - rc = mq_getattr(si->si_int, &attr);
>> - if (rc != -1)
>> - mq_receive(si->si_int, buf, attr.mq_msgsize, NULL);
>> + received++;
>> }
>>
>> -/*
>> - * child_fn() - Inside container
>> - *
>> - * XXX (garrcoop): add more calls to cleanup_child()?
>> - */
>> -int child_fn(void *arg)
>> +static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
>> {
>> - pid_t pid, ppid;
>> + pid_t cpid, ppid;
>> struct sigaction sa;
>> struct sigevent notif;
>> - char buf[5];
>> + mqd_t mqd_child;
>>
>> - /* Set process id and parent pid */
>> - pid = getpid();
>> + cpid = getpid();
>> ppid = getppid();
>>
>> - if (pid != CHILD_PID || ppid != PARENT_PID) {
>> - printf("pidns was not created\n");
>> - return 1;
>> + if (cpid != 1 || ppid != 0) {
>> + tst_res(TFAIL, "got unexpected result of cpid=%d ppid=%d", cpid, ppid);
>> + return 0;
> Again TST_EXP_EQ*
>
>> }
>>
>> - /* Close the appropriate end of each pipe */
>> - close(child_to_father[0]);
>> - close(father_to_child[1]);
>> + TST_CHECKPOINT_WAIT(0);
>>
>> - while (read(father_to_child[0], buf, 1) != 1)
>> - sleep(1);
>> -
>> - mqd = tst_syscall(__NR_mq_open, mqname, O_RDONLY, 0, NULL);
>> - if (mqd == -1) {
>> - perror("mq_open failed");
>> - return 1;
>> - } else
>> - printf("mq_open succeeded\n");
>> -
>> - /* Register for notification on message arrival */
>> + mqd_child = SAFE_MQ_OPEN(MQNAME, O_RDONLY, 0, NULL);
>> notif.sigev_notify = SIGEV_SIGNAL;
>> notif.sigev_signo = SIGUSR1;
>> - notif.sigev_value.sival_int = mqd;
>> - if (tst_syscall(__NR_mq_notify, mqd, ¬if) == -1) {
>> - perror("mq_notify failed");
>> - return 1;
>> - } else
>> - printf("successfully registered for notification\n");
>> + notif.sigev_value.sival_int = mqd_child;
>> +
>> + SAFE_MQ_NOTIFY(mqd_child, ¬if);
>>
>> - /* Define handler for SIGUSR1 */
>> sa.sa_flags = SA_SIGINFO;
>> - sigemptyset(&sa.sa_mask);
>> + SAFE_SIGEMPTYSET(&sa.sa_mask);
>> sa.sa_sigaction = child_signal_handler;
>> - if (sigaction(SIGUSR1, &sa, NULL) == -1) {
>> - perror("sigaction failed");
>> - return 1;
>> - } else
>> - printf("successfully registered handler for SIGUSR1\n");
>> + SAFE_SIGACTION(SIGUSR1, &sa, NULL);
>>
>> - /* Ask parent to send a message to the mqueue */
>> - if (write(child_to_father[1], "c:ok", 5) != 5) {
>> - perror("write failed");
>> - return 1;
>> - }
>> -
>> - sleep(3);
>> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>>
>> - /* Has parent sent a message? */
>> - read(father_to_child[0], buf, 5);
>> - if (strcmp(buf, "f:ok") != 0) {
>> - printf("parent did not send the message!\n");
>> - return 1;
>> + if (received != 1) {
>> + tst_res(TFAIL, "Signal hasn't been received after mqueue
>> event");
> If we receive more than one signal then this will also be printed. In
> any case we can shorted this with TST_EXP_EQ_*.
>
>> + return 0;
>> }
>> - printf("parent is done - cleaning up\n");
>>
>> - cleanup_child();
>> + tst_res(TPASS, "Signal has been received after mqueue event");
>>
>> - exit(0);
>> + return 0;
>> }
>>
>> -static void setup(void)
>> +static void cleanup(void)
>> {
>> - tst_require_root();
>> - check_newpid();
>> + remove_mqueue(mqd);
>> }
>>
>> -int main(void)
>> +static void run(void)
>> {
>> - int status;
>> - char buf[5];
>> - pid_t cpid;
>> -
>> - setup();
>> -
>> - if (pipe(child_to_father) == -1 || pipe(father_to_child) == -1) {
>> - tst_brkm(TBROK | TERRNO, cleanup, "pipe failed");
>> - }
>> + int ret;
>>
>> - tst_syscall(__NR_mq_unlink, mqname);
>> + remove_mqueue(mqd);
>>
>> - /* container creation on PID namespace */
>> - cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
>> - if (cpid == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
>> + ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
>> + if (ret < 0)
>> + tst_brk(TBROK | TERRNO, "clone failed");
>>
>> - mqd =
>> - tst_syscall(__NR_mq_open, mqname, O_RDWR | O_CREAT | O_EXCL, 0777,
>> - NULL);
>> - if (mqd == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "mq_open failed");
>> - else
>> - tst_resm(TINFO, "successfully created posix mqueue");
>> + mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, 0);
>>
>> - if (write(father_to_child[1], buf, 1) != 1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
>> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>>
>> - /* Close the appropriate end of each pipe */
>> - close(child_to_father[1]);
>> - close(father_to_child[0]);
>> + SAFE_MQ_SEND(mqd, "pippo", 5, 1);
>>
>> - /* Is container ready */
>> - read(child_to_father[0], buf, 5);
>> - if (strcmp(buf, "c:ok") != 0)
>> - tst_brkm(TBROK, cleanup,
>> - "container did not respond as expected!");
>> -
>> - rc = mq_send(mqd, MSG, strlen(MSG), MSG_PRIO);
>> - if (rc == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "mq_send failed");
>> - else
>> - tst_resm(TINFO, "mq_send succeeded");
>> -
>> - /* Tell the child the message has been sent */
>> - if (write(father_to_child[1], "f:ok", 5) != 5)
>> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
>> -
>> - /* Wait for child to finish */
>> - if (wait(&status) == -1)
>> - tst_resm(TBROK | TERRNO, "wait failed");
>> -
>> - cleanup();
>> -
>> - tst_exit();
>> + TST_CHECKPOINT_WAKE(0);
>> }
>> +
>> +static struct tst_test test = {
>> + .test_all = run,
>> + .cleanup = cleanup,
>> + .needs_root = 1,
>> + .needs_checkpoints = 1,
>> +};
>> --
>> 2.35.3
>
Andrea
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH v3 3/3] Refactor pidns31 test using new LTP API
2022-08-16 11:31 [LTP] [PATCH v3 0/3] Refactor pidns30 and pidns31 using new LTP API Andrea Cervesato via ltp
2022-08-16 11:31 ` [LTP] [PATCH v3 1/3] Add more mqueue safe macros Andrea Cervesato via ltp
2022-08-16 11:31 ` [LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API Andrea Cervesato via ltp
@ 2022-08-16 11:31 ` Andrea Cervesato via ltp
2022-11-01 12:54 ` Richard Palethorpe
2 siblings, 1 reply; 9+ messages in thread
From: Andrea Cervesato via ltp @ 2022-08-16 11:31 UTC (permalink / raw)
To: ltp
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
testcases/kernel/containers/pidns/pidns31.c | 347 +++++---------------
1 file changed, 74 insertions(+), 273 deletions(-)
diff --git a/testcases/kernel/containers/pidns/pidns31.c b/testcases/kernel/containers/pidns/pidns31.c
index 8821ec83c..20f6117fd 100644
--- a/testcases/kernel/containers/pidns/pidns31.c
+++ b/testcases/kernel/containers/pidns/pidns31.c
@@ -1,330 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
/*
-* Copyright (c) Bull S.A.S. 2008
-* 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
-*
-***************************************************************************
-* File: pidns31.c
-*
-* Description:
-* This testcase checks if the si_pid is correctly set when a process
-* that has registered for notification on a posix mqueue is in an
-* ancestor namespace wrt the process that sends a message to that posix
-* mqueue.
-*
-* Test Assertion & Strategy:
-* Parent Child
-* --------------------------------------------------------------------------
-* Create a POSIX mqueue.
-* Create a PID namespace container.
-* Register for notification when a
-* message arrives in that mqueue
-* Install a handler for SIGUSR1.
-* Open that mqueue for writing
-* Write something to the mqueue.
-* Inside the handler, check that
-* si_pid is set to the child's pid
-*
-* Usage: <for command-line>
-* pidns31
-*
-* History:
-* DATE NAME DESCRIPTION
-* 04/12/08 Nadia Derbey Creation of this test.
-* <Nadia.Derbey@bull.net>
-*
-******************************************************************************/
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE
-#endif
-#include <sys/wait.h>
-#include <sys/types.h>
-#include <signal.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <stdio.h>
-#include <mqueue.h>
-#include "lapi/syscalls.h"
-#include "pidns_helper.h"
-#include "test.h"
-
-char *TCID = "pidns31";
-int TST_TOTAL = 1;
-
-char *mqname = "mq1";
-int result = TFAIL;
-
-int father_to_child[2];
+ * Copyright (c) Bull S.A.S. 2008
+ * 01/12/08 Nadia Derbey <Nadia.Derbey@bull.net>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
-#define CHILD_PID 1
-#define PARENT_PID 0
+/*\
+ * [Description]
+ *
+ * Clone a process with CLONE_NEWPID flag, register notification on a posix
+ * mqueue and send a mqueue message from the child. Then check if signal
+ * notification contains si_pid of the child.
+ */
-#define MSG "HOW ARE YOU"
-#define MSG_PRIO 1
+#define _GNU_SOURCE 1
+#include <signal.h>
+#include <mqueue.h>
+#include "tst_test.h"
+#include "tst_safe_posix_ipc.h"
+#include "lapi/namespaces_constants.h"
-#define NO_STEP -1
-#define F_STEP_0 0x00
-#define F_STEP_1 0x01
-#define F_STEP_2 0x02
-#define F_STEP_3 0x03
-#define C_STEP_0 0x10
-#define C_STEP_1 0x11
+#define MQNAME "/LTP_PIDNS30_MQ"
struct notify_info {
mqd_t mqd;
pid_t pid;
};
-static void remove_pipe(int *fd)
-{
- close(fd[0]);
- close(fd[1]);
-}
+static mqd_t mqd = -1;
+static volatile int received;
static void remove_mqueue(mqd_t mqd)
{
- mq_close(mqd);
- tst_syscall(__NR_mq_unlink, mqname);
-}
-
-/*
- * steps F_STEP_XX : called from main
- * steps C_STEP_XX : called from child_fn
- */
-static void cleanup_resources(int step, mqd_t mqd)
-{
- switch (step) {
- case C_STEP_1:
- close(father_to_child[0]);
- /* fall through */
- case C_STEP_0:
- mq_close(mqd);
- break;
-
- case F_STEP_3:
- remove_mqueue(mqd);
- close(father_to_child[1]);
- break;
-
- case F_STEP_2:
- tst_syscall(__NR_mq_notify, mqd, NULL);
- /* fall through */
- case F_STEP_1:
- remove_mqueue(mqd);
- /* fall through */
- case F_STEP_0:
- remove_pipe(father_to_child);
- break;
- default:
- tst_resm(TWARN, "Unknown code - no resource removed.");
- break;
- }
-}
-
-/*
- * cleanup_mqueue() - performs all ONE TIME cleanup for this test at
- * completion or premature exit.
- * step == -1 means no local resource to remove.
- */
-void cleanup_mqueue(int result, int step, mqd_t mqd)
-{
- if (step != NO_STEP)
- cleanup_resources(step, mqd);
+ if (mqd != -1)
+ SAFE_MQ_CLOSE(mqd);
- tst_exit();
+ mq_unlink(MQNAME);
}
-/*
- * child_fn() - Inside container
- */
-int child_fn(void *arg)
+static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
{
- pid_t pid, ppid;
- mqd_t mqd;
- char buf[5];
-
- /* Set process id and parent pid */
- pid = getpid();
- ppid = getppid();
-
- if (pid != CHILD_PID || ppid != PARENT_PID) {
- tst_resm(TBROK, "cinit: pidns is not created");
- cleanup_mqueue(TBROK, NO_STEP, 0);
- }
+ struct notify_info *info;
- /* Close the appropriate end of pipe */
- close(father_to_child[1]);
+ received = 0;
- /* Is parent ready to receive a message? */
- read(father_to_child[0], buf, 5);
- if (strcmp(buf, "f:ok")) {
- tst_resm(TBROK, "cinit: parent did not send the message!");
- cleanup_mqueue(TBROK, NO_STEP, 0);
- }
- tst_resm(TINFO, "cinit: my father is ready to receive a message");
+ tst_res(TINFO, "Received signal %s from pid %d", tst_strsig(si->si_signo), si->si_pid);
- mqd = tst_syscall(__NR_mq_open, mqname, O_WRONLY, 0, NULL);
- if (mqd == (mqd_t) - 1) {
- tst_resm(TBROK, "cinit: mq_open() failed (%s)",
- strerror(errno));
- cleanup_mqueue(TBROK, NO_STEP, 0);
- }
- tst_resm(TINFO, "cinit: mq_open succeeded");
+ if (si->si_signo != SIGUSR1 || si->si_code != SI_MESGQ)
+ return;
- if (mq_send(mqd, MSG, strlen(MSG), MSG_PRIO) == (mqd_t) - 1) {
- tst_resm(TBROK, "cinit: mq_send() failed (%s)",
- strerror(errno));
- cleanup_mqueue(TBROK, C_STEP_0, mqd);
- }
- tst_resm(TINFO, "cinit: mq_send() succeeded");
+ info = (struct notify_info*)si->si_ptr;
- /* Cleanup and exit */
- cleanup_resources(C_STEP_1, mqd);
- exit(0);
+ if (si->si_pid == info->pid)
+ received++;
}
-/*
- * father_signal_handler()
- */
-static void father_signal_handler(int sig, siginfo_t * si, void *unused)
+static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
{
- char buf[256];
- struct mq_attr attr;
- struct notify_info *info;
-
- if (si->si_signo != SIGUSR1) {
- tst_resm(TBROK, "father: received %s unexpectedly",
- strsignal(si->si_signo));
- return;
- }
+ pid_t cpid, ppid;
+ mqd_t mqd_child;
- if (si->si_code != SI_MESGQ) {
- tst_resm(TBROK, "father: expected signal code SI_MESGQ - "
- "Got %d", si->si_code);
- return;
- }
+ cpid = getpid();
+ ppid = getppid();
- if (!si->si_ptr) {
- tst_resm(TBROK, "father: expected si_ptr - Got NULL");
- return;
+ if (cpid != 1 || ppid != 0) {
+ tst_res(TFAIL, "got unexpected result of cpid=%d ppid=%d", cpid, ppid);
+ return 0;
}
- info = (struct notify_info *)si->si_ptr;
+ TST_CHECKPOINT_WAIT(0);
- if (si->si_pid != info->pid) {
- tst_resm(TFAIL,
- "father: expected signal originator PID = %d - Got %d",
- info->pid, si->si_pid);
- return;
- }
+ mqd_child = SAFE_MQ_OPEN(MQNAME, O_WRONLY, 0, NULL);
+ SAFE_MQ_SEND(mqd_child, "pippo", 5, 1);
- tst_resm(TPASS, "father: signal originator PID = %d", si->si_pid);
- result = TPASS;
+ TST_CHECKPOINT_WAKE(0);
- /*
- * Now read the message - Be silent on errors since this is not the
- * test purpose.
- */
- if (!mq_getattr(info->mqd, &attr))
- mq_receive(info->mqd, buf, attr.mq_msgsize, NULL);
+ return 0;
}
-static void setup(void)
+static void cleanup(void)
{
- tst_require_root();
- check_newpid();
+ remove_mqueue(mqd);
}
-/***********************************************************************
-* M A I N
-***********************************************************************/
-
-int main(void)
+static void run(void)
{
- pid_t cpid;
- mqd_t mqd;
- struct sigevent notif;
+ int cpid, status;
struct sigaction sa;
- int status;
+ struct sigevent notif;
struct notify_info info;
- setup();
-
- if (pipe(father_to_child) == -1) {
- tst_resm(TBROK, "parent: pipe() failed. aborting!");
- cleanup_mqueue(TBROK, NO_STEP, 0);
- }
+ remove_mqueue(mqd);
- tst_syscall(__NR_mq_unlink, mqname);
- mqd =
- tst_syscall(__NR_mq_open, mqname, O_RDWR | O_CREAT | O_EXCL, 0777,
- NULL);
- if (mqd == (mqd_t) - 1) {
- tst_resm(TBROK, "parent: mq_open() failed (%s)",
- strerror(errno));
- cleanup_mqueue(TBROK, F_STEP_0, 0);
- }
- tst_resm(TINFO, "parent: successfully created posix mqueue");
+ cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
+ if (cpid < 0)
+ tst_brk(TBROK | TERRNO, "clone failed");
- /* container creation on PID namespace */
- cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
- if (cpid < 0) {
- tst_resm(TBROK, "parent: clone() failed(%s)", strerror(errno));
- cleanup_mqueue(TBROK, F_STEP_1, mqd);
- }
- tst_resm(TINFO, "parent: successfully created child (pid = %d)", cpid);
+ mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
- /* Register for notification on message arrival */
notif.sigev_notify = SIGEV_SIGNAL;
notif.sigev_signo = SIGUSR1;
info.mqd = mqd;
info.pid = cpid;
notif.sigev_value.sival_ptr = &info;
- if (tst_syscall(__NR_mq_notify, mqd, ¬if) == (mqd_t) -1) {
- tst_resm(TBROK, "parent: mq_notify() failed (%s)",
- strerror(errno));
- cleanup_mqueue(TBROK, F_STEP_1, mqd);
- }
- tst_resm(TINFO, "parent: successfully registered for notification");
- /* Define handler for SIGUSR1 */
- sa.sa_flags = SA_SIGINFO;
- sigemptyset(&sa.sa_mask);
- sa.sa_sigaction = father_signal_handler;
- if (sigaction(SIGUSR1, &sa, NULL) == -1) {
- tst_resm(TBROK, "parent: sigaction() failed(%s)",
- strerror(errno));
- cleanup_mqueue(TBROK, F_STEP_2, mqd);
- }
- tst_resm(TINFO, "parent: successfully registered handler for SIGUSR1");
+ SAFE_MQ_NOTIFY(mqd, ¬if);
- /* Close the appropriate end of pipe */
- close(father_to_child[0]);
+ sa.sa_flags = SA_SIGINFO;
+ SAFE_SIGEMPTYSET(&sa.sa_mask);
+ sa.sa_sigaction = child_signal_handler;
+ SAFE_SIGACTION(SIGUSR1, &sa, NULL);
- /* Tell the child a message can be sent */
- if (write(father_to_child[1], "f:ok", 5) != 5) {
- tst_resm(TBROK, "parent: pipe is broken(%s)", strerror(errno));
- cleanup_mqueue(TBROK, F_STEP_2, mqd);
- }
+ TST_CHECKPOINT_WAKE_AND_WAIT(0);
- sleep(3);
+ SAFE_WAITPID(cpid, &status, 0);
- /* Wait for child to finish */
- if (wait(&status) == -1) {
- tst_resm(TBROK, "parent: wait() failed(%s)", strerror(errno));
- cleanup_mqueue(TBROK, F_STEP_1, mqd);
+ if (received != 1) {
+ tst_res(TFAIL, "Signal hasn't been received after mqueue event");
+ return;
}
- cleanup_mqueue(result, F_STEP_3, mqd);
-
- tst_exit();
+ tst_res(TPASS, "Signal has been received after mqueue event");
}
+
+static struct tst_test test = {
+ .test_all = run,
+ .cleanup = cleanup,
+ .needs_root = 1,
+ .needs_checkpoints = 1,
+};
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH v3 3/3] Refactor pidns31 test using new LTP API
2022-08-16 11:31 ` [LTP] [PATCH v3 3/3] Refactor pidns31 " Andrea Cervesato via ltp
@ 2022-11-01 12:54 ` Richard Palethorpe
0 siblings, 0 replies; 9+ messages in thread
From: Richard Palethorpe @ 2022-11-01 12:54 UTC (permalink / raw)
To: Andrea Cervesato; +Cc: ltp
Hello,
Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> testcases/kernel/containers/pidns/pidns31.c | 347 +++++---------------
> 1 file changed, 74 insertions(+), 273 deletions(-)
>
> diff --git a/testcases/kernel/containers/pidns/pidns31.c b/testcases/kernel/containers/pidns/pidns31.c
> index 8821ec83c..20f6117fd 100644
> --- a/testcases/kernel/containers/pidns/pidns31.c
> +++ b/testcases/kernel/containers/pidns/pidns31.c
> @@ -1,330 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> -* Copyright (c) Bull S.A.S. 2008
> -* 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
> -*
> -***************************************************************************
> -* File: pidns31.c
> -*
> -* Description:
> -* This testcase checks if the si_pid is correctly set when a process
> -* that has registered for notification on a posix mqueue is in an
> -* ancestor namespace wrt the process that sends a message to that posix
> -* mqueue.
> -*
> -* Test Assertion & Strategy:
> -* Parent Child
> -* --------------------------------------------------------------------------
> -* Create a POSIX mqueue.
> -* Create a PID namespace container.
> -* Register for notification when a
> -* message arrives in that mqueue
> -* Install a handler for SIGUSR1.
> -* Open that mqueue for writing
> -* Write something to the mqueue.
> -* Inside the handler, check that
> -* si_pid is set to the child's pid
> -*
> -* Usage: <for command-line>
> -* pidns31
> -*
> -* History:
> -* DATE NAME DESCRIPTION
> -* 04/12/08 Nadia Derbey Creation of this test.
> -* <Nadia.Derbey@bull.net>
> -*
> -******************************************************************************/
> -#ifndef _GNU_SOURCE
> -#define _GNU_SOURCE
> -#endif
> -#include <sys/wait.h>
> -#include <sys/types.h>
> -#include <signal.h>
> -#include <stdlib.h>
> -#include <unistd.h>
> -#include <stdio.h>
> -#include <mqueue.h>
> -#include "lapi/syscalls.h"
> -#include "pidns_helper.h"
> -#include "test.h"
> -
> -char *TCID = "pidns31";
> -int TST_TOTAL = 1;
> -
> -char *mqname = "mq1";
> -int result = TFAIL;
> -
> -int father_to_child[2];
> + * Copyright (c) Bull S.A.S. 2008
> + * 01/12/08 Nadia Derbey <Nadia.Derbey@bull.net>
> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
>
> -#define CHILD_PID 1
> -#define PARENT_PID 0
> +/*\
> + * [Description]
> + *
> + * Clone a process with CLONE_NEWPID flag, register notification on a posix
> + * mqueue and send a mqueue message from the child. Then check if signal
> + * notification contains si_pid of the child.
> + */
>
> -#define MSG "HOW ARE YOU"
> -#define MSG_PRIO 1
> +#define _GNU_SOURCE 1
> +#include <signal.h>
> +#include <mqueue.h>
> +#include "tst_test.h"
> +#include "tst_safe_posix_ipc.h"
> +#include "lapi/namespaces_constants.h"
>
> -#define NO_STEP -1
> -#define F_STEP_0 0x00
> -#define F_STEP_1 0x01
> -#define F_STEP_2 0x02
> -#define F_STEP_3 0x03
> -#define C_STEP_0 0x10
> -#define C_STEP_1 0x11
> +#define MQNAME "/LTP_PIDNS30_MQ"
>
> struct notify_info {
> mqd_t mqd;
> pid_t pid;
> };
>
> -static void remove_pipe(int *fd)
> -{
> - close(fd[0]);
> - close(fd[1]);
> -}
> +static mqd_t mqd = -1;
> +static volatile int received;
>
> static void remove_mqueue(mqd_t mqd)
> {
> - mq_close(mqd);
> - tst_syscall(__NR_mq_unlink, mqname);
> -}
> -
> -/*
> - * steps F_STEP_XX : called from main
> - * steps C_STEP_XX : called from child_fn
> - */
> -static void cleanup_resources(int step, mqd_t mqd)
> -{
> - switch (step) {
> - case C_STEP_1:
> - close(father_to_child[0]);
> - /* fall through */
> - case C_STEP_0:
> - mq_close(mqd);
> - break;
> -
> - case F_STEP_3:
> - remove_mqueue(mqd);
> - close(father_to_child[1]);
> - break;
> -
> - case F_STEP_2:
> - tst_syscall(__NR_mq_notify, mqd, NULL);
> - /* fall through */
> - case F_STEP_1:
> - remove_mqueue(mqd);
> - /* fall through */
> - case F_STEP_0:
> - remove_pipe(father_to_child);
> - break;
> - default:
> - tst_resm(TWARN, "Unknown code - no resource removed.");
> - break;
> - }
> -}
> -
> -/*
> - * cleanup_mqueue() - performs all ONE TIME cleanup for this test at
> - * completion or premature exit.
> - * step == -1 means no local resource to remove.
> - */
> -void cleanup_mqueue(int result, int step, mqd_t mqd)
> -{
> - if (step != NO_STEP)
> - cleanup_resources(step, mqd);
> + if (mqd != -1)
> + SAFE_MQ_CLOSE(mqd);
>
> - tst_exit();
> + mq_unlink(MQNAME);
Again we never check this.
> }
>
> -/*
> - * child_fn() - Inside container
> - */
> -int child_fn(void *arg)
> +static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
> {
> - pid_t pid, ppid;
> - mqd_t mqd;
> - char buf[5];
> -
> - /* Set process id and parent pid */
> - pid = getpid();
> - ppid = getppid();
> -
> - if (pid != CHILD_PID || ppid != PARENT_PID) {
> - tst_resm(TBROK, "cinit: pidns is not created");
> - cleanup_mqueue(TBROK, NO_STEP, 0);
> - }
> + struct notify_info *info;
>
> - /* Close the appropriate end of pipe */
> - close(father_to_child[1]);
> + received = 0;
>
> - /* Is parent ready to receive a message? */
> - read(father_to_child[0], buf, 5);
> - if (strcmp(buf, "f:ok")) {
> - tst_resm(TBROK, "cinit: parent did not send the message!");
> - cleanup_mqueue(TBROK, NO_STEP, 0);
> - }
> - tst_resm(TINFO, "cinit: my father is ready to receive a message");
> + tst_res(TINFO, "Received signal %s from pid %d",
> tst_strsig(si->si_signo), si->si_pid);
Again, printing in a signal handler should be avoided. However it's
definitely good to have this information.
>
> - mqd = tst_syscall(__NR_mq_open, mqname, O_WRONLY, 0, NULL);
> - if (mqd == (mqd_t) - 1) {
> - tst_resm(TBROK, "cinit: mq_open() failed (%s)",
> - strerror(errno));
> - cleanup_mqueue(TBROK, NO_STEP, 0);
> - }
> - tst_resm(TINFO, "cinit: mq_open succeeded");
> + if (si->si_signo != SIGUSR1 || si->si_code != SI_MESGQ)
> + return;
>
> - if (mq_send(mqd, MSG, strlen(MSG), MSG_PRIO) == (mqd_t) - 1) {
> - tst_resm(TBROK, "cinit: mq_send() failed (%s)",
> - strerror(errno));
> - cleanup_mqueue(TBROK, C_STEP_0, mqd);
> - }
> - tst_resm(TINFO, "cinit: mq_send() succeeded");
> + info = (struct notify_info*)si->si_ptr;
>
> - /* Cleanup and exit */
> - cleanup_resources(C_STEP_1, mqd);
> - exit(0);
> + if (si->si_pid == info->pid)
> + received++;
> }
>
> -/*
> - * father_signal_handler()
> - */
> -static void father_signal_handler(int sig, siginfo_t * si, void *unused)
> +static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
> {
> - char buf[256];
> - struct mq_attr attr;
> - struct notify_info *info;
> -
> - if (si->si_signo != SIGUSR1) {
> - tst_resm(TBROK, "father: received %s unexpectedly",
> - strsignal(si->si_signo));
> - return;
> - }
> + pid_t cpid, ppid;
> + mqd_t mqd_child;
>
> - if (si->si_code != SI_MESGQ) {
> - tst_resm(TBROK, "father: expected signal code SI_MESGQ - "
> - "Got %d", si->si_code);
> - return;
> - }
> + cpid = getpid();
> + ppid = getppid();
>
> - if (!si->si_ptr) {
> - tst_resm(TBROK, "father: expected si_ptr - Got NULL");
> - return;
> + if (cpid != 1 || ppid != 0) {
> + tst_res(TFAIL, "got unexpected result of cpid=%d ppid=%d", cpid, ppid);
> + return 0;
Again, could use TST_EXP_EQ... and/or put it in a function as it is
repeated 3+ times.
> }
>
> - info = (struct notify_info *)si->si_ptr;
> + TST_CHECKPOINT_WAIT(0);
>
> - if (si->si_pid != info->pid) {
> - tst_resm(TFAIL,
> - "father: expected signal originator PID = %d - Got %d",
> - info->pid, si->si_pid);
> - return;
> - }
> + mqd_child = SAFE_MQ_OPEN(MQNAME, O_WRONLY, 0, NULL);
> + SAFE_MQ_SEND(mqd_child, "pippo", 5, 1);
>
> - tst_resm(TPASS, "father: signal originator PID = %d", si->si_pid);
> - result = TPASS;
> + TST_CHECKPOINT_WAKE(0);
>
> - /*
> - * Now read the message - Be silent on errors since this is not the
> - * test purpose.
> - */
> - if (!mq_getattr(info->mqd, &attr))
> - mq_receive(info->mqd, buf, attr.mq_msgsize, NULL);
> + return 0;
> }
>
> -static void setup(void)
> +static void cleanup(void)
> {
> - tst_require_root();
> - check_newpid();
> + remove_mqueue(mqd);
> }
>
> -/***********************************************************************
> -* M A I N
> -***********************************************************************/
> -
> -int main(void)
> +static void run(void)
> {
> - pid_t cpid;
> - mqd_t mqd;
> - struct sigevent notif;
> + int cpid, status;
> struct sigaction sa;
> - int status;
> + struct sigevent notif;
> struct notify_info info;
>
> - setup();
> -
> - if (pipe(father_to_child) == -1) {
> - tst_resm(TBROK, "parent: pipe() failed. aborting!");
> - cleanup_mqueue(TBROK, NO_STEP, 0);
> - }
> + remove_mqueue(mqd);
>
> - tst_syscall(__NR_mq_unlink, mqname);
> - mqd =
> - tst_syscall(__NR_mq_open, mqname, O_RDWR | O_CREAT | O_EXCL, 0777,
> - NULL);
> - if (mqd == (mqd_t) - 1) {
> - tst_resm(TBROK, "parent: mq_open() failed (%s)",
> - strerror(errno));
> - cleanup_mqueue(TBROK, F_STEP_0, 0);
> - }
> - tst_resm(TINFO, "parent: successfully created posix mqueue");
> + cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
> + if (cpid < 0)
> + tst_brk(TBROK | TERRNO, "clone failed");
>
> - /* container creation on PID namespace */
> - cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
> - if (cpid < 0) {
> - tst_resm(TBROK, "parent: clone() failed(%s)", strerror(errno));
> - cleanup_mqueue(TBROK, F_STEP_1, mqd);
> - }
> - tst_resm(TINFO, "parent: successfully created child (pid = %d)", cpid);
> + mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
>
> - /* Register for notification on message arrival */
> notif.sigev_notify = SIGEV_SIGNAL;
> notif.sigev_signo = SIGUSR1;
> info.mqd = mqd;
> info.pid = cpid;
> notif.sigev_value.sival_ptr = &info;
> - if (tst_syscall(__NR_mq_notify, mqd, ¬if) == (mqd_t) -1) {
> - tst_resm(TBROK, "parent: mq_notify() failed (%s)",
> - strerror(errno));
> - cleanup_mqueue(TBROK, F_STEP_1, mqd);
> - }
> - tst_resm(TINFO, "parent: successfully registered for notification");
>
> - /* Define handler for SIGUSR1 */
> - sa.sa_flags = SA_SIGINFO;
> - sigemptyset(&sa.sa_mask);
> - sa.sa_sigaction = father_signal_handler;
> - if (sigaction(SIGUSR1, &sa, NULL) == -1) {
> - tst_resm(TBROK, "parent: sigaction() failed(%s)",
> - strerror(errno));
> - cleanup_mqueue(TBROK, F_STEP_2, mqd);
> - }
> - tst_resm(TINFO, "parent: successfully registered handler for SIGUSR1");
> + SAFE_MQ_NOTIFY(mqd, ¬if);
>
> - /* Close the appropriate end of pipe */
> - close(father_to_child[0]);
> + sa.sa_flags = SA_SIGINFO;
> + SAFE_SIGEMPTYSET(&sa.sa_mask);
> + sa.sa_sigaction = child_signal_handler;
> + SAFE_SIGACTION(SIGUSR1, &sa, NULL);
>
> - /* Tell the child a message can be sent */
> - if (write(father_to_child[1], "f:ok", 5) != 5) {
> - tst_resm(TBROK, "parent: pipe is broken(%s)", strerror(errno));
> - cleanup_mqueue(TBROK, F_STEP_2, mqd);
> - }
> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>
> - sleep(3);
> + SAFE_WAITPID(cpid, &status, 0);
>
> - /* Wait for child to finish */
> - if (wait(&status) == -1) {
> - tst_resm(TBROK, "parent: wait() failed(%s)", strerror(errno));
> - cleanup_mqueue(TBROK, F_STEP_1, mqd);
> + if (received != 1) {
> + tst_res(TFAIL, "Signal hasn't been received after mqueue
> event");
Again you actually increment received so could be > 1 and we can use
TST_EXP_EQ to simplify things.
> + return;
> }
>
> - cleanup_mqueue(result, F_STEP_3, mqd);
> -
> - tst_exit();
> + tst_res(TPASS, "Signal has been received after mqueue event");
> }
> +
> +static struct tst_test test = {
> + .test_all = run,
> + .cleanup = cleanup,
> + .needs_root = 1,
> + .needs_checkpoints = 1,
> +};
> --
> 2.35.3
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread