| Message ID | 20220603143242.870-1-mpdesouza@suse.com |
|---|---|
| Headers | show |
| Series | livepatch: Move tests from lib/livepatch to selftests/livepatch | expand |
On 6/3/22 8:32 AM, Marcos Paulo de Souza wrote: > Hi there, > > The first patch moves the current livepatch tests to selftests, allowing it > be better suited to contain more complex tests, like using userspace C code > to use the livepatched kernel code. As a bonus it allows to use > "gen_tar" to export the livepatch selftests, rebuild the modules by > running make in selftests/livepatch directory and simplifies the process > of creating and debugging new selftests. > In general selftests don't include modules. We keep test modules under lib. One of the reasons is that modules have dependencies on the kernel and should be built when kernel is built. I don't fully buy the argument that moving modules under selftest would simplify the process. > It keeps the ability to execute the tests by running the shell scripts, > like "test-livepatch.sh", but beware that the kernel modules > might not be up-to-date. > I am not what you mean by this. > The second patch includes a new test to exercise the functionality to livepatch > a heavy hammered function. The test uses getpid in this case. > > I tested the changes by running the tests within the kernel source tree and running > from the gen_tar extracted directory. > I would like to understand the negatives of continuing to keep modules under lib? thanks, -- Shuah
On 6/9/22 4:16 PM, Shuah Khan wrote: > On 6/3/22 8:32 AM, Marcos Paulo de Souza wrote: >> Hi there, >> >> The first patch moves the current livepatch tests to selftests, >> allowing it >> be better suited to contain more complex tests, like using userspace C >> code >> to use the livepatched kernel code. As a bonus it allows to use >> "gen_tar" to export the livepatch selftests, rebuild the modules by >> running make in selftests/livepatch directory and simplifies the process >> of creating and debugging new selftests. >> > > In general selftests don't include modules. We keep test modules under lib. > One of the reasons is that modules have dependencies on the kernel and > should > be built when kernel is built. > > I don't fully buy the argument that moving modules under selftest would > simplify > the process. > Hi Shuah, I see that there is tools/testing/selftests/bpf/bpf_testmod/ which claims to be a "conceptually out-of-tree module". Would similarly moving livepatch test modules under tools/ give us flexibility to write them build for multiple kernel versions? Then one could theoretically build and run the latest, greatest selftests against older kernels (assuming the associate script/module/kernel supports the idea)? Regards,
On Thu, Jun 09, 2022 at 02:16:34PM -0600, Shuah Khan wrote: > On 6/3/22 8:32 AM, Marcos Paulo de Souza wrote: > > Hi there, > > > > The first patch moves the current livepatch tests to selftests, allowing it > > be better suited to contain more complex tests, like using userspace C code > > to use the livepatched kernel code. As a bonus it allows to use > > "gen_tar" to export the livepatch selftests, rebuild the modules by > > running make in selftests/livepatch directory and simplifies the process > > of creating and debugging new selftests. > > > > In general selftests don't include modules. We keep test modules under lib. > One of the reasons is that modules have dependencies on the kernel and should > be built when kernel is built. > > I don't fully buy the argument that moving modules under selftest would simplify > the process. As mentioned by Joe in other reply, epbf also contains a kernel module. > > > It keeps the ability to execute the tests by running the shell scripts, > > like "test-livepatch.sh", but beware that the kernel modules > > might not be up-to-date. > > > > I am not what you mean by this. The plan is to upstream some tests that SUSE is using: https://github.com/lpechacek/qa_test_klp/ The current patchset implement the tc_3 from the above repository. > > > The second patch includes a new test to exercise the functionality to livepatch > > a heavy hammered function. The test uses getpid in this case. > > > > I tested the changes by running the tests within the kernel source tree and running > > from the gen_tar extracted directory. > > > > I would like to understand the negatives of continuing to keep modules under lib? Along with the ability to test different kernel versions by exporting the tests, the plan is to have a template kernel module, for building them on the fly. Such approach would benefit porting more tests from this repository. One example is the testcase 5, which applies successive livepatches one after another, just changing it's name. The current patchset makes this move easier in the future. > > thanks, > -- Shuah
On Fri 2022-06-10 09:06:16, Joe Lawrence wrote: > On 6/9/22 4:16 PM, Shuah Khan wrote: > > On 6/3/22 8:32 AM, Marcos Paulo de Souza wrote: > >> Hi there, > >> > >> The first patch moves the current livepatch tests to selftests, > >> allowing it > >> be better suited to contain more complex tests, like using userspace C > >> code > >> to use the livepatched kernel code. As a bonus it allows to use > >> "gen_tar" to export the livepatch selftests, rebuild the modules by > >> running make in selftests/livepatch directory and simplifies the process > >> of creating and debugging new selftests. > >> > > > > In general selftests don't include modules. We keep test modules under lib. > > One of the reasons is that modules have dependencies on the kernel and > > should > > be built when kernel is built. > > > > I don't fully buy the argument that moving modules under selftest would > > simplify > > the process. > > > > Hi Shuah, > > I see that there is tools/testing/selftests/bpf/bpf_testmod/ which > claims to be a "conceptually out-of-tree module". Would similarly > moving livepatch test modules under tools/ give us flexibility to write > them build for multiple kernel versions? Then one could theoretically > build and run the latest, greatest selftests against older kernels > (assuming the associate script/module/kernel supports the idea)? +1 Another motivation is that the new selftest also needs an executable binary. It would be nice to handle both modules and binaries the same way. Honestly, lib/* is a mess. It mixes real functionality and test modules. The relation between the modules and tools/testing/* is far from clear. IMHO, it would be more clean to have the related stuff together. Of course, we could not move all test modules from lib/* easily. Some of them might be used on its own or even as built-in tests. But preventing the move looks like a step in the wrong direction to me. Best Regards, Petr
On 6/10/22 8:48 AM, Petr Mladek wrote: > On Fri 2022-06-10 09:06:16, Joe Lawrence wrote: >> On 6/9/22 4:16 PM, Shuah Khan wrote: >>> On 6/3/22 8:32 AM, Marcos Paulo de Souza wrote: >>>> Hi there, >>>> >>>> The first patch moves the current livepatch tests to selftests, >>>> allowing it >>>> be better suited to contain more complex tests, like using userspace C >>>> code >>>> to use the livepatched kernel code. As a bonus it allows to use >>>> "gen_tar" to export the livepatch selftests, rebuild the modules by >>>> running make in selftests/livepatch directory and simplifies the process >>>> of creating and debugging new selftests. >>>> >>> >>> In general selftests don't include modules. We keep test modules under lib. >>> One of the reasons is that modules have dependencies on the kernel and >>> should >>> be built when kernel is built. >>> >>> I don't fully buy the argument that moving modules under selftest would >>> simplify >>> the process. >>> >> >> Hi Shuah, >> >> I see that there is tools/testing/selftests/bpf/bpf_testmod/ which >> claims to be a "conceptually out-of-tree module". Would similarly >> moving livepatch test modules under tools/ give us flexibility to write >> them build for multiple kernel versions? Then one could theoretically >> build and run the latest, greatest selftests against older kernels >> (assuming the associate script/module/kernel supports the idea)? > > +1 > > Another motivation is that the new selftest also needs > an executable binary. It would be nice to handle both modules > and binaries the same way. > > Honestly, lib/* is a mess. It mixes real functionality and test > modules. The relation between the modules and tools/testing/* > is far from clear. IMHO, it would be more clean to have the related > stuff together. > > Of course, we could not move all test modules from lib/* easily. > Some of them might be used on its own or even as built-in > tests. But preventing the move looks like a step in > the wrong direction to me. > As such bpf_testmod is the only one that is currently under kselftests. I don't have an objection to it from technical stand point. My concern is more from the standpoint of people writing modules that can't be built out of tree. We would add another requirement to kselftest that the out of tree modules should build successfully. As long as that concern is addressed and also test gracefully fails if the module fails to build, we can move on that direction. I would hesitate to extend this to modules dependent on hardware and architecture features such as cpufreq test drivers for example. thanks, -- Shuah
Hi there, The first patch moves the current livepatch tests to selftests, allowing it be better suited to contain more complex tests, like using userspace C code to use the livepatched kernel code. As a bonus it allows to use "gen_tar" to export the livepatch selftests, rebuild the modules by running make in selftests/livepatch directory and simplifies the process of creating and debugging new selftests. It keeps the ability to execute the tests by running the shell scripts, like "test-livepatch.sh", but beware that the kernel modules might not be up-to-date. The second patch includes a new test to exercise the functionality to livepatch a heavy hammered function. The test uses getpid in this case. I tested the changes by running the tests within the kernel source tree and running from the gen_tar extracted directory. Marcos Paulo de Souza (2): livepatch: Move tests from lib/livepatch to selftests/livepatch selftests: livepatch: Test livepatching a heavily called syscall arch/s390/configs/debug_defconfig | 1 - arch/s390/configs/defconfig | 1 - lib/Kconfig.debug | 22 ------- lib/Makefile | 2 - lib/livepatch/Makefile | 14 ----- tools/testing/selftests/livepatch/Makefile | 35 ++++++++++- tools/testing/selftests/livepatch/README | 5 +- tools/testing/selftests/livepatch/config | 1 - .../testing/selftests/livepatch/functions.sh | 34 ++++------- .../selftests/livepatch/test-callbacks.sh | 50 ++++++++-------- .../selftests/livepatch/test-ftrace.sh | 6 +- .../selftests/livepatch/test-livepatch.sh | 10 ++-- .../selftests/livepatch/test-shadow-vars.sh | 2 +- .../testing/selftests/livepatch/test-state.sh | 18 +++--- .../selftests/livepatch/test-syscall.sh | 46 ++++++++++++++ .../test_binaries/test_klp-call_getpid.c | 48 +++++++++++++++ .../selftests/livepatch/test_modules/Makefile | 25 ++++++++ .../test_modules}/test_klp_atomic_replace.c | 0 .../test_modules}/test_klp_callbacks_busy.c | 0 .../test_modules}/test_klp_callbacks_demo.c | 0 .../test_modules}/test_klp_callbacks_demo2.c | 0 .../test_modules}/test_klp_callbacks_mod.c | 0 .../test_modules}/test_klp_livepatch.c | 0 .../test_modules}/test_klp_shadow_vars.c | 0 .../livepatch/test_modules}/test_klp_state.c | 0 .../livepatch/test_modules}/test_klp_state2.c | 0 .../livepatch/test_modules}/test_klp_state3.c | 0 .../livepatch/test_modules/test_klp_syscall.c | 60 +++++++++++++++++++ 28 files changed, 269 insertions(+), 111 deletions(-) delete mode 100644 lib/livepatch/Makefile create mode 100755 tools/testing/selftests/livepatch/test-syscall.sh create mode 100644 tools/testing/selftests/livepatch/test_binaries/test_klp-call_getpid.c create mode 100644 tools/testing/selftests/livepatch/test_modules/Makefile rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_atomic_replace.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_busy.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_demo.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_demo2.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_callbacks_mod.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_livepatch.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_shadow_vars.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_state.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_state2.c (100%) rename {lib/livepatch => tools/testing/selftests/livepatch/test_modules}/test_klp_state3.c (100%) create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c