| Message ID | 20251209211629.95436-1-terry.tritton@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ioctl_pidfd02-06: Add CONFIG_USER_NS and CONFIG_PID_NS to needs_kconfigs | expand |
| Context | Check | Description |
|---|---|---|
| ltpci/alpine_latest_gcc | fail | failure |
| ltpci/ubuntu_jammy_gcc | fail | failure |
| ltpci/fedora_latest_clang | fail | failure |
| ltpci/debian_oldstable_gcc | fail | failure |
| ltpci/debian_stable_gcc | fail | failure |
| ltpci/debian_oldstable_clang | fail | failure |
| ltpci/ubuntu_bionic_gcc | fail | failure |
| ltpci/debian_testing_gcc | fail | failure |
| ltpci/debian_testing_clang | fail | failure |
| ltpci/debian_stable_gcc | fail | failure |
| ltpci/debian_stable_powerpc64le-linux-gnu-gcc_ppc64el | fail | failure |
| ltpci/debian_stable_aarch64-linux-gnu-gcc_arm64 | fail | failure |
| ltpci/debian_stable_s390x-linux-gnu-gcc_s390x | fail | failure |
| ltpci/quay-io-centos-centos_stream9_gcc | fail | failure |
| ltpci/opensuse-leap_latest_gcc | fail | failure |
| ltpci/opensuse-archive_42-2_gcc | fail | failure |
Hi!
Obviously correct, thanks.
Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>
Hi all, > The flags CLONE_NEWUSER and CLONE_NEWPID require specific namespace support. > Add CONFIG_USER_NS and CONFIG_PID_NS to needs_kconfigs so these tests return > TCONF instead of failing. > Signed-off-by: Terry Tritton <terry.tritton@linaro.org> > --- > testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c | 5 +++++ > testcases/kernel/syscalls/ioctl/ioctl_pidfd03.c | 5 +++++ > testcases/kernel/syscalls/ioctl/ioctl_pidfd04.c | 5 +++++ > testcases/kernel/syscalls/ioctl/ioctl_pidfd05.c | 5 +++++ > testcases/kernel/syscalls/ioctl/ioctl_pidfd06.c | 5 +++++ > 5 files changed, 25 insertions(+) > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c > index 7eb60e7fc..6983259e4 100644 > --- a/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c > +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c > @@ -81,5 +81,10 @@ static struct tst_test test = { > {&info0, .size = sizeof(*info0)}, > {&info1, .size = sizeof(*info1)}, > {} > + }, > + .needs_kconfigs = (const char *[]) { > + "CONFIG_USER_NS", > + "CONFIG_PID_NS", How about to check /proc/self/ns/user and /proc/self/ns/pid as ioctl_ns06.c does? int exists = access("/proc/self/ns/user", F_OK); if (exists < 0) tst_res(TCONF, "namespace not available"); Long time ago we tried to avoid forcing config. Is it now considered as better? (maybe more readable?) Or we would keep checking /proc (or /sys) but add a comment for required functions? Kind regards, Petr > + NULL
Hi, On Mon Dec 15, 2025 at 4:53 PM CET, Petr Vorel wrote: > Hi all, > > > The flags CLONE_NEWUSER and CLONE_NEWPID require specific namespace support. > > Add CONFIG_USER_NS and CONFIG_PID_NS to needs_kconfigs so these tests return > > TCONF instead of failing. > > > Signed-off-by: Terry Tritton <terry.tritton@linaro.org> > > --- > > testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c | 5 +++++ > > testcases/kernel/syscalls/ioctl/ioctl_pidfd03.c | 5 +++++ > > testcases/kernel/syscalls/ioctl/ioctl_pidfd04.c | 5 +++++ > > testcases/kernel/syscalls/ioctl/ioctl_pidfd05.c | 5 +++++ > > testcases/kernel/syscalls/ioctl/ioctl_pidfd06.c | 5 +++++ > > 5 files changed, 25 insertions(+) > > > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c > > index 7eb60e7fc..6983259e4 100644 > > --- a/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c > > +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c > > @@ -81,5 +81,10 @@ static struct tst_test test = { > > {&info0, .size = sizeof(*info0)}, > > {&info1, .size = sizeof(*info1)}, > > {} > > + }, > > + .needs_kconfigs = (const char *[]) { > > + "CONFIG_USER_NS", > > + "CONFIG_PID_NS", > > How about to check /proc/self/ns/user and /proc/self/ns/pid as ioctl_ns06.c > does? > > int exists = access("/proc/self/ns/user", F_OK); > > if (exists < 0) > tst_res(TCONF, "namespace not available"); > > Long time ago we tried to avoid forcing config. Is it now considered as better? > (maybe more readable?) Or we would keep checking /proc (or /sys) but add a > comment for required functions? This case is specific to the CONFIG_PID_NS/CONFIG_USER_NS configurations and the feature can't be tested if kernel is not configured with them. Manual is clear about it: https://www.man7.org/linux/man-pages/man7/pid_namespaces.7.html > > Kind regards, > Petr > > > + NULL
Hi, ... > > > +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c > > > @@ -81,5 +81,10 @@ static struct tst_test test = { > > > {&info0, .size = sizeof(*info0)}, > > > {&info1, .size = sizeof(*info1)}, > > > {} > > > + }, > > > + .needs_kconfigs = (const char *[]) { > > > + "CONFIG_USER_NS", > > > + "CONFIG_PID_NS", > > How about to check /proc/self/ns/user and /proc/self/ns/pid as ioctl_ns06.c > > does? > > int exists = access("/proc/self/ns/user", F_OK); > > if (exists < 0) > > tst_res(TCONF, "namespace not available"); > > Long time ago we tried to avoid forcing config. Is it now considered as better? > > (maybe more readable?) Or we would keep checking /proc (or /sys) but add a > > comment for required functions? > This case is specific to the CONFIG_PID_NS/CONFIG_USER_NS configurations > and the feature can't be tested if kernel is not configured with them. > Manual is clear about it: https://www.man7.org/linux/man-pages/man7/pid_namespaces.7.html And https://www.man7.org/linux/man-pages/man7/user_namespaces.7.html. Yeah, I understand that. The dependency of CLONE_NEWUSER/CLONE_NEWPID is also visible in kernel sources (e.g. fs/nsfs.c). But my question was different: Do we now prefer everything kind of document with .needs_kconfigs, even it's possible to detect it otherwise? (speed of parsing kconfig, kind of hard request for kconfig being available even we can figure the support otherwise). And if we decide for forcing kconfig, we should update ioctl_ns06.c, which does /proc based detection (i.e. to use the same approach). Kind regards, Petr
> And https://www.man7.org/linux/man-pages/man7/user_namespaces.7.html. > > Yeah, I understand that. The dependency of CLONE_NEWUSER/CLONE_NEWPID is also > visible in kernel sources (e.g. fs/nsfs.c). But my question was different: > Do we now prefer everything kind of document with .needs_kconfigs, even it's > possible to detect it otherwise? (speed of parsing kconfig, kind of hard request > for kconfig being available even we can figure the support otherwise). I believe we shouldn't see this as black/white but use this feature when it's really needed. This is the case. > > And if we decide for forcing kconfig, we should update ioctl_ns06.c, which does > /proc based detection (i.e. to use the same approach). I didn't check this, but I'm pretty sure we should go all around and verify many other tests with the same issue. We should do it in this patch-set or on a searate one. > > Kind regards, > Petr
> > And https://www.man7.org/linux/man-pages/man7/user_namespaces.7.html. > > Yeah, I understand that. The dependency of CLONE_NEWUSER/CLONE_NEWPID is also > > visible in kernel sources (e.g. fs/nsfs.c). But my question was different: > > Do we now prefer everything kind of document with .needs_kconfigs, even it's > > possible to detect it otherwise? (speed of parsing kconfig, kind of hard request > > for kconfig being available even we can figure the support otherwise). > I believe we shouldn't see this as black/white but use this feature when > it's really needed. This is the case. Sure, .needs_kconfigs is used when test request some functionality based on kconfig. But many tests use /proc or /sys based detection (e.g. ioctl_ns06.c) or based on certain errno, see include/lapi/syscalls.h or testcases/kernel/syscalls/fanotify/fanotify.h) because these were added before LTP supported kconfig. Later, when kconfig was added it was considering as a last resort (when there was no way to detect dependency otherwise). Have we decide to move everything into kconfig? I'm not sure myself. needs_kconfigs is simpler and obvious, but it requires kernel config. I suppose the speed of parsing config is not an issue. It'd be nice to mention the resolution (preferred vs. only if no other way to detect the support) into https://linux-test-project.readthedocs.io/en/latest/developers/writing_tests.html or into upcommig doc/developers/ground_rules.rst https://patchwork.ozlabs.org/project/ltp/patch/20251215124404.16395-2-chrubis@suse.cz/ Kind regards, Petr > > And if we decide for forcing kconfig, we should update ioctl_ns06.c, which does > > /proc based detection (i.e. to use the same approach). > I didn't check this, but I'm pretty sure we should go all around and > verify many other tests with the same issue. We should do it in this > patch-set or on a searate one. > > Kind regards, > > Petr
Hi, On Mon Dec 15, 2025 at 5:52 PM CET, Petr Vorel wrote: > > > And https://www.man7.org/linux/man-pages/man7/user_namespaces.7.html. > > > > Yeah, I understand that. The dependency of CLONE_NEWUSER/CLONE_NEWPID is also > > > visible in kernel sources (e.g. fs/nsfs.c). But my question was different: > > > Do we now prefer everything kind of document with .needs_kconfigs, even it's > > > possible to detect it otherwise? (speed of parsing kconfig, kind of hard request > > > for kconfig being available even we can figure the support otherwise). > > > I believe we shouldn't see this as black/white but use this feature when > > it's really needed. This is the case. > > Sure, .needs_kconfigs is used when test request some functionality based on > kconfig. But many tests use /proc or /sys based detection (e.g. ioctl_ns06.c) > or based on certain errno, see include/lapi/syscalls.h or > testcases/kernel/syscalls/fanotify/fanotify.h) because these were > added before LTP supported kconfig. Later, when kconfig was added it was > considering as a last resort (when there was no way to detect dependency > otherwise). > > Have we decide to move everything into kconfig? > > I'm not sure myself. needs_kconfigs is simpler and obvious, but it requires > kernel config. I suppose the speed of parsing config is not an issue. > > It'd be nice to mention the resolution (preferred vs. only if no other way to > detect the support) into > https://linux-test-project.readthedocs.io/en/latest/developers/writing_tests.html Feel free to add this comment, but for me it's obvious that if a feature can't be present in the kernel due to kconfigs we should check kconfig :-) > > or into upcommig doc/developers/ground_rules.rst > https://patchwork.ozlabs.org/project/ltp/patch/20251215124404.16395-2-chrubis@suse.cz/ > > Kind regards, > Petr > > > > And if we decide for forcing kconfig, we should update ioctl_ns06.c, which does > > > /proc based detection (i.e. to use the same approach). > > > I didn't check this, but I'm pretty sure we should go all around and > > verify many other tests with the same issue. We should do it in this > > patch-set or on a searate one. > > > > > Kind regards, > > > Petr So what we do with the patch? Should we merge it?
Hi, > On Mon Dec 15, 2025 at 5:52 PM CET, Petr Vorel wrote: > > > > And https://www.man7.org/linux/man-pages/man7/user_namespaces.7.html. > > > > > > Yeah, I understand that. The dependency of CLONE_NEWUSER/CLONE_NEWPID is also > > > > visible in kernel sources (e.g. fs/nsfs.c). But my question was different: > > > > Do we now prefer everything kind of document with .needs_kconfigs, even it's > > > > possible to detect it otherwise? (speed of parsing kconfig, kind of hard request > > > > for kconfig being available even we can figure the support otherwise). > > > > > I believe we shouldn't see this as black/white but use this feature when > > > it's really needed. This is the case. > > > > Sure, .needs_kconfigs is used when test request some functionality based on > > kconfig. But many tests use /proc or /sys based detection (e.g. ioctl_ns06.c) > > or based on certain errno, see include/lapi/syscalls.h or > > testcases/kernel/syscalls/fanotify/fanotify.h) because these were > > added before LTP supported kconfig. Later, when kconfig was added it was > > considering as a last resort (when there was no way to detect dependency > > otherwise). > > > > Have we decide to move everything into kconfig? > > > > I'm not sure myself. needs_kconfigs is simpler and obvious, but it requires > > kernel config. I suppose the speed of parsing config is not an issue. > > > > It'd be nice to mention the resolution (preferred vs. only if no other way to > > detect the support) into > > https://linux-test-project.readthedocs.io/en/latest/developers/writing_tests.html > > Feel free to add this comment, but for me it's obvious that if a > feature can't be present in the kernel due to kconfigs we should check > kconfig :-) I've just taken another look at this and it appears the test would still fail if the config is not present or if KCONFIG_SKIP_CHECK is set, in which case perhaps the run time detection may be preferred as it will still work in these cases? I'm not sure how common either of those cases are though? Would it be better to have the run time detection in tst_kconfig_check as a fall back in case the config is not present? Then the tests can just define the needs_kconfigs and not have to worry about other checks. > > or into upcommig doc/developers/ground_rules.rst > > https://patchwork.ozlabs.org/project/ltp/patch/20251215124404.16395-2-chrubis@suse.cz/ > > > > Kind regards, > > Petr > > > > > > And if we decide for forcing kconfig, we should update ioctl_ns06.c, which does > > > > /proc based detection (i.e. to use the same approach). > > > > > I didn't check this, but I'm pretty sure we should go all around and > > > verify many other tests with the same issue. We should do it in this > > > patch-set or on a searate one. > > > > > > > > Kind regards, > > > > Petr > > So what we do with the patch? Should we merge it? I'm happy to go through and update other tests in this patch or another. Thanks Terry
> Hi, > > On Mon Dec 15, 2025 at 5:52 PM CET, Petr Vorel wrote: > > > > > And https://www.man7.org/linux/man-pages/man7/user_namespaces.7.html. > > > > > Yeah, I understand that. The dependency of CLONE_NEWUSER/CLONE_NEWPID is also > > > > > visible in kernel sources (e.g. fs/nsfs.c). But my question was different: > > > > > Do we now prefer everything kind of document with .needs_kconfigs, even it's > > > > > possible to detect it otherwise? (speed of parsing kconfig, kind of hard request > > > > > for kconfig being available even we can figure the support otherwise). > > > > I believe we shouldn't see this as black/white but use this feature when > > > > it's really needed. This is the case. > > > Sure, .needs_kconfigs is used when test request some functionality based on > > > kconfig. But many tests use /proc or /sys based detection (e.g. ioctl_ns06.c) > > > or based on certain errno, see include/lapi/syscalls.h or > > > testcases/kernel/syscalls/fanotify/fanotify.h) because these were > > > added before LTP supported kconfig. Later, when kconfig was added it was > > > considering as a last resort (when there was no way to detect dependency > > > otherwise). > > > Have we decide to move everything into kconfig? > > > I'm not sure myself. needs_kconfigs is simpler and obvious, but it requires > > > kernel config. I suppose the speed of parsing config is not an issue. > > > It'd be nice to mention the resolution (preferred vs. only if no other way to > > > detect the support) into > > > https://linux-test-project.readthedocs.io/en/latest/developers/writing_tests.html > > Feel free to add this comment, but for me it's obvious that if a > > feature can't be present in the kernel due to kconfigs we should check > > kconfig :-) > I've just taken another look at this and it appears the test would still > fail if the config is not present or if KCONFIG_SKIP_CHECK is set, in > which case perhaps the run time detection may be preferred as it will > still work in these cases? Maybe I'm missing something obvious. IMHO kconfig check for CONFIG_USER_NS and CONFIG_PID_NS is equal to checking for /proc/self/ns/user and /proc/self/ns/pid. "runtime detection" you mean looking at /proc, but actually both ways are "runtime detections" because LTP expects correct kernel config for running kernel or looks into /proc/config.gz. Just looking at /proc is faster and works without KCONFIG_SKIP_CHECK. OTOH kconfig is somehow documenting it. It could be documented in normal test doc if we inspect /proc. @Li @Cyril, I'm sorry to raise this again (I haven't found a thread in which we discussed it last time). Which one do we prefer in case both can be used? > I'm not sure how common either of those cases are though? > Would it be better to have the run time detection in tst_kconfig_check > as a fall back in case the config is not present? > Then the tests can just define the needs_kconfigs and not have to worry > about other checks. Using both looks to me overkill. Kind regards, Petr > > > or into upcommig doc/developers/ground_rules.rst > > > https://patchwork.ozlabs.org/project/ltp/patch/20251215124404.16395-2-chrubis@suse.cz/ > > > Kind regards, > > > Petr > > > > > And if we decide for forcing kconfig, we should update ioctl_ns06.c, which does > > > > > /proc based detection (i.e. to use the same approach). > > > > I didn't check this, but I'm pretty sure we should go all around and > > > > verify many other tests with the same issue. We should do it in this > > > > patch-set or on a searate one. > > > > > Kind regards, > > > > > Petr > > So what we do with the patch? Should we merge it? > I'm happy to go through and update other tests in this patch or another. > Thanks > Terry
Hi! > > > Sure, .needs_kconfigs is used when test request some functionality based on > > > kconfig. But many tests use /proc or /sys based detection (e.g. ioctl_ns06.c) > > > or based on certain errno, see include/lapi/syscalls.h or > > > testcases/kernel/syscalls/fanotify/fanotify.h) because these were > > > added before LTP supported kconfig. Later, when kconfig was added it was > > > considering as a last resort (when there was no way to detect dependency > > > otherwise). > > > > > > Have we decide to move everything into kconfig? > > > > > > I'm not sure myself. needs_kconfigs is simpler and obvious, but it requires > > > kernel config. I suppose the speed of parsing config is not an issue. > > > > > > It'd be nice to mention the resolution (preferred vs. only if no other way to > > > detect the support) into > > > https://linux-test-project.readthedocs.io/en/latest/developers/writing_tests.html > > > > Feel free to add this comment, but for me it's obvious that if a > > feature can't be present in the kernel due to kconfigs we should check > > kconfig :-) > > I've just taken another look at this and it appears the test would still > fail if the config is not present or if KCONFIG_SKIP_CHECK is set, in > which case perhaps the run time detection may be preferred as it will > still work in these cases? The KCONFIG_SKIP_CHECK is a flag aimed at developers, it shouldn't be enabled in production testing. As for the missing config there is 95 testcases that have needs_kconfigs set at this moment and the number is growing steadily. I would argue that you cannot run LTP without having config available. And the config location is autodetected on common distributions as well. > Would it be better to have the run time detection in tst_kconfig_check > as a fall back in case the config is not present? > Then the tests can just define the needs_kconfigs and not have to worry > about other checks. I would avoid any complexity that isn't strictly necessary, the less we do, the less breakage we have to deal with later. In that sense adding the needs_kconfigs and expect the config to be there is probably the most straightforward solution.
> Hi! > > > > Sure, .needs_kconfigs is used when test request some functionality based on > > > > kconfig. But many tests use /proc or /sys based detection (e.g. ioctl_ns06.c) > > > > or based on certain errno, see include/lapi/syscalls.h or > > > > testcases/kernel/syscalls/fanotify/fanotify.h) because these were > > > > added before LTP supported kconfig. Later, when kconfig was added it was > > > > considering as a last resort (when there was no way to detect dependency > > > > otherwise). > > > > Have we decide to move everything into kconfig? > > > > I'm not sure myself. needs_kconfigs is simpler and obvious, but it requires > > > > kernel config. I suppose the speed of parsing config is not an issue. > > > > It'd be nice to mention the resolution (preferred vs. only if no other way to > > > > detect the support) into > > > > https://linux-test-project.readthedocs.io/en/latest/developers/writing_tests.html > > > Feel free to add this comment, but for me it's obvious that if a > > > feature can't be present in the kernel due to kconfigs we should check > > > kconfig :-) > > I've just taken another look at this and it appears the test would still > > fail if the config is not present or if KCONFIG_SKIP_CHECK is set, in > > which case perhaps the run time detection may be preferred as it will > > still work in these cases? > The KCONFIG_SKIP_CHECK is a flag aimed at developers, it shouldn't be > enabled in production testing. > As for the missing config there is 95 testcases that have needs_kconfigs > set at this moment and the number is growing steadily. I would argue > that you cannot run LTP without having config available. And the config > location is autodetected on common distributions as well. > > Would it be better to have the run time detection in tst_kconfig_check > > as a fall back in case the config is not present? > > Then the tests can just define the needs_kconfigs and not have to worry > > about other checks. > I would avoid any complexity that isn't strictly necessary, the less we > do, the less breakage we have to deal with later. In that sense adding > the needs_kconfigs and expect the config to be there is probably the > most straightforward solution. Thanks for your input. I understand that you're for replacing in ioctl_ns06.c: int exists = access("/proc/self/ns/user", F_OK); if (exists < 0) tst_res(TCONF, "namespace not available"); with .needs_kconfigs: .needs_kconfigs = (const char *[]) { "CONFIG_USER_NS", NULL } Because that was my question - really always prefer kconfig even there is a simple runtime solution? I'd like to have some "rule" like conclusion we can point during review. Kind regards, Petr
Hi! > Thanks for your input. I understand that you're for replacing in ioctl_ns06.c: > > int exists = access("/proc/self/ns/user", F_OK); > > if (exists < 0) > tst_res(TCONF, "namespace not available"); > > with .needs_kconfigs: > > .needs_kconfigs = (const char *[]) { > "CONFIG_USER_NS", > NULL > } > > Because that was my question - really always prefer kconfig even there is a > simple runtime solution? I'd like to have some "rule" like conclusion we can > point during review. I think that from a long term view this is going to be simpler solution than having many different types of checks. The less diverse these checks are the easier they are to review and maintain. Hence I lean towards kernel config checks even though they are slower (mostly unmeasurable on today's harware) than the alternatives.
> Hi! > > Thanks for your input. I understand that you're for replacing in ioctl_ns06.c: > > int exists = access("/proc/self/ns/user", F_OK); > > if (exists < 0) > > tst_res(TCONF, "namespace not available"); > > with .needs_kconfigs: > > .needs_kconfigs = (const char *[]) { > > "CONFIG_USER_NS", > > NULL > > } > > Because that was my question - really always prefer kconfig even there is a > > simple runtime solution? I'd like to have some "rule" like conclusion we can > > point during review. > I think that from a long term view this is going to be simpler solution > than having many different types of checks. The less diverse these > checks are the easier they are to review and maintain. Hence I lean > towards kernel config checks even though they are slower (mostly > unmeasurable on today's harware) than the alternatives. Great, thanks for a general resolution. Unless somebody objects I'm ok with the conclusion (ideally we should formalise it in rules in docs). Kind regards, Petr
On Wed, Jan 7, 2026 at 5:15 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > Thanks for your input. I understand that you're for replacing in ioctl_ns06.c: > > > > int exists = access("/proc/self/ns/user", F_OK); > > > > if (exists < 0) > > tst_res(TCONF, "namespace not available"); > > > > with .needs_kconfigs: > > > > .needs_kconfigs = (const char *[]) { > > "CONFIG_USER_NS", > > NULL > > } > > > > Because that was my question - really always prefer kconfig even there is a > > simple runtime solution? I'd like to have some "rule" like conclusion we can > > point during review. > > I think that from a long term view this is going to be simpler solution > than having many different types of checks. The less diverse these > checks are the easier they are to review and maintain. Hence I lean > towards kernel config checks even though they are slower (mostly > unmeasurable on today's harware) than the alternatives. I think I lean opposite way, and rather have a check for right environment to support the test. You can have feature X enabled in kernel config, but still disabled later at boot/runtime (e.g. max_user_namespaces=0), or a module simply not being loaded. > > -- > Cyril Hrubis > chrubis@suse.cz >
> On Wed, Jan 7, 2026 at 5:15 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > Thanks for your input. I understand that you're for replacing in ioctl_ns06.c: > > > int exists = access("/proc/self/ns/user", F_OK); > > > if (exists < 0) > > > tst_res(TCONF, "namespace not available"); > > > with .needs_kconfigs: > > > .needs_kconfigs = (const char *[]) { > > > "CONFIG_USER_NS", > > > NULL > > > } > > > Because that was my question - really always prefer kconfig even there is a > > > simple runtime solution? I'd like to have some "rule" like conclusion we can > > > point during review. > > I think that from a long term view this is going to be simpler solution > > than having many different types of checks. The less diverse these > > checks are the easier they are to review and maintain. Hence I lean > > towards kernel config checks even though they are slower (mostly > > unmeasurable on today's harware) than the alternatives. > I think I lean opposite way, and rather have a check for right > environment to support the test. > You can have feature X enabled in kernel config, but still disabled > later at boot/runtime > (e.g. max_user_namespaces=0), or a module simply not being loaded. +1, all "tristate" configured as module might be missing. (openSUSE JeOS suffers with this when minimal kernel is installed). But "bool" are safe unless depend on "tristate" configured as module (not the case of CONFIG_USER_NS). Kind regards, Petr > > -- > > Cyril Hrubis > > chrubis@suse.cz
... > > > > Because that was my question - really always prefer kconfig even there is a > > > > simple runtime solution? I'd like to have some "rule" like conclusion we can > > > > point during review. > > > I think that from a long term view this is going to be simpler solution > > > than having many different types of checks. The less diverse these > > > checks are the easier they are to review and maintain. Hence I lean > > > towards kernel config checks even though they are slower (mostly > > > unmeasurable on today's harware) than the alternatives. > > I think I lean opposite way, and rather have a check for right > > environment to support the test. > > You can have feature X enabled in kernel config, but still disabled > > later at boot/runtime > > (e.g. max_user_namespaces=0), or a module simply not being loaded. > +1, all "tristate" configured as module might be missing. (openSUSE JeOS suffers > with this when minimal kernel is installed). > But "bool" are safe unless depend on "tristate" configured as module (not the > case of CONFIG_USER_NS). > Kind regards, > Petr OK, it's a bool, let's merge it. I'm sorry to block it with a long discussion, but IMHO it was useful to bring some conclusion (I'm going to send a patch for rules). Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
Hi! > > > Because that was my question - really always prefer kconfig even there is a > > > simple runtime solution? I'd like to have some "rule" like conclusion we can > > > point during review. > > > > I think that from a long term view this is going to be simpler solution > > than having many different types of checks. The less diverse these > > checks are the easier they are to review and maintain. Hence I lean > > towards kernel config checks even though they are slower (mostly > > unmeasurable on today's harware) than the alternatives. > > I think I lean opposite way, and rather have a check for right > environment to support the test. > You can have feature X enabled in kernel config, but still disabled > later at boot/runtime > (e.g. max_user_namespaces=0), or a module simply not being loaded. That is a good catch. Maybe the best way forward would be to add hooks for certain config options into the LTP kernel config parser that would do additional runtime checks. That way we would have both the information on which kernel configs should be enabled in test metadata as well as runtime checks. What about eventually adding something as: diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c index 9bcd57721..f6abe6cc7 100644 --- a/lib/tst_kconfig.c +++ b/lib/tst_kconfig.c @@ -110,6 +110,18 @@ static void close_kconfig(FILE *fp) fclose(fp); } +static void runtime_check(struct tst_kconfig_var *var) +{ + if (strstr(var->id, "CONFIG_USER_NS")) { + if (!tst_user_ns_enabled()) { + tst_res(TINFO, "CONFIG_USER_NS present but runtime is disabled"); + var->val = 'n'; + } + } else if (...) + ... + } +} + static inline int kconfig_parse_line(const char *line, struct tst_kconfig_var *vars, unsigned int vars_len) @@ -183,9 +195,11 @@ out: switch (val[0]) { case 'y': vars[i].choice = 'y'; + runtime_check(&vars[i]); return 1; case 'm': vars[i].choice = 'm'; + runtime_check(&vars[i]); return 1; } }
Hi! > OK, it's a bool, let's merge it. > I'm sorry to block it with a long discussion, but IMHO it was useful to bring > some conclusion (I'm going to send a patch for rules). I would merge this before the release, since this works fine as long as namespaces are not disabled on commandline, which should be a corner case and I would like to fix that with the hooks I've proposed in reply to email from Jan. Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Hi! > > OK, it's a bool, let's merge it. > > I'm sorry to block it with a long discussion, but IMHO it was useful to bring > > some conclusion (I'm going to send a patch for rules). > I would merge this before the release, since this works fine as long as > namespaces are not disabled on commandline, which should be a corner > case and I would like to fix that with the hooks I've proposed in reply > to email from Jan. > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Right, finally merged. Kind regards, Petr
Hi all, [ Cc Martin ] > Hi! > > > > Because that was my question - really always prefer kconfig even there is a > > > > simple runtime solution? I'd like to have some "rule" like conclusion we can > > > > point during review. > > > I think that from a long term view this is going to be simpler solution > > > than having many different types of checks. The less diverse these > > > checks are the easier they are to review and maintain. Hence I lean > > > towards kernel config checks even though they are slower (mostly > > > unmeasurable on today's harware) than the alternatives. > > I think I lean opposite way, and rather have a check for right > > environment to support the test. > > You can have feature X enabled in kernel config, but still disabled > > later at boot/runtime > > (e.g. max_user_namespaces=0), or a module simply not being loaded. > That is a good catch. > Maybe the best way forward would be to add hooks for certain config > options into the LTP kernel config parser that would do additional > runtime checks. That way we would have both the information on which > kernel configs should be enabled in test metadata as well as runtime > checks. I like the idea: having metadata doc + runtime check is nice. FYI (I know you all watch GitHub PR but just in case) to document that for some tests require more things to check, e.g. kconfig + min_kver: https://github.com/linux-test-project/ltp/pull/1285 Kind regards, Petr > What about eventually adding something as: > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c > index 9bcd57721..f6abe6cc7 100644 > --- a/lib/tst_kconfig.c > +++ b/lib/tst_kconfig.c > @@ -110,6 +110,18 @@ static void close_kconfig(FILE *fp) > fclose(fp); > } > +static void runtime_check(struct tst_kconfig_var *var) > +{ > + if (strstr(var->id, "CONFIG_USER_NS")) { > + if (!tst_user_ns_enabled()) { > + tst_res(TINFO, "CONFIG_USER_NS present but runtime is disabled"); > + var->val = 'n'; > + } > + } else if (...) > + ... > + } > +} > + > static inline int kconfig_parse_line(const char *line, > struct tst_kconfig_var *vars, > unsigned int vars_len) > @@ -183,9 +195,11 @@ out: > switch (val[0]) { > case 'y': > vars[i].choice = 'y'; > + runtime_check(&vars[i]); > return 1; > case 'm': > vars[i].choice = 'm'; > + runtime_check(&vars[i]); > return 1; > } > }
> > Maybe the best way forward would be to add hooks for certain config > > options into the LTP kernel config parser that would do additional > > runtime checks. That way we would have both the information on which > > kernel configs should be enabled in test metadata as well as runtime > > checks. > > I like the idea: having metadata doc + runtime check is nice. > +1
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c index 7eb60e7fc..6983259e4 100644 --- a/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c @@ -81,5 +81,10 @@ static struct tst_test test = { {&info0, .size = sizeof(*info0)}, {&info1, .size = sizeof(*info1)}, {} + }, + .needs_kconfigs = (const char *[]) { + "CONFIG_USER_NS", + "CONFIG_PID_NS", + NULL } }; diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd03.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd03.c index 8f2779be2..5ea64a9bd 100644 --- a/testcases/kernel/syscalls/ioctl/ioctl_pidfd03.c +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd03.c @@ -60,5 +60,10 @@ static struct tst_test test = { {&args, .size = sizeof(*args)}, {&info, .size = sizeof(*info)}, {} + }, + .needs_kconfigs = (const char *[]) { + "CONFIG_USER_NS", + "CONFIG_PID_NS", + NULL } }; diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd04.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd04.c index cf8393dec..d4a1a1ea3 100644 --- a/testcases/kernel/syscalls/ioctl/ioctl_pidfd04.c +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd04.c @@ -67,5 +67,10 @@ static struct tst_test test = { {&args, .size = sizeof(*args)}, {&info, .size = sizeof(*info)}, {} + }, + .needs_kconfigs = (const char *[]) { + "CONFIG_USER_NS", + "CONFIG_PID_NS", + NULL } }; diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd05.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd05.c index 31439f0b7..3a5bc7592 100644 --- a/testcases/kernel/syscalls/ioctl/ioctl_pidfd05.c +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd05.c @@ -71,5 +71,10 @@ static struct tst_test test = { {&args, .size = sizeof(*args)}, {&info_invalid, .size = sizeof(*info_invalid)}, {} + }, + .needs_kconfigs = (const char *[]) { + "CONFIG_USER_NS", + "CONFIG_PID_NS", + NULL } }; diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd06.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd06.c index 2a8bc1432..386a1e235 100644 --- a/testcases/kernel/syscalls/ioctl/ioctl_pidfd06.c +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd06.c @@ -62,5 +62,10 @@ static struct tst_test test = { {&args, .size = sizeof(*args)}, {&info, .size = sizeof(*info)}, {} + }, + .needs_kconfigs = (const char *[]) { + "CONFIG_USER_NS", + "CONFIG_PID_NS", + NULL } };
The flags CLONE_NEWUSER and CLONE_NEWPID require specific namespace support. Add CONFIG_USER_NS and CONFIG_PID_NS to needs_kconfigs so these tests return TCONF instead of failing. Signed-off-by: Terry Tritton <terry.tritton@linaro.org> --- testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c | 5 +++++ testcases/kernel/syscalls/ioctl/ioctl_pidfd03.c | 5 +++++ testcases/kernel/syscalls/ioctl/ioctl_pidfd04.c | 5 +++++ testcases/kernel/syscalls/ioctl/ioctl_pidfd05.c | 5 +++++ testcases/kernel/syscalls/ioctl/ioctl_pidfd06.c | 5 +++++ 5 files changed, 25 insertions(+)