On 03/26/2018 06:56 AM, Petr Mladek wrote: > On Mon 2018-03-12 14:57:04, Joe Lawrence wrote: >> Hi Petr, >> >> These are the callback tests that I hacked up into a livepatch >> kselftest. (Basically I copied a bunch of the sample modules and >> verified the expected dmesg output as I had listed in in the >> Documentation/livepatch/callbacks.txt file.) The script is still a >> little rough and maybe this isn't the direction we want to go for proper >> kselftests, but perhaps it saves you some time/sanity for verifying this >> patchset. > > > >> Hope this helps, >> >> -- Joe >> >> -- >8 -- >8 -- >8 -- >8 -- >> >> >From 0364430c53e12e21923bed20cb651374b4cf9ba9 Mon Sep 17 00:00:00 2001 >> From: Joe Lawrence <joe.lawre...@redhat.com> >> Date: Tue, 6 Mar 2018 17:32:25 -0500 >> Subject: WIP - livepatch kselftest >> >> CONFIG_TEST_LIVEPATCH=m >> % make -C tools/testing/selftests TARGETS=livepatch run_tests >> >> Signed-off-by: Joe Lawrence <joe.lawre...@redhat.com> >> --- >> lib/Kconfig.debug | 11 + >> lib/Makefile | 9 + >> lib/test_klp_callbacks_busy.c | 58 ++ >> lib/test_klp_callbacks_demo.c | 205 +++++++ >> lib/test_klp_callbacks_demo2.c | 149 +++++ >> lib/test_klp_callbacks_mod.c | 39 ++ > > I would suggest to put it into lib/livepatch/ subdirectory. I hope > that we will add more tests in the long term.
Good idea, we should encourage more tests in a livepatch/ subdir and not clutter up the lib/ directory. >> diff --git a/tools/testing/selftests/livepatch/livepatch-test >> b/tools/testing/selftests/livepatch/livepatch-test >> new file mode 100755 >> index 000000000000..798317bf69f6 >> --- /dev/null >> +++ b/tools/testing/selftests/livepatch/livepatch-test > > s/livepatch-test/livepatch-test.sh/ ? > > It seems to be common to add .sh suffix for bash script names. I'll rename with the suffix. >> @@ -0,0 +1,658 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (C) 2018 Joe Lawrence <joe.lawre...@redhat.com> >> + >> +MAX_RETRIES=30 >> +RETRY_INTERVAL=2 # seconds >> +BETWEEN_TESTS=20 # seconds > > These 20 seconds kept me in a tense (waiting for the final result) > for a very long time ;-) Is there any particular reason for such > a long delay? It certainly builds suspense :) > I wonder if we need a delay at all or if let's say 2 seconds might > be enough. I removed the delays completely and the tests ran successfully. What might be better than a between test delay would be some kind of initial-condition verification, ie, make sure that the test starts/ends with none of the livepatch test modules loaded. For the test cases which load multiple livepatches, is there an easy way to determine the patch stack order from userspace? I think that would be helpful when trying to remove all of them. >> +echo -n "TEST1 ... " >> +dmesg -C >> + >> +load_mod $MOD_TARGET >> +load_mod $MOD_LIVEPATCH >> +wait_for_transition $MOD_LIVEPATCH >> +disable_lp $MOD_LIVEPATCH >> +unload_mod $MOD_LIVEPATCH >> +unload_mod $MOD_TARGET >> + >> +check_result "% modprobe $MOD_TARGET >> +$MOD_TARGET: livepatch_callbacks_mod_init >> +% modprobe $MOD_LIVEPATCH >> +livepatch: enabling patch '$MOD_LIVEPATCH' >> +livepatch: '$MOD_LIVEPATCH': initializing patching transition >> +$MOD_LIVEPATCH: pre_patch_callback: $MOD_TARGET -> [MODULE_STATE_LIVE] >> Normal state >> +$MOD_LIVEPATCH: pre_patch_callback: vmlinux >> +livepatch: '$MOD_LIVEPATCH': starting patching transition >> +livepatch: '$MOD_LIVEPATCH': completing patching transition >> +$MOD_LIVEPATCH: post_patch_callback: $MOD_TARGET -> [MODULE_STATE_LIVE] >> Normal state >> +$MOD_LIVEPATCH: post_patch_callback: vmlinux >> +livepatch: '$MOD_LIVEPATCH': patching complete >> +% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH/enabled >> +livepatch: '$MOD_LIVEPATCH': initializing unpatching transition >> +$MOD_LIVEPATCH: pre_unpatch_callback: $MOD_TARGET -> [MODULE_STATE_LIVE] >> Normal state >> +$MOD_LIVEPATCH: pre_unpatch_callback: vmlinux >> +livepatch: '$MOD_LIVEPATCH': starting unpatching transition >> +livepatch: '$MOD_LIVEPATCH': completing unpatching transition >> +$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET -> [MODULE_STATE_LIVE] >> Normal state >> +$MOD_LIVEPATCH: post_unpatch_callback: vmlinux >> +livepatch: '$MOD_LIVEPATCH': unpatching complete >> +% rmmod $MOD_LIVEPATCH >> +% rmmod $MOD_TARGET >> +$MOD_TARGET: livepatch_callbacks_mod_exit" > > I was a bit surprised when seeing this way of checking results. > But on the other hand, it looks pretty effective, especially for > the callbacks. And the 3rd look, any patched function might write > something into the log when called. This was a quickly scripted version of what I was manually verifying with the sample example livepatches. I don't know if it will scale, but it was pretty easy to add tests this way. I wonder though if better dmesg filters will be required as the livepatch core adds more debug msgs? > I like it. Let's see how it works in the long term. But I am rather > positive. > > Thanks a lot for working on it. Thanks for taking a look and running the tests. I'll make some of your suggested changes and send it up for a proper review soon. -- Joe
