Re: [PATCH v2 0/2] livepatch: Move tests from lib/livepatch to selftests/livepatch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Shuah,

On Thu, 30 Jun 2022, Shuah Khan wrote:

> On 6/30/22 8:12 AM, Marcos Paulo de Souza wrote:
> > Hi there,
> > 
> > this is the v2 of the patchset. The v1 can be found at [1]. There is only
> > one
> > change in patch 1, which changed the target directory to build the test
> > modules.
> > All other changes happen in patch 2.
> > 
> > Thanks for reviewing!
> > 
> > Changes from v1:
> > # test_modules/Makefile
> >    * Build the test modules targeting /lib/modules, instead of ksrc when
> >    building
> >      from the kernel source.
> > 
> > # test_modules/test_klp_syscall.c
> >    * Added a parameter array to receive the pids that should transition to
> >    the
> >      new system call. (suggedted by Joe)
> >    * Create a new sysfs file /sys/kernel/test_klp_syscall/npids to show how
> >    many
> >      pids from the argument need to transition to the new state. (suggested
> >      by
> >      Joe)
> >    * Fix the PPC32 support by adding the syscall wrapper for archs that
> >    select it
> >      by default, without erroring out. PPC does not set SYSCALL_WRAPPER, so
> >      having it set in v1 was a mistake. (suggested by Joe)
> >    * The aarch64 syscall prefix was added too, since the livepatch support
> >    will come soon.
> > 
> > # test_binaries/test_klp-call_getpid.c
> >    * Change %d/%u in printf (suggested byu Joe)
> >    * Change run -> stop variable name, and inverted the assignments
> >    (suggested by
> >    * Joe).
> > 
> > # File test-syscall.sh
> >    * Fixed test-syscall.sh to call test_klp-call-getpid in test_binaries dir
> >    * Load test_klp_syscall passed the pids of the test_klp-call_getpid
> >    instances.
> >      Check the sysfs file from test_klp_syscall module to check that all
> >      pids
> >      transitioned correctly. (suggested by Joe)
> >    * Simplified the loop that calls test_klp-call_getpid. (suggested by Joe)
> >    * Removed the "success" comment from the script, as it's implicit that it
> >      succeed. Otherwise load_lp would error out. (suggested by Joe)
> > 
> > * Changed the commit message of patch 2 to further detail what means
> > "tricky"
> >    when livepatching syscalls. (suggested by Joe)
> > 
> > [1]: 20220603143242.870-1-mpdesouza@xxxxxxxx
> > 
> 
> Sorry Nack on this. Let's not add modules under selftests. Any usage of
> module_init()
> doesn't belong under selftests.

as mentioned before, that ship has already sailed with 
tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c. Anyway...

You wrote before that you did not have a problem with it. And you would 
not have a problem with Marcos' approach if modules can be compiled and if 
not, the tests would fail gracefully. What has changed? If you see a 
problem in the patch set regarding this, can we fix it?
 
> Leave these under lib and use KSTM_MODULE_LOADERS to load these modules that
> live under lib.

I may misunderstand but KSTM_MODULE_LOADERS does not seem to provide the 
flexibility we need (yes, it could be hacked around, but I do not think 
that the result would be nice). See what we have in 
tools/testing/selftests/livepatch/functions.sh to make sure that a live 
patch module is properly loaded and unloaded.

My main question is different though. As Marcos mentioned before, we would 
like to have our tests really flexible and a possibility to prepare and 
load different live patch modules based on a template is a part of it. 
What is your proposal regarding this? I can imagine having a template in 
lib/livepatch/ which would not be compilable and a script in 
tools/testing/selftests/livepatch/ would copy it many times, amend the 
copies (meaning parameters would be filled in with sed or the code would 
be changed), compile them and load them. But this sounds horrible to me, 
especially when compared to Marcos' approach in this patch set which is 
quite straightforward.

Then there is an opportunity which Joe described. To run the latest 
livepatch kselftests on an older kernel. Having test modules in lib/ is 
kind of an obstacle there.

Regards

Miroslav



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux