Re: [PATCH] selftest/livepatch: Only run test-kprobe with CONFIG_KPROBES_ON_FTRACE

[Joe Lawrence <joe.lawrence@redhat.com>]

On Tue, Mar 18, 2025 at 11:14:55AM +0100, Miroslav Benes wrote:
> Hi,
> 
> On Mon, 17 Mar 2025, Song Liu wrote:
> 
> > On Mon, Mar 17, 2025 at 11:59 AM Joe Lawrence <joe.lawrence@redhat.com> wrote:
> > >
> > > On 3/17/25 12:51, Song Liu wrote:
> > > > CONFIG_KPROBES_ON_FTRACE is required for test-kprobe. Skip test-kprobe
> > > > when CONFIG_KPROBES_ON_FTRACE is not set.
> > > >
> > > > Signed-off-by: Song Liu <song@kernel.org>
> > > > ---
> > > >  tools/testing/selftests/livepatch/test-kprobe.sh | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/tools/testing/selftests/livepatch/test-kprobe.sh b/tools/testing/selftests/livepatch/test-kprobe.sh
> > > > index 115065156016..fd823dd5dd7f 100755
> > > > --- a/tools/testing/selftests/livepatch/test-kprobe.sh
> > > > +++ b/tools/testing/selftests/livepatch/test-kprobe.sh
> > > > @@ -5,6 +5,8 @@
> > > >
> > > >  . $(dirname $0)/functions.sh
> > > >
> > > > +zgrep KPROBES_ON_FTRACE /proc/config.gz || skip "test-kprobe requires CONFIG_KPROBES_ON_FTRACE"
> > > > +
> > >
> > > Hi Song,
> > >
> > > This in turn depends on CONFIG_IKCONFIG_PROC for /proc/config.gz (not
> > > set for RHEL distro kernels).
> > 
> > I was actually worrying about this when testing it.
> > 
> > > Is there a dynamic way to figure out CONFIG_KPROBES_ON_FTRACE support?
> > 
> > How about we grep kprobe_ftrace_ops from /proc/kallsyms?
> 
> which relies on having KALLSYMS_ALL enabled but since CONFIG_LIVEPATCH 
> depends on it, we are good. So I would say yes, it is a better option. 
> Please, add a comment around.
> 

Kallsyms is a good workaround as kprobe_ftrace_ops should be stable (I
hope :)

Since Song probably noticed this when upgrading and the new kprobes test
unexpectedly failed, I'd add a Fixes tag to help out backporters:

  Fixes: 62597edf6340 ("selftests: livepatch: test livepatching a kprobed function")

but IMHO not worth rushing as important through the merge window.


Also, while poking at this today with virtme-ng, my initial attempt had
build a fairly minimal kernel without CONFIG_DYNAMIC_DEBUG.  We can also
check for that via the sysfs interface to avoid confusing path-not-found
msgs, like the following ...

-- Joe

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

From c71f1966cfb7311bb1dde90c9f1be40704a253b6 Mon Sep 17 00:00:00 2001
From: Joe Lawrence <joe.lawrence@redhat.com>
Date: Tue, 18 Mar 2025 09:26:55 -0400
Subject: [PATCH] selftests/livepatch: check for CONFIG_DYNAMIC_DEBUG
Content-type: text/plain

The livepatch kselftests set and restore dynamic debug to verify
subsystem behavior through expected printk messages.  The tests should
first check that this facility is available, otherwise just skip
running.

Fixes: a2818ee4dce5 ("selftests/livepatch: introduce tests")
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
 tools/testing/selftests/livepatch/functions.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 15601402dee6..92bc083225a6 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -58,6 +58,9 @@ function die() {
 }
 
 function push_config() {
+	if [[ ! -e "$SYSFS_DEBUG_DIR/dynamic_debug" ]]; then
+		skip "test requires CONFIG_DYNAMIC_DEBUG"
+	fi
 	DYNAMIC_DEBUG=$(grep '^kernel/livepatch' "$SYSFS_DEBUG_DIR/dynamic_debug/control" | \
 			awk -F'[: ]' '{print "file " $1 " line " $2 " " $4}')
 	FTRACE_ENABLED=$(sysctl --values kernel.ftrace_enabled)
-- 
2.42.0