[LTP] [PATCH v2 1/4] lib: Add tst_fd iterator
Petr Vorel
pvorel@suse.cz
Fri Jan 5 01:42:36 CET 2024
Hi Cyril,
> Which allows tests to loop over different types of file descriptors
Nice API, thanks!
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> include/tst_fd.h | 61 +++++++++
> include/tst_test.h | 1 +
> lib/tst_fd.c | 331 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 393 insertions(+)
> create mode 100644 include/tst_fd.h
> create mode 100644 lib/tst_fd.c
> diff --git a/include/tst_fd.h b/include/tst_fd.h
> new file mode 100644
> index 000000000..2f15a06c8
> --- /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 75c2109b9..5eee36bac 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..3e0a0fe20
> --- /dev/null
> +++ b/lib/tst_fd.c
> @@ -0,0 +1,331 @@
> +// 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"
centos stream 9 (glibc 2.34)
https://github.com/pevik/ltp/actions/runs/7415994730/job/20180154319
In file included from /usr/include/linux/fs.h:19,
from /__w/ltp/ltp/include/lapi/io_uring.h:17,
from /__w/ltp/ltp/lib/tst_fd.c:21:
/usr/include/x86_64-linux-gnu/sys/mount.h:35:3: error: expected identifier before numeric constant
35 | MS_RDONLY = 1, /* Mount read-only. */
| ^~~~~~~~~
CC lib/tst_fill_file.o
make[1]: *** [/__w/ltp/ltp/include/mk/rules.mk:15: tst_fd.o] Error 1
make[1]: *** Waiting for unfinished jobs....
https://sourceware.org/glibc/wiki/Synchronizing_Headers
does mention conflict between <linux/mount.h> and <sys/mount.h>,
and that's what happen - <linux/fs.h> includes <linux/mount.h>.
I send a fix for this which should be applied before the release:
https://patchwork.ozlabs.org/project/ltp/patch/20240105002914.1463989-1-pvorel@suse.cz/
It fixes most of the distros:
https://github.com/pevik/ltp/actions/runs/7416413061/job/20181348475
But unfortunately it fails on one distro: Ubuntu Bionic (glibc 2.27):
https://github.com/pevik/ltp/actions/runs/7416413061/job/20181348475
In file included from ../include/lapi/io_uring.h:17:0,
from tst_fd.c:21:
/usr/include/x86_64-linux-gnu/sys/mount.h:35:3: error: expected identifier before numeric constant
MS_RDONLY = 1, /* Mount read-only. */
^
../include/mk/rules.mk:15: recipe for target 'tst_fd.o' failed
I'm not sure if we can fix it. Somebody tried to fix it for QEMU:
https://lore.kernel.org/qemu-devel/20220802164134.1851910-1-berrange@redhat.com/
which got later deleted due accepted glibc fix:
https://lore.kernel.org/qemu-devel/20231109135933.1462615-46-mjt@tls.msk.ru/
Maybe it's time to drop Ubuntu Bionic? We have Leap 42.2, which is the oldest
distro we care and it works on it (probably it does not have HAVE_FSOPEN
defined).
There is yet another error for very old distros ie. old Leap 42.2 (glibc 2.22),
probably missing fallback definitions?
https://github.com/pevik/ltp/actions/runs/7415994730/job/20180153354
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. */
^
tst_fd.c: In function 'open_io_uring':
tst_fd.c:195:9: warning: missing initializer for field 'sq_entries' of 'struct io_uring_params' [-Wmissing-field-initializers]
struct io_uring_params uring_params = {};
^
In file included from tst_fd.c:21:0:
../include/lapi/io_uring.h:198:11: note: 'sq_entries' declared here
uint32_t sq_entries;
^
tst_fd.c: In function 'open_bpf_map':
tst_fd.c:208:3: warning: missing initializer for field 'key_size' of 'struct <anonymous>' [-Wmissing-field-initializers]
.key_size = 4,
^
In file included from tst_fd.c:22:0:
../include/lapi/bpf.h:185:12: note: 'key_size' declared here
uint32_t key_size; /* size of key in bytes */
^
tst_fd.c:209:3: warning: missing initializer for field 'value_size' of 'struct <anonymous>' [-Wmissing-field-initializers]
.value_size = 8,
^
In file included from tst_fd.c:22:0:
../include/lapi/bpf.h:186:12: note: 'value_size' declared here
uint32_t value_size; /* size of value in bytes */
^
tst_fd.c:210:3: warning: missing initializer for field 'max_entries' of 'struct <anonymous>' [-Wmissing-field-initializers]
.max_entries = 1,
^
In file included from tst_fd.c:22:0:
../include/lapi/bpf.h:187:12: note: 'max_entries' declared here
uint32_t max_entries; /* max number of entries in a map */
^
tst_fd.c:211:2: warning: missing initializer for field 'map_flags' of 'struct <anonymous>' [-Wmissing-field-initializers]
};
^
In file included from tst_fd.c:22:0:
../include/lapi/bpf.h:188:12: note: 'map_flags' declared here
uint32_t map_flags; /* BPF_MAP_CREATE related
^
make[1]: *** [tst_fd.o] Error 1
../include/mk/rules.mk:15: recipe for target 'tst_fd.o' failed
> +#include "lapi/bpf.h"
> +#include "lapi/fsmount.h"
> +
> +#include "tst_fd.h"
...
> +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));
Why there is sometimes TCONF? Permissions? I would expect some check which would
determine whether TCONF or TBROK. Again, I suppose you'll be able to check, when
TST_EXP_FAIL() merged, right?
https://lore.kernel.org/ltp/20240103115700.14585-1-chrubis@suse.cz/
If not, some local macro which would wrap error handling would be useful.
> + }
> +}
> +
> +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);
FYI we have safe_fanotify_init(), which checks for ENOSYS.
> + 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);
Wouldn't be safe to use tst_syscall() ?
> +
> + if (fd->fd < 0) {
> + tst_res(TCONF | TERRNO,
> + "Skipping %s", tst_fd_desc(fd));
> + }
> +}
...
> + [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";
Maybe use assert() instead?
> +
> + return fd_desc[fd->type].desc;
> +}
> +
> +void tst_fd_init(struct tst_fd *fd)
This is not in tst_fd.h, thus check complains about not static.
> +{
> + fd->type = TST_FD_FILE;
> + fd->fd = -1;
> +}
...
Kind regards,
Petr
More information about the ltp
mailing list