Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Fri, 8 Dec 2023 13:43:24 +0100
From:   Petr Mladek <pmladek@suse.com>
To:     Miroslav Benes <mbenes@suse.cz>
Cc:     Joe Lawrence <joe.lawrence@redhat.com>,
        Marcos Paulo de Souza <mpdesouza@suse.com>,
        Shuah Khan <shuah@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Heiko Carstens <hca@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Sven Schnelle <svens@linux.ibm.com>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Jiri Kosina <jikos@kernel.org>,
        linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
        live-patching@vger.kernel.org
Subject: Re: [PATCH v3 2/3] livepatch: Move tests from lib/livepatch to
 selftests/livepatch
Message-ID: <ZXL_Lmx9J8U25fq-@alley>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.LSU.2.21.2312080854100.14729@pobox.suse.cz>
Precedence: bulk

On Fri 2023-12-08 09:06:30, Miroslav Benes wrote:
> > > My idea is to abandon this way completely, take the selftests and build 
> > > and run them on the system right away.
> > > 
> > > Both should be doable, hopefully, if we wire it all correctly... and 
> > > document it.
> > > 
> > I can't think of why it shouldn't continue to work, even in a future
> > where newer livepatching selftests support older kernels.  (We would
> > just have newer selftests sources backported to test older kernel sources.)
> > 
> > Are there any test cases which truly need to be build on-the-fly?  Aside
> > from testing different toolchain pieces?
> 
> https://github.com/SUSE/qa_test_klp is what we would like to migrate to 
> selftests to have just one place for all tests.
> 
> There is basically just one live patch template and one supporting kernel 
> module template which is livepatched. The final result is driven by a set 
> of macros and function parameters. In some cases more modules are compiled 
> as parts of a test in a loop.
>
> However, I do not think there is anything which truly needs to be built 
> on-the-fly in the end. Everything can be worked around. Templates may be 
> abandoned and we would have a live patch and a module(s) per test. Some 
> tests are probably not worth it and may be removed. So it is a question of 
> convenience and maintainability. When we, for example, simplified API and 
> klp_register_patch() was removed, only one place needed to be amended. 
> Also, the current state in lib/livepatch/ could be simplified with the 
> proposed infrastructure as some files could be merged together.

In the patchset reworking livepatch states, I solved this problem
by including the same sources in another module source, like:

$> cat lib/livepatch/test_klp_speaker_livepatch2.c
// SPDX-License-Identifier: GPL-2.0
// Copyright (C) 2023 SUSE

/* Same livepatch with the same features. */
#include "test_klp_speaker_livepatch.c"

=========

$> cat lib/livepatch/test_klp_speaker2.c
// SPDX-License-Identifier: GPL-2.0
// Copyright (C) 2023 SUSE

/* Use versioned function name for livepatched functions */
#define _VER_NAME(name) name ## 2

/* Same module with the same features. */
#include "test_klp_speaker.c"

==========

And the behavior was changed by module parameters. The test
lookes like:

$> cat tools/testing/selftests/livepatch/test-modules.sh
[...]
start_test "multiple target modules"

load_mod $MOD_TARGET
read_module_param $MOD_TARGET welcome

load_lp $MOD_LIVEPATCH add_applause=1
read_module_param $MOD_TARGET welcome

load_mod $MOD_TARGET2
read_module_param $MOD_TARGET2 welcome

unload_mod $MOD_TARGET2
disable_lp $MOD_LIVEPATCH
read_module_param $MOD_TARGET welcome

unload_lp $MOD_LIVEPATCH
unload_mod $MOD_TARGET

===========

It is a kind of hack. But it would allow to build and package the
test modules. It has several advantages:

   + Less modules are needed. The behavior is modified by
     the parameters.

   + The separate parameters are easier to parse in compare
     with embedding the behavior into the module name.

   + Build problems would be solved before the packages
     reach QA department

   + The package would have lightweight dependencies.

   + Running the tests would be faster.


Regarding disadvantages:

   + The source included in all the other variants would be more
     complex.

     But the same would happen when building the modules during
     the tests. It would also require a more complicated template
     and an extra script generating the particular module sources.


I personally prefer the solution with "#include" because it has
all the mentioned advantages. The "#include" is a hack but it is
needed only when we need more modules with all the features.

Best Regards,
Petr