diff mbox series

[testsuite] tests/key_socket: skip the test if CONFIG_NET_KEY is not enabled

Message ID 20240827145518.469490-1-omosnace@redhat.com (mailing list archive)
State Accepted
Delegated to: Ondrej Mosnáček
Headers show
Series [testsuite] tests/key_socket: skip the test if CONFIG_NET_KEY is not enabled | expand

Commit Message

Ondrej Mosnacek Aug. 27, 2024, 2:55 p.m. UTC
RHEL/CentOS Stream 10+ and Fedora ELN will have CONFIG_NET_KEY disabled
[1]. Make the test skip itself when it detects that PF_KEY is not
supported so that the testsuite can still pass out-of-the-box on these
platforms.

[1] https://gitlab.com/cki-project/kernel-ark/-/commit/99d6d1c86fe1bb1df5c0b80f4717826c2330e291

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 tests/key_socket/test | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Stephen Smalley Aug. 29, 2024, 1:36 p.m. UTC | #1
On Tue, Aug 27, 2024 at 11:09 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> RHEL/CentOS Stream 10+ and Fedora ELN will have CONFIG_NET_KEY disabled
> [1]. Make the test skip itself when it detects that PF_KEY is not
> supported so that the testsuite can still pass out-of-the-box on these
> platforms.
>
> [1] https://gitlab.com/cki-project/kernel-ark/-/commit/99d6d1c86fe1bb1df5c0b80f4717826c2330e291
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Wondering if we should drop NET_KEY from the testsuite defconfig too then.

> ---
>  tests/key_socket/test | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tests/key_socket/test b/tests/key_socket/test
> index a13327f..3f371fe 100755
> --- a/tests/key_socket/test
> +++ b/tests/key_socket/test
> @@ -16,7 +16,13 @@ BEGIN {
>          $v = " ";
>      }
>
> -    plan tests => 5;
> +    $result = system "$basedir/key_sock $v 2>&1";
> +    if ( $result >> 8 eq 97 ) {    # EAFNOSUPPORT
> +        plan skip_all => "PF_KEY not supported by kernel";
> +    }
> +    else {
> +        plan tests => 5;
> +    }
>  }
>
>  ############ Test key_socket #############
> --
> 2.46.0
>
>
Paul Moore Aug. 29, 2024, 2:35 p.m. UTC | #2
On Thu, Aug 29, 2024 at 9:37 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Tue, Aug 27, 2024 at 11:09 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > RHEL/CentOS Stream 10+ and Fedora ELN will have CONFIG_NET_KEY disabled
> > [1]. Make the test skip itself when it detects that PF_KEY is not
> > supported so that the testsuite can still pass out-of-the-box on these
> > platforms.
> >
> > [1] https://gitlab.com/cki-project/kernel-ark/-/commit/99d6d1c86fe1bb1df5c0b80f4717826c2330e291
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>
> Wondering if we should drop NET_KEY from the testsuite defconfig too then.

If we have a test for it, it seems like it might be worthwhile keeping
it as long as the upstream kernel still supports PF_KEY.  I'm not sure
if Fedora plans to disable CONFIG_NET_KEY, but as of kernel
v6.11.0-0.rc5.20240827xxx CONFIG_NET_KEY is still enabled as a module.
Even if Fedora does disable it in their build I can enable it in my
testing, I already do that now for a few things.

--
paul-moore.com
Ondrej Mosnacek Aug. 30, 2024, 6:23 a.m. UTC | #3
On Thu, Aug 29, 2024 at 4:35 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Aug 29, 2024 at 9:37 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Tue, Aug 27, 2024 at 11:09 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > RHEL/CentOS Stream 10+ and Fedora ELN will have CONFIG_NET_KEY disabled
> > > [1]. Make the test skip itself when it detects that PF_KEY is not
> > > supported so that the testsuite can still pass out-of-the-box on these
> > > platforms.
> > >
> > > [1] https://gitlab.com/cki-project/kernel-ark/-/commit/99d6d1c86fe1bb1df5c0b80f4717826c2330e291
> > >
> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> >
> > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> >
> > Wondering if we should drop NET_KEY from the testsuite defconfig too then.
>
> If we have a test for it, it seems like it might be worthwhile keeping
> it as long as the upstream kernel still supports PF_KEY.  I'm not sure
> if Fedora plans to disable CONFIG_NET_KEY, but as of kernel
> v6.11.0-0.rc5.20240827xxx CONFIG_NET_KEY is still enabled as a module.
> Even if Fedora does disable it in their build I can enable it in my
> testing, I already do that now for a few things.

No, Fedora doesn't have any plans to disable it as far as I know.
Fedora doesn't have any contractual obligation for maintenance and
mostly just tracks the upstream kernel, so there is little motivation
to disable functionality because of the lack of maintenance upstream.
("If it works, why risk breaking users? And if it doesn't, then
upstream should be the one to remove it." seems to be the general
philosophy in such cases.)

I agree with keeping it in defconfig - after all, CONFIG_ANDROID=y is
also there, even though it isn't strictly required (and so are other
configs).
Ondrej Mosnacek Aug. 30, 2024, 6:30 a.m. UTC | #4
On Fri, Aug 30, 2024 at 8:23 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Thu, Aug 29, 2024 at 4:35 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Thu, Aug 29, 2024 at 9:37 AM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > >
> > > On Tue, Aug 27, 2024 at 11:09 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > >
> > > > RHEL/CentOS Stream 10+ and Fedora ELN will have CONFIG_NET_KEY disabled
> > > > [1]. Make the test skip itself when it detects that PF_KEY is not
> > > > supported so that the testsuite can still pass out-of-the-box on these
> > > > platforms.
> > > >
> > > > [1] https://gitlab.com/cki-project/kernel-ark/-/commit/99d6d1c86fe1bb1df5c0b80f4717826c2330e291
> > > >
> > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > >
> > > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> > >
> > > Wondering if we should drop NET_KEY from the testsuite defconfig too then.
> >
> > If we have a test for it, it seems like it might be worthwhile keeping
> > it as long as the upstream kernel still supports PF_KEY.  I'm not sure
> > if Fedora plans to disable CONFIG_NET_KEY, but as of kernel
> > v6.11.0-0.rc5.20240827xxx CONFIG_NET_KEY is still enabled as a module.
> > Even if Fedora does disable it in their build I can enable it in my
> > testing, I already do that now for a few things.
>
> No, Fedora doesn't have any plans to disable it as far as I know.
> Fedora doesn't have any contractual obligation for maintenance and
> mostly just tracks the upstream kernel, so there is little motivation
> to disable functionality because of the lack of maintenance upstream.
> ("If it works, why risk breaking users? And if it doesn't, then
> upstream should be the one to remove it." seems to be the general
> philosophy in such cases.)
>
> I agree with keeping it in defconfig - after all, CONFIG_ANDROID=y is
> also there, even though it isn't strictly required (and so are other
> configs).

Now applied:
https://github.com/SELinuxProject/selinux-testsuite/commit/a9e631f0f1d5b11756a62679e8da073b3cc85b13
diff mbox series

Patch

diff --git a/tests/key_socket/test b/tests/key_socket/test
index a13327f..3f371fe 100755
--- a/tests/key_socket/test
+++ b/tests/key_socket/test
@@ -16,7 +16,13 @@  BEGIN {
         $v = " ";
     }
 
-    plan tests => 5;
+    $result = system "$basedir/key_sock $v 2>&1";
+    if ( $result >> 8 eq 97 ) {    # EAFNOSUPPORT
+        plan skip_all => "PF_KEY not supported by kernel";
+    }
+    else {
+        plan tests => 5;
+    }
 }
 
 ############ Test key_socket #############