lkml.org 
[lkml]   [2018]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v10 00/10] livepatch: Atomic replace feature
From
Date
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.lawrence@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.lawrence@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.lawrence@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

\
 
 \ /
  Last update: 2018-03-26 20:12    [W:0.071 / U:39.568 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site