diff mbox series

[v3,1/4] lib: Add tst_fd iterator

Message ID 20240115125351.7266-2-chrubis@suse.cz
State Accepted
Headers show
Series Add tst_fd iterator API | expand

Commit Message

Cyril Hrubis Jan. 15, 2024, 12:53 p.m. UTC
Which allows tests to loop over different types of file descriptors

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_fd.h   |  61 +++++++++
 include/tst_test.h |   1 +
 lib/tst_fd.c       | 325 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 387 insertions(+)
 create mode 100644 include/tst_fd.h
 create mode 100644 lib/tst_fd.c

Comments

Petr Vorel Jan. 15, 2024, 11:09 p.m. UTC | #1
Hi,

...
> --- /dev/null
> +++ b/lib/tst_fd.c
> @@ -0,0 +1,325 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
...
> +static void open_eventfd(struct tst_fd *fd)
> +{
> +	fd->fd = eventfd(0, 0);
> +
> +	if (fd->fd < 0) {
> +		tst_res(TCONF | TERRNO,
very nit: this could be on single line, without brackets.
> +			"Skipping %s", tst_fd_desc(fd));
> +	}
> +}
> +
> +static void open_signalfd(struct tst_fd *fd)
> +{
> +	sigset_t sfd_mask;
nit: space here saves checkpatch warning.
> +	sigemptyset(&sfd_mask);
> +
> +	fd->fd = signalfd(-1, &sfd_mask, 0);
> +	if (fd->fd < 0) {
> +		tst_res(TCONF | TERRNO,
> +			"Skipping %s", tst_fd_desc(fd));
> +	}
> +}
> +
> +static void open_timerfd(struct tst_fd *fd)
> +{
> +	fd->fd = timerfd_create(CLOCK_REALTIME, 0);
> +	if (fd->fd < 0) {
> +		tst_res(TCONF | TERRNO,
Here as well.
> +			"Skipping %s", tst_fd_desc(fd));
> +	}
> +}
...

Obviously ready to merge, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr
Petr Vorel Jan. 16, 2024, 12:32 a.m. UTC | #2
Hi all,

[ removed kernel folks to not bother them with our old distros ]

> Obviously ready to merge, thanks!

I'm sorry, there are still 2 build failures [1] even with my attempt to fix it [2]:

Both Leap 42.2 and Ubuntu Bionic fail to build on:

In file included from ../include/lapi/io_uring.h:17:0,
                 from tst_fd.c:21:
/usr/include/sys/mount.h:35:3: error: expected identifier before numeric constant
   MS_RDONLY = 1,  /* Mount read-only.  */
   ^

I found quite useful to have new LTP for these old distros, but if it's too much
pain I'm ok to drop them. I'm not even sure if it's possible to fix it.
The only way to do that would be for these old distros (which cannot combine
these two headers) to also avoid BPF and io_uring. Code below compiles [3], but
I don't think myself it's a good solution even if we polish the check (transform
into m4 macro).

OTOH fsopen haven't been defined in <sys/mount.h> yet in libc (checked: glibc,
musl, uclibc-ng, bionic), thus the effect would be that we'd just use
<linux/mount.h> instead of <sys/mount.h> in lib/*.c. And mount() related tests
which test <sys/mount.h> (or use lapi/mount.h) could not include <linux/mount.h>
nor <linux/io_uring.h> nor <lapi/bpf.h>. Of course, we can't keep these two old
distros too long.

Kind regards,
Petr

[1] https://github.com/pevik/ltp/actions/runs/7534892435
[2] https://lore.kernel.org/ltp/20240105002914.1463989-1-pvorel@suse.cz/
[3] https://github.com/pevik/ltp/actions/runs/7535285434

> Kind regards,
> Petr

diff --git include/lapi/fsmount.h include/lapi/fsmount.h
index 07eb42ffa..a9491a16d 100644
--- include/lapi/fsmount.h
+++ include/lapi/fsmount.h
@@ -11,12 +11,11 @@
 #include "config.h"
 #include <sys/syscall.h>
 #include <sys/types.h>
-#include <sys/mount.h>
 
-#ifndef HAVE_FSOPEN
-# ifdef HAVE_LINUX_MOUNT_H
+#if !defined(HAVE_FSOPEN) && defined(HAVE_LINUX_MOUNT_H)
 # include <linux/mount.h>
-# endif
+#else
+# include <sys/mount.h>
 #endif
 
 #include "lapi/fcntl.h"
diff --git include/tst_fd.h include/tst_fd.h
index 2183ea068..cdf9867e6 100644
--- include/tst_fd.h
+++ include/tst_fd.h
@@ -26,8 +26,10 @@ enum tst_fd_type {
 	TST_FD_INOTIFY,
 	TST_FD_USERFAULTFD,
 	TST_FD_PERF_EVENT,
+#if defined(HAVE_FSOPEN) || !defined(HAVE_LINUX_MOUNT_H)
 	TST_FD_IO_URING,
 	TST_FD_BPF_MAP,
+#endif
 	TST_FD_FSOPEN,
 	TST_FD_FSPICK,
 	TST_FD_OPEN_TREE,
diff --git lib/tst_fd.c lib/tst_fd.c
index b0d6fb1d6..0766328a9 100644
--- lib/tst_fd.c
+++ lib/tst_fd.c
@@ -18,9 +18,12 @@
 #include "tst_safe_macros.h"
 
 #include "lapi/pidfd.h"
+#include "lapi/fsmount.h"
+
+#if defined(HAVE_FSOPEN) || !defined(HAVE_LINUX_MOUNT_H)
 #include "lapi/io_uring.h"
 #include "lapi/bpf.h"
-#include "lapi/fsmount.h"
+#endif
 
 #include "tst_fd.h"
 
@@ -190,6 +193,7 @@ static void open_perf_event(struct tst_fd *fd)
 	}
 }
 
+#if defined(HAVE_FSOPEN) || !defined(HAVE_LINUX_MOUNT_H)
 static void open_io_uring(struct tst_fd *fd)
 {
 	struct io_uring_params uring_params = {};
@@ -216,6 +220,7 @@ static void open_bpf_map(struct tst_fd *fd)
 			"Skipping %s", tst_fd_desc(fd));
 	}
 }
+#endif
 
 static void open_fsopen(struct tst_fd *fd)
 {
@@ -281,8 +286,10 @@ static struct tst_fd_desc fd_desc[] = {
 	[TST_FD_INOTIFY] = {.open_fd = open_inotify, .desc = "inotify"},
 	[TST_FD_USERFAULTFD] = {.open_fd = open_userfaultfd, .desc = "userfaultfd"},
 	[TST_FD_PERF_EVENT] = {.open_fd = open_perf_event, .desc = "perf event"},
+#if defined(HAVE_FSOPEN) || !defined(HAVE_LINUX_MOUNT_H)
 	[TST_FD_IO_URING] = {.open_fd = open_io_uring, .desc = "io uring"},
 	[TST_FD_BPF_MAP] = {.open_fd = open_bpf_map, .desc = "bpf map"},
+#endif
 	[TST_FD_FSOPEN] = {.open_fd = open_fsopen, .desc = "fsopen"},
 	[TST_FD_FSPICK] = {.open_fd = open_fspick, .desc = "fspick"},
 	[TST_FD_OPEN_TREE] = {.open_fd = open_open_tree, .desc = "open_tree"},
Cyril Hrubis Jan. 16, 2024, 3:19 p.m. UTC | #3
Hi!
> In file included from ../include/lapi/io_uring.h:17:0,
>                  from tst_fd.c:21:
> /usr/include/sys/mount.h:35:3: error: expected identifier before numeric constant
>    MS_RDONLY = 1,  /* Mount read-only.  */
>    ^

Looks like the best solution for now is:

diff --git a/include/lapi/io_uring.h b/include/lapi/io_uring.h
index a63741a08..03c45190e 100644
--- a/include/lapi/io_uring.h
+++ b/include/lapi/io_uring.h
@@ -14,7 +14,6 @@
 #include <sys/types.h>
 #include <sys/uio.h>
 #include <stdlib.h>
-#include <linux/fs.h>

 #include "lapi/syscalls.h"


As far as I can tell the only reason this header is included is the
__kernel_rwf_t definition which has a fallback definition just a few
lines below.
Petr Vorel Jan. 16, 2024, 3:31 p.m. UTC | #4
> Hi!
> > In file included from ../include/lapi/io_uring.h:17:0,
> >                  from tst_fd.c:21:
> > /usr/include/sys/mount.h:35:3: error: expected identifier before numeric constant
> >    MS_RDONLY = 1,  /* Mount read-only.  */
> >    ^

> Looks like the best solution for now is:

> diff --git a/include/lapi/io_uring.h b/include/lapi/io_uring.h
> index a63741a08..03c45190e 100644
> --- a/include/lapi/io_uring.h
> +++ b/include/lapi/io_uring.h
> @@ -14,7 +14,6 @@
>  #include <sys/types.h>
>  #include <sys/uio.h>
>  #include <stdlib.h>
> -#include <linux/fs.h>

>  #include "lapi/syscalls.h"


> As far as I can tell the only reason this header is included is the
> __kernel_rwf_t definition which has a fallback definition just a few
> lines below.

Great!
Feel free to merge with my ack before the patchset + whole patchset.

Kind regards,
Petr
Cyril Hrubis Jan. 17, 2024, 3:38 p.m. UTC | #5
Hi!
Thanks everyone for the reviews, pushed.
diff mbox series

Patch

diff --git a/include/tst_fd.h b/include/tst_fd.h
new file mode 100644
index 000000000..2183ea068
--- /dev/null
+++ b/include/tst_fd.h
@@ -0,0 +1,61 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/*
+ * Copyright (C) 2023 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+#ifndef TST_FD_H__
+#define TST_FD_H__
+
+enum tst_fd_type {
+	TST_FD_FILE,
+	TST_FD_PATH,
+	TST_FD_DIR,
+	TST_FD_DEV_ZERO,
+	TST_FD_PROC_MAPS,
+	TST_FD_PIPE_READ,
+	TST_FD_PIPE_WRITE,
+	TST_FD_UNIX_SOCK,
+	TST_FD_INET_SOCK,
+	TST_FD_EPOLL,
+	TST_FD_EVENTFD,
+	TST_FD_SIGNALFD,
+	TST_FD_TIMERFD,
+	TST_FD_PIDFD,
+	TST_FD_FANOTIFY,
+	TST_FD_INOTIFY,
+	TST_FD_USERFAULTFD,
+	TST_FD_PERF_EVENT,
+	TST_FD_IO_URING,
+	TST_FD_BPF_MAP,
+	TST_FD_FSOPEN,
+	TST_FD_FSPICK,
+	TST_FD_OPEN_TREE,
+	TST_FD_MEMFD,
+	TST_FD_MEMFD_SECRET,
+	TST_FD_MAX,
+};
+
+struct tst_fd {
+	enum tst_fd_type type;
+	int fd;
+	/* used by the library, do not touch! */
+	long priv;
+};
+
+#define TST_FD_INIT {.type = TST_FD_FILE, .fd = -1}
+
+/*
+ * Advances the iterator to the next fd type, returns zero at the end.
+ */
+int tst_fd_next(struct tst_fd *fd);
+
+#define TST_FD_FOREACH(fd) \
+	for (struct tst_fd fd = TST_FD_INIT; tst_fd_next(&fd); )
+
+/*
+ * Returns human readable name for the file descriptor type.
+ */
+const char *tst_fd_desc(struct tst_fd *fd);
+
+#endif /* TST_FD_H__ */
diff --git a/include/tst_test.h b/include/tst_test.h
index 0c3171e5b..fda696eeb 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -44,6 +44,7 @@ 
 #include "tst_taint.h"
 #include "tst_memutils.h"
 #include "tst_arch.h"
+#include "tst_fd.h"
 
 /*
  * Reports testcase result.
diff --git a/lib/tst_fd.c b/lib/tst_fd.c
new file mode 100644
index 000000000..b0d6fb1d6
--- /dev/null
+++ b/lib/tst_fd.c
@@ -0,0 +1,325 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/*
+ * Copyright (C) 2023 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+#define TST_NO_DEFAULT_MAIN
+
+#include <sys/epoll.h>
+#include <sys/eventfd.h>
+#include <sys/signalfd.h>
+#include <sys/timerfd.h>
+#include <sys/fanotify.h>
+#include <sys/inotify.h>
+#include <linux/perf_event.h>
+
+#include "tst_test.h"
+#include "tst_safe_macros.h"
+
+#include "lapi/pidfd.h"
+#include "lapi/io_uring.h"
+#include "lapi/bpf.h"
+#include "lapi/fsmount.h"
+
+#include "tst_fd.h"
+
+struct tst_fd_desc {
+	void (*open_fd)(struct tst_fd *fd);
+	void (*destroy)(struct tst_fd *fd);
+	const char *desc;
+};
+
+static void open_file(struct tst_fd *fd)
+{
+	fd->fd = SAFE_OPEN("fd_file", O_RDWR | O_CREAT, 0666);
+	SAFE_UNLINK("fd_file");
+}
+
+static void open_path(struct tst_fd *fd)
+{
+	int tfd;
+
+	tfd = SAFE_CREAT("fd_file", 0666);
+	SAFE_CLOSE(tfd);
+
+	fd->fd = SAFE_OPEN("fd_file", O_PATH);
+
+	SAFE_UNLINK("fd_file");
+}
+
+static void open_dir(struct tst_fd *fd)
+{
+	SAFE_MKDIR("fd_dir", 0700);
+	fd->fd = SAFE_OPEN("fd_dir", O_DIRECTORY);
+	SAFE_RMDIR("fd_dir");
+}
+
+static void open_dev_zero(struct tst_fd *fd)
+{
+	fd->fd = SAFE_OPEN("/dev/zero", O_RDONLY);
+}
+
+static void open_proc_self_maps(struct tst_fd *fd)
+{
+	fd->fd = SAFE_OPEN("/proc/self/maps", O_RDONLY);
+}
+
+static void open_pipe_read(struct tst_fd *fd)
+{
+	int pipe[2];
+
+	SAFE_PIPE(pipe);
+	fd->fd = pipe[0];
+	fd->priv = pipe[1];
+}
+
+static void open_pipe_write(struct tst_fd *fd)
+{
+	int pipe[2];
+
+	SAFE_PIPE(pipe);
+	fd->fd = pipe[1];
+	fd->priv = pipe[0];
+}
+
+static void destroy_pipe(struct tst_fd *fd)
+{
+	SAFE_CLOSE(fd->priv);
+}
+
+static void open_unix_sock(struct tst_fd *fd)
+{
+	fd->fd = SAFE_SOCKET(AF_UNIX, SOCK_STREAM, 0);
+}
+
+static void open_inet_sock(struct tst_fd *fd)
+{
+	fd->fd = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
+}
+
+static void open_epoll(struct tst_fd *fd)
+{
+	fd->fd = epoll_create(1);
+
+	if (fd->fd < 0)
+		tst_brk(TBROK | TERRNO, "epoll_create()");
+}
+
+static void open_eventfd(struct tst_fd *fd)
+{
+	fd->fd = eventfd(0, 0);
+
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_signalfd(struct tst_fd *fd)
+{
+	sigset_t sfd_mask;
+	sigemptyset(&sfd_mask);
+
+	fd->fd = signalfd(-1, &sfd_mask, 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_timerfd(struct tst_fd *fd)
+{
+	fd->fd = timerfd_create(CLOCK_REALTIME, 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_pidfd(struct tst_fd *fd)
+{
+	fd->fd = pidfd_open(getpid(), 0);
+	if (fd->fd < 0)
+		tst_brk(TBROK | TERRNO, "pidfd_open()");
+}
+
+static void open_fanotify(struct tst_fd *fd)
+{
+	fd->fd = fanotify_init(FAN_CLASS_NOTIF, O_RDONLY);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_inotify(struct tst_fd *fd)
+{
+	fd->fd = inotify_init();
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_userfaultfd(struct tst_fd *fd)
+{
+	fd->fd = syscall(__NR_userfaultfd, 0);
+
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_perf_event(struct tst_fd *fd)
+{
+	struct perf_event_attr pe_attr = {
+		.type = PERF_TYPE_SOFTWARE,
+		.size = sizeof(struct perf_event_attr),
+		.config = PERF_COUNT_SW_CPU_CLOCK,
+		.disabled = 1,
+		.exclude_kernel = 1,
+		.exclude_hv = 1,
+	};
+
+	fd->fd = syscall(__NR_perf_event_open, &pe_attr, 0, -1, -1, 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_io_uring(struct tst_fd *fd)
+{
+	struct io_uring_params uring_params = {};
+
+	fd->fd = io_uring_setup(1, &uring_params);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_bpf_map(struct tst_fd *fd)
+{
+	union bpf_attr array_attr = {
+		.map_type = BPF_MAP_TYPE_ARRAY,
+		.key_size = 4,
+		.value_size = 8,
+		.max_entries = 1,
+	};
+
+	fd->fd = bpf(BPF_MAP_CREATE, &array_attr, sizeof(array_attr));
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_fsopen(struct tst_fd *fd)
+{
+	fd->fd = fsopen("ext2", 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_fspick(struct tst_fd *fd)
+{
+	fd->fd = fspick(AT_FDCWD, "/", 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_open_tree(struct tst_fd *fd)
+{
+	fd->fd = open_tree(AT_FDCWD, "/", 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_memfd(struct tst_fd *fd)
+{
+	fd->fd = syscall(__NR_memfd_create, "ltp_memfd", 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static void open_memfd_secret(struct tst_fd *fd)
+{
+	fd->fd = syscall(__NR_memfd_secret, 0);
+	if (fd->fd < 0) {
+		tst_res(TCONF | TERRNO,
+			"Skipping %s", tst_fd_desc(fd));
+	}
+}
+
+static struct tst_fd_desc fd_desc[] = {
+	[TST_FD_FILE] = {.open_fd = open_file, .desc = "file"},
+	[TST_FD_PATH] = {.open_fd = open_path, .desc = "O_PATH file"},
+	[TST_FD_DIR] = {.open_fd = open_dir, .desc = "directory"},
+	[TST_FD_DEV_ZERO] = {.open_fd = open_dev_zero, .desc = "/dev/zero"},
+	[TST_FD_PROC_MAPS] = {.open_fd = open_proc_self_maps, .desc = "/proc/self/maps"},
+	[TST_FD_PIPE_READ] = {.open_fd = open_pipe_read, .desc = "pipe read end", .destroy = destroy_pipe},
+	[TST_FD_PIPE_WRITE] = {.open_fd = open_pipe_write, .desc = "pipe write end", .destroy = destroy_pipe},
+	[TST_FD_UNIX_SOCK] = {.open_fd = open_unix_sock, .desc = "unix socket"},
+	[TST_FD_INET_SOCK] = {.open_fd = open_inet_sock, .desc = "inet socket"},
+	[TST_FD_EPOLL] = {.open_fd = open_epoll, .desc = "epoll"},
+	[TST_FD_EVENTFD] = {.open_fd = open_eventfd, .desc = "eventfd"},
+	[TST_FD_SIGNALFD] = {.open_fd = open_signalfd, .desc = "signalfd"},
+	[TST_FD_TIMERFD] = {.open_fd = open_timerfd, .desc = "timerfd"},
+	[TST_FD_PIDFD] = {.open_fd = open_pidfd, .desc = "pidfd"},
+	[TST_FD_FANOTIFY] = {.open_fd = open_fanotify, .desc = "fanotify"},
+	[TST_FD_INOTIFY] = {.open_fd = open_inotify, .desc = "inotify"},
+	[TST_FD_USERFAULTFD] = {.open_fd = open_userfaultfd, .desc = "userfaultfd"},
+	[TST_FD_PERF_EVENT] = {.open_fd = open_perf_event, .desc = "perf event"},
+	[TST_FD_IO_URING] = {.open_fd = open_io_uring, .desc = "io uring"},
+	[TST_FD_BPF_MAP] = {.open_fd = open_bpf_map, .desc = "bpf map"},
+	[TST_FD_FSOPEN] = {.open_fd = open_fsopen, .desc = "fsopen"},
+	[TST_FD_FSPICK] = {.open_fd = open_fspick, .desc = "fspick"},
+	[TST_FD_OPEN_TREE] = {.open_fd = open_open_tree, .desc = "open_tree"},
+	[TST_FD_MEMFD] = {.open_fd = open_memfd, .desc = "memfd"},
+	[TST_FD_MEMFD_SECRET] = {.open_fd = open_memfd_secret, .desc = "memfd secret"},
+};
+
+const char *tst_fd_desc(struct tst_fd *fd)
+{
+	if (fd->type >= ARRAY_SIZE(fd_desc))
+		return "invalid";
+
+	return fd_desc[fd->type].desc;
+}
+
+int tst_fd_next(struct tst_fd *fd)
+{
+	size_t len = ARRAY_SIZE(fd_desc);
+
+	if (fd->fd >= 0) {
+		SAFE_CLOSE(fd->fd);
+
+		if (fd_desc[fd->type].destroy)
+			fd_desc[fd->type].destroy(fd);
+
+		fd->type++;
+	}
+
+	for (;;) {
+		if (fd->type >= len)
+			return 0;
+
+		fd_desc[fd->type].open_fd(fd);
+
+		if (fd->fd >= 0)
+			return 1;
+
+		fd->type++;
+	}
+}