The ftrace selftests do not currently produce KTAP output, they produce a
custom format much nicer for human consumption. This means that when run in
automated test systems we just get a single result for the suite as a whole
rather than recording results for individual test cases, making it harder
to look at the test data and masking things like inappropriate skips.
Address this by adding support for KTAP output to the ftracetest script and
providing a trivial wrapper which will be invoked by the kselftest runner
to generate output in this format by default, users using ftracetest
directly will continue to get the existing output.
This is not the most elegant solution but it is simple and effective. I
did consider implementing this by post processing the existing output
format but that felt more complex and likely to result in all output being
lost if something goes seriously wrong during the run which would not be
helpful. I did also consider just writing a separate runner script but
there's enough going on with things like the signal handling for that to
seem like it would be duplicating too much.
Signed-off-by: Mark Brown <broonie(a)kernel.org>
---
tools/testing/selftests/ftrace/Makefile | 3 +-
tools/testing/selftests/ftrace/ftracetest | 63 ++++++++++++++++++++++++--
tools/testing/selftests/ftrace/ftracetest-ktap | 8 ++++
3 files changed, 70 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/ftrace/Makefile b/tools/testing/selftests/ftrace/Makefile
index d6e106fbce11..a1e955d2de4c 100644
--- a/tools/testing/selftests/ftrace/Makefile
+++ b/tools/testing/selftests/ftrace/Makefile
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
all:
-TEST_PROGS := ftracetest
+TEST_PROGS_EXTENDED := ftracetest
+TEST_PROGS := ftracetest-ktap
TEST_FILES := test.d settings
EXTRA_CLEAN := $(OUTPUT)/logs/*
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index c3311c8c4089..539c8d6d5d71 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -13,6 +13,7 @@ echo "Usage: ftracetest [options] [testcase(s)] [testcase-directory(s)]"
echo " Options:"
echo " -h|--help Show help message"
echo " -k|--keep Keep passed test logs"
+echo " -K|--KTAP Output in KTAP format"
echo " -v|--verbose Increase verbosity of test messages"
echo " -vv Alias of -v -v (Show all results in stdout)"
echo " -vvv Alias of -v -v -v (Show all commands immediately)"
@@ -85,6 +86,10 @@ parse_opts() { # opts
KEEP_LOG=1
shift 1
;;
+ --ktap|-K)
+ KTAP=1
+ shift 1
+ ;;
--verbose|-v|-vv|-vvv)
if [ $VERBOSE -eq -1 ]; then
usage "--console can not use with --verbose"
@@ -178,6 +183,7 @@ TEST_DIR=$TOP_DIR/test.d
TEST_CASES=`find_testcases $TEST_DIR`
LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
KEEP_LOG=0
+KTAP=0
DEBUG=0
VERBOSE=0
UNSUPPORTED_RESULT=0
@@ -229,7 +235,7 @@ prlog() { # messages
newline=
shift
fi
- printf "$*$newline"
+ [ "$KTAP" != "1" ] && printf "$*$newline"
[ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE
}
catlog() { #file
@@ -260,11 +266,11 @@ TOTAL_RESULT=0
INSTANCE=
CASENO=0
+CASENAME=
testcase() { # testfile
CASENO=$((CASENO+1))
- desc=`grep "^#[ \t]*description:" $1 | cut -f2- -d:`
- prlog -n "[$CASENO]$INSTANCE$desc"
+ CASENAME=`grep "^#[ \t]*description:" $1 | cut -f2- -d:`
}
checkreq() { # testfile
@@ -277,40 +283,68 @@ test_on_instance() { # testfile
grep -q "^#[ \t]*flags:.*instance" $1
}
+ktaptest() { # result comment
+ if [ "$KTAP" != "1" ]; then
+ return
+ fi
+
+ local result=
+ if [ "$1" = "1" ]; then
+ result="ok"
+ else
+ result="not ok"
+ fi
+ shift
+
+ local comment=$*
+ if [ "$comment" != "" ]; then
+ comment="# $comment"
+ fi
+
+ echo $CASENO $result $INSTANCE$CASENAME $comment
+}
+
eval_result() { # sigval
case $1 in
$PASS)
prlog " [${color_green}PASS${color_reset}]"
+ ktaptest 1
PASSED_CASES="$PASSED_CASES $CASENO"
return 0
;;
$FAIL)
prlog " [${color_red}FAIL${color_reset}]"
+ ktaptest 0
FAILED_CASES="$FAILED_CASES $CASENO"
return 1 # this is a bug.
;;
$UNRESOLVED)
prlog " [${color_blue}UNRESOLVED${color_reset}]"
+ ktaptest 0 UNRESOLVED
UNRESOLVED_CASES="$UNRESOLVED_CASES $CASENO"
return $UNRESOLVED_RESULT # depends on use case
;;
$UNTESTED)
prlog " [${color_blue}UNTESTED${color_reset}]"
+ ktaptest 1 SKIP
UNTESTED_CASES="$UNTESTED_CASES $CASENO"
return 0
;;
$UNSUPPORTED)
prlog " [${color_blue}UNSUPPORTED${color_reset}]"
+ ktaptest 1 SKIP
UNSUPPORTED_CASES="$UNSUPPORTED_CASES $CASENO"
return $UNSUPPORTED_RESULT # depends on use case
;;
$XFAIL)
prlog " [${color_green}XFAIL${color_reset}]"
+ ktaptest 1 XFAIL
XFAILED_CASES="$XFAILED_CASES $CASENO"
return 0
;;
*)
prlog " [${color_blue}UNDEFINED${color_reset}]"
+ ktaptest 0 error
UNDEFINED_CASES="$UNDEFINED_CASES $CASENO"
return 1 # this must be a test bug
;;
@@ -371,6 +405,7 @@ __run_test() { # testfile
run_test() { # testfile
local testname=`basename $1`
testcase $1
+ prlog -n "[$CASENO]$INSTANCE$CASENAME"
if [ ! -z "$LOG_FILE" ] ; then
local testlog=`mktemp $LOG_DIR/${CASENO}-${testname}-log.XXXXXX`
else
@@ -405,6 +440,17 @@ run_test() { # testfile
# load in the helper functions
. $TEST_DIR/functions
+if [ "$KTAP" = "1" ]; then
+ echo "TAP version 13"
+
+ casecount=`echo $TEST_CASES | wc -w`
+ for t in $TEST_CASES; do
+ test_on_instance $t || continue
+ casecount=$((casecount+1))
+ done
+ echo "1..${casecount}"
+fi
+
# Main loop
for t in $TEST_CASES; do
run_test $t
@@ -439,6 +485,17 @@ prlog "# of unsupported: " `echo $UNSUPPORTED_CASES | wc -w`
prlog "# of xfailed: " `echo $XFAILED_CASES | wc -w`
prlog "# of undefined(test bug): " `echo $UNDEFINED_CASES | wc -w`
+if [ "$KTAP" = "1" ]; then
+ echo -n "# Totals:"
+ echo -n " pass:"`echo $PASSED_CASES | wc -w`
+ echo -n " faii:"`echo $FAILED_CASES | wc -w`
+ echo -n " xfail:"`echo $XFAILED_CASES | wc -w`
+ echo -n " xpass:0"
+ echo -n " skip:"`echo $UNTESTED_CASES $UNSUPPORTED_CASES | wc -w`
+ echo -n " error:"`echo $UNRESOLVED_CASES $UNDEFINED_CASES | wc -w`
+ echo
+fi
+
cleanup
# if no error, return 0
diff --git a/tools/testing/selftests/ftrace/ftracetest-ktap b/tools/testing/selftests/ftrace/ftracetest-ktap
new file mode 100755
index 000000000000..b3284679ef3a
--- /dev/null
+++ b/tools/testing/selftests/ftrace/ftracetest-ktap
@@ -0,0 +1,8 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# ftracetest-ktap: Wrapper to integrate ftracetest with the kselftest runner
+#
+# Copyright (C) Arm Ltd., 2023
+
+./ftracetest -K
---
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
change-id: 20230302-ftrace-kselftest-ktap-9d7878691557
Best regards,
--
Mark Brown <broonie(a)kernel.org>
From: Roberto Sassu <roberto.sassu(a)huawei.com>
A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
which runs a user space process from a binary blob, and creates a
bidirectional pipe, so that the kernel can make a request to that process,
and the latter provides its response. It is currently used by bpfilter,
although it does not seem to do any useful work.
The problem is, if other users would like to implement a UMD similar to
bpfilter, they would have to duplicate the code. Instead, make an UMD
management library and API from the existing bpfilter and sockopt code,
and move it to common kernel code.
Also, define the software architecture and the main components of the
library: the UMD Manager, running in the kernel, acting as the frontend
interface to any user or kernel-originated request; the UMD Loader, also
running in the kernel, responsible to load the UMD Handler; the UMD
Handler, running in user space, responsible to handle requests from the UMD
Manager and to send to it the response.
I have two use cases, but for sake of brevity I will propose one.
I would like to add support for PGP keys and signatures in the kernel, so
that I can extend secure boot to applications, and allow/deny code
execution based on the signed file digests included in RPM headers.
While I proposed a patch set a while ago (based on a previous work of David
Howells), the main objection was that the PGP packet parser should not run
in the kernel.
That makes a perfect example for using a UMD. If the PGP parser is moved to
user space (UMD Handler), and the kernel (UMD Manager) just instantiates
the key and verifies the signature on already parsed data, this would
address the concern.
Patch 1 moves the function bpfilter_send_req() to usermode_driver.c and
makes the pipe between the kernel and the user space process suitable for
larger quantity of data (> 64K).
Patch 2 introduces the management library and API.
Patch 3 replaces the existing bpfilter and sockopt code with calls
to the management API. To use the new mechanism, sockopt itself (acts as
UMD Manager) now sends/receives messages to/from bpfilter_umh (acts as UMD
Handler), instead of bpfilter (acts as UMD Loader).
Patch 4 introduces a sample UMD, useful for other implementors, and uses it
for testing.
Patch 5 introduces the documentation of the new management library and API.
Roberto Sassu (5):
usermode_driver: Introduce umd_send_recv() from bpfilter
usermode_driver_mgmt: Introduce management of user mode drivers
bpfilter: Port to user mode driver management API
selftests/umd_mgmt: Add selftests for UMD management library
doc: Add documentation for the User Mode Driver management library
Documentation/driver-api/index.rst | 1 +
Documentation/driver-api/umd_mgmt.rst | 99 +++++++++++++
MAINTAINERS | 9 ++
include/linux/bpfilter.h | 12 +-
include/linux/usermode_driver.h | 2 +
include/linux/usermode_driver_mgmt.h | 35 +++++
kernel/Makefile | 2 +-
kernel/usermode_driver.c | 47 +++++-
kernel/usermode_driver_mgmt.c | 137 ++++++++++++++++++
net/bpfilter/bpfilter_kern.c | 120 +--------------
net/ipv4/bpfilter/sockopt.c | 67 +++++----
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/umd_mgmt/.gitignore | 1 +
tools/testing/selftests/umd_mgmt/Makefile | 14 ++
tools/testing/selftests/umd_mgmt/config | 1 +
.../selftests/umd_mgmt/sample_umd/Makefile | 22 +++
.../selftests/umd_mgmt/sample_umd/msgfmt.h | 13 ++
.../umd_mgmt/sample_umd/sample_binary_blob.S | 7 +
.../umd_mgmt/sample_umd/sample_handler.c | 81 +++++++++++
.../umd_mgmt/sample_umd/sample_loader.c | 28 ++++
.../umd_mgmt/sample_umd/sample_mgr.c | 124 ++++++++++++++++
tools/testing/selftests/umd_mgmt/umd_mgmt.sh | 40 +++++
22 files changed, 707 insertions(+), 156 deletions(-)
create mode 100644 Documentation/driver-api/umd_mgmt.rst
create mode 100644 include/linux/usermode_driver_mgmt.h
create mode 100644 kernel/usermode_driver_mgmt.c
create mode 100644 tools/testing/selftests/umd_mgmt/.gitignore
create mode 100644 tools/testing/selftests/umd_mgmt/Makefile
create mode 100644 tools/testing/selftests/umd_mgmt/config
create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/Makefile
create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/msgfmt.h
create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_binary_blob.S
create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_handler.c
create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_loader.c
create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_mgr.c
create mode 100755 tools/testing/selftests/umd_mgmt/umd_mgmt.sh
--
2.25.1
On Mon, Mar 27, 2023 at 07:56:03AM +0200, Markus Elfring wrote:
> >> 2. Can a cg_destroy() call ever work as expected if a cg_create() call failed?
> >
> > Perhaps next time you can answer your own question by spending 30
> > seconds actually reading the code you're "fixing":
> >
> > int cg_destroy(const char *cgroup)
> > {
> …
> > ret = rmdir(cgroup);
> …
> > if (ret && errno == ENOENT) <<< that case is explicitly handled here
> > ret = 0;
> >
> > return ret;
> > }
>
> Is it interesting somehow that a non-existing directory (which would occasionally
> not be found) is tolerated so far?
> https://elixir.bootlin.com/linux/v6.3-rc3/source/tools/testing/selftests/cg…
>
> Should such a function call be avoided because of a failed cg_create() call?
The point is that (a) you were wrong that this is fixing anything, and
(b) this patch is functionally useless. Sure, we could move some goto's
around and subjectively improve "something". Why? What's the point?
It's highly debatable that what you're doing is even an improvement, and
I'm not interested in wasting time pontificating about the merits of a
trivial "fix" for a test cleanup function that isn't even broken.
Several people have already either advised or directly asked you to stop
sending these patches. I'm not sure why you're choosing to ignore them,
but I'll throw my hat in the ring regardless and do the same. Please
stop sending these fake cleanup patches.
Add the test result "skip" to KTAP version 2 as an alternative way to
indicate a test was skipped.
The current spec uses the "#SKIP" directive to indicate that a test was
skipped. However, the "#SKIP" directive is not always evident when quickly
skimming through KTAP results.
The "skip" result would provide an alternative that could make it clearer
that a test has not successfully passed because it was skipped.
Before:
KTAP version 1
1..1
KTAP version 1
1..2
ok 1 case_1
ok 2 case_2 #SKIP
ok 1 suite
After:
KTAP version 2
1..1
KTAP version 2
1..2
ok 1 case_1
skip 2 case_2
ok 1 suite
Here is a link to a version of the KUnit parser that is able to parse
the skip test result for KTAP version 2. Note this parser is still able
to parse the "#SKIP" directive.
Link: https://kunit-review.googlesource.com/c/linux/+/5689
Signed-off-by: Rae Moar <rmoar(a)google.com>
---
Note: this patch is based on Frank's ktap_spec_version_2 branch.
Documentation/dev-tools/ktap.rst | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/Documentation/dev-tools/ktap.rst b/Documentation/dev-tools/ktap.rst
index ff77f4aaa6ef..f48aa00db8f0 100644
--- a/Documentation/dev-tools/ktap.rst
+++ b/Documentation/dev-tools/ktap.rst
@@ -74,7 +74,8 @@ They are required and must have the format:
<result> <number> [<description>][ # [<directive>] [<diagnostic data>]]
The result can be either "ok", which indicates the test case passed,
-or "not ok", which indicates that the test case failed.
+"not ok", which indicates that the test case failed, or "skip", which indicates
+the test case did not run.
<number> represents the number of the test being performed. The first test must
have the number 1 and the number then must increase by 1 for each additional
@@ -91,12 +92,13 @@ A directive is a keyword that indicates a different outcome for a test other
than passed and failed. The directive is optional, and consists of a single
keyword preceding the diagnostic data. In the event that a parser encounters
a directive it doesn't support, it should fall back to the "ok" / "not ok"
-result.
+/ "skip" result.
Currently accepted directives are:
-- "SKIP", which indicates a test was skipped (note the result of the test case
- result line can be either "ok" or "not ok" if the SKIP directive is used)
+- "SKIP", which indicates a test was skipped (note this is an alternative to
+ the "skip" result type and if the SKIP directive is used, the
+ result can be any type - "ok", "not ok", or "skip")
- "TODO", which indicates that a test is not expected to pass at the moment,
e.g. because the feature it is testing is known to be broken. While this
directive is inherited from TAP, its use in the kernel is discouraged.
@@ -110,7 +112,7 @@ Currently accepted directives are:
The diagnostic data is a plain-text field which contains any additional details
about why this result was produced. This is typically an error message for ERROR
-or failed tests, or a description of missing dependencies for a SKIP result.
+or failed tests, or a description of missing dependencies for a skipped test.
The diagnostic data field is optional, and results which have neither a
directive nor any diagnostic data do not need to include the "#" field
@@ -130,11 +132,18 @@ The test "test_case_name" failed.
::
- ok 1 test # SKIP necessary dependency unavailable
+ skip 1 test # necessary dependency unavailable
-The test "test" was SKIPPED with the diagnostic message "necessary dependency
+The test "test" was skipped with the diagnostic message "necessary dependency
unavailable".
+::
+
+ ok 1 test_2 # SKIP this test should not run
+
+The test "test_2" was skipped with the diagnostic message "this test
+should not run".
+
::
not ok 1 test # TIMEOUT 30 seconds
@@ -225,7 +234,7 @@ An example format with multiple levels of nested testing:
not ok 1 test_1
ok 2 test_2
not ok 1 test_3
- ok 2 test_4 # SKIP
+ skip 2 test_4
not ok 1 example_test_1
ok 2 example_test_2
@@ -262,7 +271,7 @@ Example KTAP output
ok 1 example_test_1
KTAP version 2
1..2
- ok 1 test_1 # SKIP test_1 skipped
+ skip 1 test_1 # test_1 skipped
ok 2 test_2
ok 2 example_test_2
KTAP version 2
base-commit: 906f02e42adfbd5ae70d328ee71656ecb602aaf5
--
2.40.0.rc1.284.g88254d51c5-goog
In the middle of the thread about a patch to add the skip test result,
I suggested documenting the process of deprecating the KTAP v1 Specification
method of marking a skipped test:
https://lore.kernel.org/all/490271eb-1429-2217-6e38-837c6e5e328b@gmail.com/…
In a reply to that email I suggested that we ought to have a process to transition
the KTAP Specification from v1 to v2, and possibly v3 and future.
This email is meant to be the root of that discussion.
My initial thinking is that there are at least three different types of project
and/or community that may have different needs in this area.
Type 1 - project controls both the test output generation and the test output
parsing tool. Both generation and parsing code are in the same repository
and/or synchronized versions are distributed together.
Devicetree unittests are an example of Type 1. I plan to maintain changes
of test output to KTAP v2 format in coordination with updating the parser
to process KTAP v2 data.
Type 2 - project controls both the test output generation and the test output
parsing tool. The test output generation and a parser modifications may be
controlled by the project BUT there are one or more external testing projects
that (1) may have their own parsers, and (2) may have a single framework that
tests multiple versions of the tests.
I think that kselftest and kunit tests are probably examples of Type 2. I also
think that DT unittests will become a Type 2 project as a result of converting
to KTAP v2 data.
Type 3 - project may create and maintain some tests, but is primarily a consumer
of tests created by other projects. Type 3 projects typically have a single
framework that is able to execute and process multiple versions of the tests.
The Fuego test project is an example of Type 3.
Maybe adding all of this complexity of different Types in my initial thinking
was silly -- maybe everything in this topic is governed by the more complex
Type 3.
My thinking was that the three different Types of project would be impacted
in different ways by transition plans. Type 3 would be the most impacted,
so I wanted to be sure that any transition plan especially considered their
needs.
There is an important aspect of the KTAP format that might ease the transition
from one version to another: All KTAP formatted results begin with a "version
line", so as soon as a parser has processed the first line of a test, it can
apply the appropriate KTAP Specification version to all subsequent lines of
test output. A parser implementation could choose to process all versions,
could choose to invoke a version specific parser, or some other approach
all together.
In the "add skip test results" thread, I suggested deprecating the v1
method of marking a skipped test in v2, with a scheduled removal of
the v1 method in v3. But since the KTAP format version is available
in the very first line of test output, is it necessary to do a slow
deprecation and removal over two versions?
One argument to doing a two version deprecation/removal process is that
a parser that is one version older the the test output _might_ be able
to process the test output without error, but would not be able to take
advantage of features added in the newer version of the Specification.
My opinion is that a two version deprecation/removal process will slow
the Specification update process and lead to more versions of the
Specification over a given time interval.
A one version deprecation/removal process puts more of a burden on Type 3
projects and external parsers for Type 2 projects to implement parsers
that can process the newer Specification more quickly and puts a burden
on test maintainers to delay a move to the newer Specification, or possibly
pressure to support selection of more than one Specification version format
for output data.
One additional item... On the KTAP Specification version 2 process wiki page,
I suggested that it is "desirable for test result parsers that understand the
KTAP Specification version 2 data also be able to parse version 1 data."
With the implication "Converting version 1 compliant data to version 2 compliant
data should not require a "flag day" switch of test result parsers." If this
thread discussion results in a different decision, I will update the wiki.
Thoughts?
-Frank
On Sun, Mar 26, 2023 at 10:15:31AM +0200, Markus Elfring wrote:
[...]
> >>
> >> Fixes: a987785dcd6c8ae2915460582aebd6481c81eb67 ("Add tests for memory.oom.group")
> >
> > Fixes what in the what now?
>
> 1. Check repetition (which can be undesirable)
>
> 2. Can a cg_destroy() call ever work as expected if a cg_create() call failed?
Perhaps next time you can answer your own question by spending 30
seconds actually reading the code you're "fixing":
int cg_destroy(const char *cgroup)
{
int ret;
retry:
ret = rmdir(cgroup);
if (ret && errno == EBUSY) {
cg_killall(cgroup);
usleep(100);
goto retry;
}
if (ret && errno == ENOENT) <<< that case is explicitly handled here
ret = 0;
return ret;
}
This patch series includes miscellaneous update to the cpuset and its
testing code.
Patch 2 is actually a follow-up of commit 3fb906e7fabb ("cgroup/cpuset:
Don't filter offline CPUs in cpuset_cpus_allowed() for top cpuset tasks").
Patches 3-4 are for handling corner cases when dealing with
task_cpu_possible_mask().
Waiman Long (5):
cgroup/cpuset: Skip task update if hotplug doesn't affect current
cpuset
cgroup/cpuset: Include offline CPUs when tasks' cpumasks in top_cpuset
are updated
cgroup/cpuset: Find another usable CPU if none found in current cpuset
cgroup/cpuset: Add CONFIG_DEBUG_CPUSETS config for cpuset testing
cgroup/cpuset: Minor updates to test_cpuset_prs.sh
init/Kconfig | 5 +
kernel/cgroup/cpuset.c | 155 +++++++++++++++++-
.../selftests/cgroup/test_cpuset_prs.sh | 25 +--
3 files changed, 165 insertions(+), 20 deletions(-)
--
2.31.1